[Qemu-devel] [PATCH v2 0/4] tests: VM build in OpenBSD 6.3

[Qemu-devel] [PATCH v2 0/4] tests: VM build in OpenBSD 6.3

[Fam Zheng]

In this new version:

    - Include the virtio fix by Paolo so that it's easier to test this series.
    - Improve the slirp patch on input validation and buffer overflow. [Thomas]
    - Use OpenBSD 6.3 image; Use guestfwd and tftp-server-name.

Thanks to the loose dependency, the three parts (patch 1; patches 2+3; patch 4)
can go to individual maintainers if desired.

Fam Zheng (3):
  slirp: Add sanity check for str option length
  slirp: Implement RFC2132 TFTP server name
  tests: vm: auto_install OpenBSD

Paolo Bonzini (1):
  virtio: update MemoryRegionCaches when guest negotiates features

 hw/virtio/virtio.c | 15 +++++++--
 net/slirp.c        | 21 ++++++++++--
 qapi/net.json      |  5 ++-
 qemu-options.hx    |  7 +++-
 slirp/bootp.c      | 45 ++++++++++++++++++++------
 slirp/bootp.h      |  1 +
 slirp/libslirp.h   |  1 +
 slirp/slirp.c      |  2 ++
 slirp/slirp.h      |  1 +
 tests/vm/basevm.py |  6 ++--
 tests/vm/openbsd   | 81 ++++++++++++++++++++++++++++++++++++++++------
 11 files changed, 156 insertions(+), 29 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH v2 1/4] virtio: update MemoryRegionCaches when guest negotiates features

[Fam Zheng]

From: Paolo Bonzini <pbonzini@redhat.com>

Because the cache is sized to include the rings and the event indices,
negotiating the VIRTIO_RING_F_EVENT_IDX feature will result in the size
of the cache changing.  And because MemoryRegionCache accesses are
range-checked, if we skip this we end up with an assertion failure.
This happens with OpenBSD 6.3.

Reported-by: Fam Zheng <famz@redhat.com>
Fixes: 97cd965c070152bc626c7507df9fb356bbe1cd81
Cc: qemu-stable@nongnu.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 hw/virtio/virtio.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index d4e4d98b59..f6a588ab57 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2006,14 +2006,25 @@ static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val)
 
 int virtio_set_features(VirtIODevice *vdev, uint64_t val)
 {
-   /*
+    int ret;
+    /*
      * The driver must not attempt to set features after feature negotiation
      * has finished.
      */
     if (vdev->status & VIRTIO_CONFIG_S_FEATURES_OK) {
         return -EINVAL;
     }
-    return virtio_set_features_nocheck(vdev, val);
+    ret = virtio_set_features_nocheck(vdev, val);
+    if (!ret && virtio_vdev_has_feature(vdev, VIRTIO_RING_F_EVENT_IDX)) {
+        /* VIRTIO_RING_F_EVENT_IDX changes the size of the caches.  */
+        int i;
+        for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
+            if (vdev->vq[i].vring.num != 0) {
+                virtio_init_region_cache(vdev, i);
+            }
+        }
+    }
+    return ret;
 }
 
 int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
-- 
2.17.1

[Qemu-devel] [PATCH v2 2/4] slirp: Add sanity check for str option length

[Fam Zheng]

When user provides a long domainname or hostname that doesn't fit in the
DHCP packet, we mustn't overflow the response packet buffer. Instead,
report errors, following the g_warning() in the slirp->vdnssearch
branch.

Also check the strlen against 256 when initializing slirp, which limit
is also from the protocol where one byte represents the string length.
This gives an early error before the warning which is harder to notice
or diagnose.

Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
 net/slirp.c   |  9 +++++++++
 slirp/bootp.c | 32 ++++++++++++++++++++++----------
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index 1e14318b4d..fd21dc728c 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -365,6 +365,15 @@ static int net_slirp_init(NetClientState *peer, const char *model,
         return -1;
     }
 
+    if (vdomainname && strlen(vdomainname) > 255) {
+        error_setg(errp, "'domainname' parameter cannot exceed 255 bytes");
+        return -1;
+    }
+
+    if (vhostname && strlen(vhostname) > 255) {
+        error_setg(errp, "'vhostname' parameter cannot exceed 255 bytes");
+        return -1;
+    }
 
     nc = qemu_new_net_client(&net_slirp_info, peer, model, name);
 
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 9e7b53ba94..1e8185f0ec 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -159,6 +159,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
     struct in_addr preq_addr;
     int dhcp_msg_type, val;
     uint8_t *q;
+    uint8_t *end;
     uint8_t client_ethaddr[ETH_ALEN];
 
     /* extract exact DHCP msg type */
@@ -240,6 +241,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
     rbp->bp_siaddr = saddr.sin_addr; /* Server IP address */
 
     q = rbp->bp_vend;
+    end = (uint8_t *)&rbp[1];
     memcpy(q, rfc1533_cookie, 4);
     q += 4;
 
@@ -292,24 +294,33 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
 
         if (*slirp->client_hostname) {
             val = strlen(slirp->client_hostname);
-            *q++ = RFC1533_HOSTNAME;
-            *q++ = val;
-            memcpy(q, slirp->client_hostname, val);
-            q += val;
+            if (q + val + 2 >= end) {
+                g_warning("DHCP packet size exceeded, "
+                    "omitting host name option.");
+            } else {
+                *q++ = RFC1533_HOSTNAME;
+                *q++ = val;
+                memcpy(q, slirp->client_hostname, val);
+                q += val;
+            }
         }
 
         if (slirp->vdomainname) {
             val = strlen(slirp->vdomainname);
-            *q++ = RFC1533_DOMAINNAME;
-            *q++ = val;
-            memcpy(q, slirp->vdomainname, val);
-            q += val;
+            if (q + val + 2 >= end) {
+                g_warning("DHCP packet size exceeded, "
+                    "omitting domain name option.");
+            } else {
+                *q++ = RFC1533_DOMAINNAME;
+                *q++ = val;
+                memcpy(q, slirp->vdomainname, val);
+                q += val;
+            }
         }
 
         if (slirp->vdnssearch) {
-            size_t spaceleft = sizeof(rbp->bp_vend) - (q - rbp->bp_vend);
             val = slirp->vdnssearch_len;
-            if (val + 1 > spaceleft) {
+            if (q + val >= end) {
                 g_warning("DHCP packet size exceeded, "
                     "omitting domain-search option.");
             } else {
@@ -331,6 +342,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
         memcpy(q, nak_msg, sizeof(nak_msg) - 1);
         q += sizeof(nak_msg) - 1;
     }
+    assert(q < end);
     *q = RFC1533_END;
 
     daddr.sin_addr.s_addr = 0xffffffffu;
-- 
2.17.1

[Qemu-devel] [PATCH v2 3/4] slirp: Implement RFC2132 TFTP server name

[Fam Zheng]

This new usernet option can be used to add data for option 66 (tftp
server name) in the BOOTP reply, which is useful in PXE based automatic
OS install such as OpenBSD.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 net/slirp.c      | 12 ++++++++++--
 qapi/net.json    |  5 ++++-
 qemu-options.hx  |  7 ++++++-
 slirp/bootp.c    | 13 +++++++++++++
 slirp/bootp.h    |  1 +
 slirp/libslirp.h |  1 +
 slirp/slirp.c    |  2 ++
 slirp/slirp.h    |  1 +
 8 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index fd21dc728c..53f7b89696 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -158,6 +158,7 @@ static int net_slirp_init(NetClientState *peer, const char *model,
                           const char *vnameserver, const char *vnameserver6,
                           const char *smb_export, const char *vsmbserver,
                           const char **dnssearch, const char *vdomainname,
+                          const char *tftp_server_name,
                           Error **errp)
 {
     /* default settings according to historic slirp */
@@ -375,6 +376,11 @@ static int net_slirp_init(NetClientState *peer, const char *model,
         return -1;
     }
 
+    if (tftp_server_name && strlen(tftp_server_name) > 255) {
+        error_setg(errp, "'tftp-server-name' parameter cannot exceed 255 bytes");
+        return -1;
+    }
+
     nc = qemu_new_net_client(&net_slirp_info, peer, model, name);
 
     snprintf(nc->info_str, sizeof(nc->info_str),
@@ -385,7 +391,8 @@ static int net_slirp_init(NetClientState *peer, const char *model,
 
     s->slirp = slirp_init(restricted, ipv4, net, mask, host,
                           ipv6, ip6_prefix, vprefix6_len, ip6_host,
-                          vhostname, tftp_export, bootfile, dhcp,
+                          vhostname, tftp_server_name,
+                          tftp_export, bootfile, dhcp,
                           dns, ip6_dns, dnssearch, vdomainname, s);
     QTAILQ_INSERT_TAIL(&slirp_stacks, s, entry);
 
@@ -975,7 +982,8 @@ int net_init_slirp(const Netdev *netdev, const char *name,
                          user->ipv6_host, user->hostname, user->tftp,
                          user->bootfile, user->dhcpstart,
                          user->dns, user->ipv6_dns, user->smb,
-                         user->smbserver, dnssearch, user->domainname, errp);
+                         user->smbserver, dnssearch, user->domainname,
+                         user->tftp_server_name, errp);
 
     while (slirp_configs) {
         config = slirp_configs;
diff --git a/qapi/net.json b/qapi/net.json
index c86f351161..8f99fd911d 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -174,6 +174,8 @@
 #
 # @guestfwd: forward guest TCP connections
 #
+# @tftp-server-name: RFC2132 "TFTP server name" string (Since 3.1)
+#
 # Since: 1.2
 ##
 { 'struct': 'NetdevUserOptions',
@@ -198,7 +200,8 @@
     '*smb':       'str',
     '*smbserver': 'str',
     '*hostfwd':   ['String'],
-    '*guestfwd':  ['String'] } }
+    '*guestfwd':  ['String'],
+    '*tftp-server-name': 'str' } }
 
 ##
 # @NetdevTapOptions:
diff --git a/qemu-options.hx b/qemu-options.hx
index 654ef484d9..2c2acbb14b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1842,7 +1842,7 @@ DEF("netdev", HAS_ARG, QEMU_OPTION_netdev,
     "         [,ipv6[=on|off]][,ipv6-net=addr[/int]][,ipv6-host=addr]\n"
     "         [,restrict=on|off][,hostname=host][,dhcpstart=addr]\n"
     "         [,dns=addr][,ipv6-dns=addr][,dnssearch=domain][,domainname=domain]\n"
-    "         [,tftp=dir][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
+    "         [,tftp=dir][,tftp-server-name=name][,bootfile=f][,hostfwd=rule][,guestfwd=rule]"
 #ifndef _WIN32
                                              "[,smb=dir[,smbserver=addr]]\n"
 #endif
@@ -2079,6 +2079,11 @@ server. The files in @var{dir} will be exposed as the root of a TFTP server.
 The TFTP client on the guest must be configured in binary mode (use the command
 @code{bin} of the Unix TFTP client).
 
+@item tftp-server-name=@var{name}
+In BOOTP reply, broadcast @var{name} as the "TFTP server name" (RFC2132 option
+66). This can be used to advise the guest to load boot files or configurations
+from a different server than the host address.
+
 @item bootfile=@var{file}
 When using the user mode network stack, broadcast @var{file} as the BOOTP
 filename. In conjunction with @option{tftp}, this can be used to network boot
diff --git a/slirp/bootp.c b/slirp/bootp.c
index 1e8185f0ec..7b1af73c95 100644
--- a/slirp/bootp.c
+++ b/slirp/bootp.c
@@ -318,6 +318,19 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
             }
         }
 
+        if (slirp->tftp_server_name) {
+            val = strlen(slirp->tftp_server_name);
+            if (q + val + 2 >= end) {
+                g_warning("DHCP packet size exceeded, "
+                    "omitting tftp-server-name option.");
+            } else {
+                *q++ = RFC2132_TFTP_SERVER_NAME;
+                *q++ = val;
+                memcpy(q, slirp->tftp_server_name, val);
+                q += val;
+            }
+        }
+
         if (slirp->vdnssearch) {
             val = slirp->vdnssearch_len;
             if (q + val >= end) {
diff --git a/slirp/bootp.h b/slirp/bootp.h
index 394525733e..4043489835 100644
--- a/slirp/bootp.h
+++ b/slirp/bootp.h
@@ -70,6 +70,7 @@
 #define RFC2132_MAX_SIZE	57
 #define RFC2132_RENEWAL_TIME    58
 #define RFC2132_REBIND_TIME     59
+#define RFC2132_TFTP_SERVER_NAME 66
 
 #define DHCPDISCOVER		1
 #define DHCPOFFER		2
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index 740408a96e..42e42e9a2a 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -13,6 +13,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
                   bool in6_enabled,
                   struct in6_addr vprefix_addr6, uint8_t vprefix_len,
                   struct in6_addr vhost6, const char *vhostname,
+                  const char *tftp_server_name,
                   const char *tftp_path, const char *bootfile,
                   struct in_addr vdhcp_start, struct in_addr vnameserver,
                   struct in6_addr vnameserver6, const char **vdnssearch,
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 5c3bd6163f..51de41fc02 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -283,6 +283,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
                   bool in6_enabled,
                   struct in6_addr vprefix_addr6, uint8_t vprefix_len,
                   struct in6_addr vhost6, const char *vhostname,
+                  const char *tftp_server_name,
                   const char *tftp_path, const char *bootfile,
                   struct in_addr vdhcp_start, struct in_addr vnameserver,
                   struct in6_addr vnameserver6, const char **vdnssearch,
@@ -321,6 +322,7 @@ Slirp *slirp_init(int restricted, bool in_enabled, struct in_addr vnetwork,
     slirp->vdhcp_startaddr = vdhcp_start;
     slirp->vnameserver_addr = vnameserver;
     slirp->vnameserver_addr6 = vnameserver6;
+    slirp->tftp_server_name = g_strdup(tftp_server_name);
 
     if (vdnssearch) {
         translate_dnssearch(slirp, vdnssearch);
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 10b410898a..b80725a0d6 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -212,6 +212,7 @@ struct Slirp {
     /* tftp states */
     char *tftp_prefix;
     struct tftp_session tftp_sessions[TFTP_SESSIONS_MAX];
+    char *tftp_server_name;
 
     ArpTable arp_table;
     NdpTable ndp_table;
-- 
2.17.1

[Qemu-devel] [PATCH v2 4/4] tests: vm: auto_install OpenBSD

[Fam Zheng]

Upgrade OpenBSD to 6.3 using auto_install. Especially, drop SDL1,
include SDL2.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/vm/basevm.py |  6 ++--
 tests/vm/openbsd   | 81 ++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 74 insertions(+), 13 deletions(-)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 7e58d9e0ca..3f5e9e48cb 100755
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -65,8 +65,6 @@ class BaseVM(object):
             self._stdout = self._devnull
         self._args = [ \
             "-nodefaults", "-m", "4G",
-            "-netdev", "user,id=vnet,hostfwd=:127.0.0.1:0-:22",
-            "-device", "virtio-net-pci,netdev=vnet",
             "-vnc", "127.0.0.1:0,to=20",
             "-serial", "file:%s" % os.path.join(self._tmpdir, "serial.out")]
         if vcpus:
@@ -145,8 +143,10 @@ class BaseVM(object):
                             "-device",
                             "virtio-blk,drive=%s,serial=%s,bootindex=1" % (name, name)]
 
-    def boot(self, img, extra_args=[]):
+    def boot(self, img, extra_args=[], extra_usernet_args=""):
         args = self._args + [
+            "-netdev", "user,id=vnet,hostfwd=:127.0.0.1:0-:22" + extra_usernet_args,
+            "-device", "virtio-net-pci,netdev=vnet",
             "-device", "VGA",
             "-drive", "file=%s,if=none,id=drive0,cache=writeback" % img,
             "-device", "virtio-blk,drive=drive0,bootindex=0"]
diff --git a/tests/vm/openbsd b/tests/vm/openbsd
index 52500ee52b..629a219ffb 100755
--- a/tests/vm/openbsd
+++ b/tests/vm/openbsd
@@ -14,6 +14,9 @@
 import os
 import sys
 import subprocess
+import time
+import atexit
+import tempfile
 import basevm
 
 class OpenBSDVM(basevm.BaseVM):
@@ -23,22 +26,80 @@ class OpenBSDVM(basevm.BaseVM):
         rm -rf /var/tmp/qemu-test.*
         cd $(mktemp -d /var/tmp/qemu-test.XXXXXX);
         tar -xf /dev/rsd1c;
-        ./configure --cc=x86_64-unknown-openbsd6.1-gcc-4.9.4 --python=python2.7 {configure_opts};
+        ./configure {configure_opts};
         gmake --output-sync -j{jobs} {verbose};
         # XXX: "gmake check" seems to always hang or fail
         #gmake --output-sync -j{jobs} check {verbose};
     """
 
+    def _install_os(self, img):
+        tmpdir = tempfile.mkdtemp()
+        pxeboot = self._download_with_cache("https://fastly.cdn.openbsd.org/pub/OpenBSD/6.3/amd64/pxeboot",
+                sha256sum="60029919798f48ea40ecb123adfed6217f099d5ed9cd1a6c7de5b544d7b7b0f6")
+        bsd_rd = self._download_with_cache("https://fastly.cdn.openbsd.org/pub/OpenBSD/6.3/amd64/bsd.rd",
+                sha256sum="1c0adb43a02ae3aee512bcf0829dac0ccb2e4d614b161049af7ce530e5da2dfc")
+        install = self._download_with_cache("https://fastly.cdn.openbsd.org/pub/OpenBSD/6.3/amd64/install63.iso",
+                sha256sum='ee775405dd7926975befbc3fef23de8c4b5a726c3b5075e4848fcd3a2a712ea8')
+        subprocess.check_call(["qemu-img", "create", img, "32G"])
+        subprocess.check_call(["cp", pxeboot, os.path.join(tmpdir, "auto_install")])
+        subprocess.check_call(["cp", bsd_rd, os.path.join(tmpdir, "bsd")])
+
+        self._gen_install_conf(tmpdir)
+        # BOOTP filename being auto_install makes sure OpenBSD installer
+        # not prompt for "auto install mode"
+        usernet_args = ",tftp=%s,bootfile=/auto_install" % tmpdir
+        usernet_args += ",tftp-server-name=10.0.2.4"
+        usernet_args += ",guestfwd=tcp:10.0.2.4:80-cmd:cat %s" % \
+            os.path.join(tmpdir, "install.conf")
+        self.boot(img,
+                  extra_args=["-boot", "once=n", "-no-reboot",
+                              "-cdrom", install],
+                  extra_usernet_args=usernet_args)
+        self.wait()
+
+    def _gen_install_conf(self, tmpdir):
+        contents = """\
+HTTP/1.0 200 OK
+
+System hostname = qemu-openbsd
+Password for root = qemupass
+Public ssh key for root = {pub_key}
+Allow root ssh login = yes
+Network interfaces = vio0
+IPv4 address for vio0 = dhcp
+Setup a user = qemu
+Password for user = qemupass
+Public ssh key for user = {pub_key}
+What timezone are you in = US/Eastern
+Server = fastly.cdn.openbsd.org
+Use http = yes
+Default IPv4 route = 10.0.2.2
+Location of sets = cd0
+Set name(s) = all
+Continue without verification = yes
+""".format(pub_key=basevm.SSH_PUB_KEY)
+        with open(os.path.join(tmpdir, "install.conf"), "w") as f:
+            f.write(contents)
+
     def build_image(self, img):
-        cimg = self._download_with_cache("http://download.patchew.org/openbsd-6.1-amd64.img.xz",
-                sha256sum='8c6cedc483e602cfee5e04f0406c64eb99138495e8ca580bc0293bcf0640c1bf')
-        img_tmp_xz = img + ".tmp.xz"
-        img_tmp = img + ".tmp"
-        subprocess.check_call(["cp", "-f", cimg, img_tmp_xz])
-        subprocess.check_call(["xz", "-df", img_tmp_xz])
-        if os.path.exists(img):
-            os.remove(img)
-        os.rename(img_tmp, img)
+
+        self._install_os(img + ".tmp")
+
+        self.boot(img + ".tmp")
+        self.wait_ssh()
+
+        self.ssh_root("usermod -G operator qemu")
+        self.ssh_root("echo https://fastly.cdn.openbsd.org/pub/OpenBSD > /etc/installurl")
+        for pkg in ["git", "gmake", "glib2", "bison", "sdl2"]:
+            self.ssh_root("pkg_add " + pkg)
+        self.ssh_root("ln -sf /usr/local/bin/python2.7 /usr/local/bin/python")
+        self.ssh_root("ln -sf /usr/local/bin/python2.7-2to3 /usr/local/bin/2to3")
+        self.ssh_root("ln -sf /usr/local/bin/python2.7-config /usr/local/bin/python-config")
+        self.ssh_root("ln -sf /usr/local/bin/pydoc2.7 /usr/local/bin/pydoc")
+        self.ssh_root("shutdown -p now")
+        self.wait()
+
+        subprocess.check_call(["mv", img + ".tmp", img])
 
 if __name__ == "__main__":
     sys.exit(basevm.main(OpenBSDVM))
-- 
2.17.1

Re: [Qemu-devel] [PATCH v2 2/4] slirp: Add sanity check for str option length

[Thomas Huth]

On 2018-09-06 07:43, Fam Zheng wrote:
> When user provides a long domainname or hostname that doesn't fit in the
> DHCP packet, we mustn't overflow the response packet buffer. Instead,
> report errors, following the g_warning() in the slirp->vdnssearch
> branch.
> 
> Also check the strlen against 256 when initializing slirp, which limit
> is also from the protocol where one byte represents the string length.
> This gives an early error before the warning which is harder to notice
> or diagnose.
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  net/slirp.c   |  9 +++++++++
>  slirp/bootp.c | 32 ++++++++++++++++++++++----------
>  2 files changed, 31 insertions(+), 10 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index 1e14318b4d..fd21dc728c 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -365,6 +365,15 @@ static int net_slirp_init(NetClientState *peer, const char *model,
>          return -1;
>      }
>  
> +    if (vdomainname && strlen(vdomainname) > 255) {
> +        error_setg(errp, "'domainname' parameter cannot exceed 255 bytes");
> +        return -1;
> +    }
> +
> +    if (vhostname && strlen(vhostname) > 255) {
> +        error_setg(errp, "'vhostname' parameter cannot exceed 255 bytes");
> +        return -1;
> +    }
>  
>      nc = qemu_new_net_client(&net_slirp_info, peer, model, name);
>  
> diff --git a/slirp/bootp.c b/slirp/bootp.c
> index 9e7b53ba94..1e8185f0ec 100644
> --- a/slirp/bootp.c
> +++ b/slirp/bootp.c
> @@ -159,6 +159,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
>      struct in_addr preq_addr;
>      int dhcp_msg_type, val;
>      uint8_t *q;
> +    uint8_t *end;
>      uint8_t client_ethaddr[ETH_ALEN];
>  
>      /* extract exact DHCP msg type */
> @@ -240,6 +241,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
>      rbp->bp_siaddr = saddr.sin_addr; /* Server IP address */
>  
>      q = rbp->bp_vend;
> +    end = (uint8_t *)&rbp[1];
>      memcpy(q, rfc1533_cookie, 4);
>      q += 4;
>  
> @@ -292,24 +294,33 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
>  
>          if (*slirp->client_hostname) {
>              val = strlen(slirp->client_hostname);
> -            *q++ = RFC1533_HOSTNAME;
> -            *q++ = val;
> -            memcpy(q, slirp->client_hostname, val);
> -            q += val;
> +            if (q + val + 2 >= end) {
> +                g_warning("DHCP packet size exceeded, "
> +                    "omitting host name option.");
> +            } else {
> +                *q++ = RFC1533_HOSTNAME;
> +                *q++ = val;
> +                memcpy(q, slirp->client_hostname, val);
> +                q += val;
> +            }
>          }
>  
>          if (slirp->vdomainname) {
>              val = strlen(slirp->vdomainname);
> -            *q++ = RFC1533_DOMAINNAME;
> -            *q++ = val;
> -            memcpy(q, slirp->vdomainname, val);
> -            q += val;
> +            if (q + val + 2 >= end) {
> +                g_warning("DHCP packet size exceeded, "
> +                    "omitting domain name option.");
> +            } else {
> +                *q++ = RFC1533_DOMAINNAME;
> +                *q++ = val;
> +                memcpy(q, slirp->vdomainname, val);
> +                q += val;
> +            }
>          }
>  
>          if (slirp->vdnssearch) {
> -            size_t spaceleft = sizeof(rbp->bp_vend) - (q - rbp->bp_vend);
>              val = slirp->vdnssearch_len;
> -            if (val + 1 > spaceleft) {
> +            if (q + val >= end) {
>                  g_warning("DHCP packet size exceeded, "
>                      "omitting domain-search option.");
>              } else {
> @@ -331,6 +342,7 @@ static void bootp_reply(Slirp *slirp, const struct bootp_t *bp)
>          memcpy(q, nak_msg, sizeof(nak_msg) - 1);
>          q += sizeof(nak_msg) - 1;
>      }
> +    assert(q < end);
>      *q = RFC1533_END;
>  
>      daddr.sin_addr.s_addr = 0xffffffffu;
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

Since this can also fix potential QEMU crashes, I think this patch
should also go into the stable branches (put on CC: now).

Re: [Qemu-devel] [PATCH v2 3/4] slirp: Implement RFC2132 TFTP server name

[Thomas Huth]

On 2018-09-06 07:43, Fam Zheng wrote:
> This new usernet option can be used to add data for option 66 (tftp
> server name) in the BOOTP reply, which is useful in PXE based automatic
> OS install such as OpenBSD.
> 
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
>  net/slirp.c      | 12 ++++++++++--
>  qapi/net.json    |  5 ++++-
>  qemu-options.hx  |  7 ++++++-
>  slirp/bootp.c    | 13 +++++++++++++
>  slirp/bootp.h    |  1 +
>  slirp/libslirp.h |  1 +
>  slirp/slirp.c    |  2 ++
>  slirp/slirp.h    |  1 +
>  8 files changed, 38 insertions(+), 4 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH v2 0/4] tests: VM build in OpenBSD 6.3

[Gerd Hoffmann]

On Thu, Sep 06, 2018 at 01:43:36PM +0800, Fam Zheng wrote:
> In this new version:
> 
>     - Include the virtio fix by Paolo so that it's easier to test this series.
>     - Improve the slirp patch on input validation and buffer overflow. [Thomas]
>     - Use OpenBSD 6.3 image; Use guestfwd and tftp-server-name.
> 
> Thanks to the loose dependency, the three parts (patch 1; patches 2+3; patch 4)
> can go to individual maintainers if desired.
> 
> Fam Zheng (3):
>   slirp: Add sanity check for str option length
>   slirp: Implement RFC2132 TFTP server name
>   tests: vm: auto_install OpenBSD
> 
> Paolo Bonzini (1):
>   virtio: update MemoryRegionCaches when guest negotiates features

Tested-by: Gerd Hoffmann <kraxel@redhat.com>

Re: [Qemu-devel] [PATCH v2 0/4] tests: VM build in OpenBSD 6.3

[Fam Zheng]

On Tue, 09/11 11:00, Gerd Hoffmann wrote:
> On Thu, Sep 06, 2018 at 01:43:36PM +0800, Fam Zheng wrote:
> > In this new version:
> > 
> >     - Include the virtio fix by Paolo so that it's easier to test this series.
> >     - Improve the slirp patch on input validation and buffer overflow. [Thomas]
> >     - Use OpenBSD 6.3 image; Use guestfwd and tftp-server-name.
> > 
> > Thanks to the loose dependency, the three parts (patch 1; patches 2+3; patch 4)
> > can go to individual maintainers if desired.
> > 
> > Fam Zheng (3):
> >   slirp: Add sanity check for str option length
> >   slirp: Implement RFC2132 TFTP server name
> >   tests: vm: auto_install OpenBSD
> > 
> > Paolo Bonzini (1):
> >   virtio: update MemoryRegionCaches when guest negotiates features
> 
> Tested-by: Gerd Hoffmann <kraxel@redhat.com>
> 

Thanks. I think Michael has included the virtio patch in a pull already. I'll
pick up your tested-by tag and queue the OpenBSD patch, then leave the slirp
ones to Samuel (the test script will still depend on the new slirp options, of
course).

Fam