bhyve: regression in virtio-9p option parsing

[Roman Bogorodskiy]

Hi,

I'm noticing a behavior that looks like a regression in virtio-9p option
parsing.

I have a command line that has been working for me for a while, relevant
part is:

 bhyve ... -s 7:0,virtio-9p,distfiles=/workspace/distfiles ...

At some point it started to fail with:

virtio-9p: more than one share name given

Even though there's only one share name.

As I can see, the relevant code for that is:

static int
pci_vt9p_legacy_config(nvlist_t *nvl, const char *opts)
{
        char *sharename, *tofree, *token, *tokens;

        if (opts == NULL)
                return (0);

        tokens = tofree = strdup(opts);
        while ((token = strsep(&tokens, ",")) != NULL) {
                if (strchr(token, '=') != NULL) {
                        if (sharename != NULL) {
                                EPRINTLN(
                            "virtio-9p: more than one share name given");
                                return (-1);
                        }

                        sharename = strsep(&token, "=");
                        set_config_value_node(nvl, "sharename", sharename);
                        set_config_value_node(nvl, "path", token);
                } else
                        set_config_bool_node(nvl, token, true);
        }
        free(tofree);
        return (0);
}

And it fails at the first iteration, likely because the sharename was
not initialised and points to some arbitrary non-NULL value.

Explicitly setting it to NULL like this:

diff --git a/usr.sbin/bhyve/pci_virtio_9p.c b/usr.sbin/bhyve/pci_virtio_9p.c
index e27159eb22cb..830e13878a71 100644
--- a/usr.sbin/bhyve/pci_virtio_9p.c
+++ b/usr.sbin/bhyve/pci_virtio_9p.c
@@ -232,7 +232,7 @@ pci_vt9p_notify(void *vsc, struct vqueue_info *vq)
 static int
 pci_vt9p_legacy_config(nvlist_t *nvl, const char *opts)
 {
-       char *sharename, *tofree, *token, *tokens;
+       char *sharename = NULL, *tofree, *token, *tokens;

        if (opts == NULL)
                return (0);

appears to fall back to the normal behavior: it accepts a single share
name just fine, but shows that error if I try to specify more than one
share name, e.g.:

-s 7:0,virtio-9p,distfiles=/workspace/distfiles,foo=bar

It looks like the regression was introduced by the global variables
commit (https://reviews.freebsd.org/D26035). The old code had
"char *sharename = NULL".

Roman Bogorodskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-virtualization/attachments/20210407/95c7e000/attachment.sig>