Skip site navigation (1)Skip section navigation (2) 
Date:      Wed, 7 Apr 2021 19:30:25 +0400
From:      Roman Bogorodskiy <novel@freebsd.org>
To:        freebsd-virtualization@freebsd.org
Subject:   bhyve: regression in virtio-9p option parsing
Message-ID:  <YG3QEdWmaqKFlncI@kloomba>
next in thread | raw e-mail | index | archive | help 

--NM/+hEyntSvwDaB0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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

--NM/+hEyntSvwDaB0
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQEzBAABCAAdFiEEi6TfKtFPmbY34ABwyW1f/gjCImoFAmBt0A4ACgkQyW1f/gjC
ImoPjwf/WUHc/7A2HuZcDNS+V39ZgqvQmGw/3dy7yRqXGY/dNhQJKvcesd1qRF3K
3HXbW6aQ150qXSq86fdI3HVoCuAnqKSaLuEOieKOpu3stMnuyY/We86ewHwiGd5W
yDsl6SK2RY0Dgl+NRTBFUxaGOdC9PGSbIYNR+G6Fw5/b7DlpkJWUwQceKIHkj+8O
Ow7pjoor9V7UyHttCIIiH+0VIppbte4z0GqNSL7+UzVGh87ghJ0pgWKBPwHtCqkp
3HeU5XuTu+tB7FKet2pe3MsvV4Q9lXsNrX8X6pP6fOCAAb/6H0u2fggS5m8prJ8h
u2dN5+I+YPUpZQIPFQrmhbWNIBTY2A==
=/aLQ
-----END PGP SIGNATURE-----

--NM/+hEyntSvwDaB0--

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?YG3QEdWmaqKFlncI>