[Qemu-devel] [PATCH V5 for 3.1 0/5] Fix possible OOB during queuing packets

[Qemu-devel] [PATCH V5 for 3.1 0/5] Fix possible OOB during queuing packets

[Jason Wang]

Hi:

This series tries to fix a possible OOB during queueing packets
through qemu_net_queue_append_iov(). This could happen when it tries
to queue a packet whose size is larger than INT_MAX which may lead
integer overflow. We've fixed similar issue in the past during
qemu_net_queue_deliver_iov() by ignoring large packets there. Let's
just move the check earlier to qemu_sendv_packet_async() and reduce
the limitation to NET_BUFSIZE. A simple qtest were also added this.

Please review.

Thanks

Changes from V1:
- slient compiling warnings
Changes from V2:
- annotate pci_test_start() with GCC_FMT_ATTR()
- drop intermediate cmd string variable
Changes from V4:
- silent hub warning if qtest is enabled
- make qemu_deliver_packet_iov() static
- add one more test for packet size slightly greater than NET_BUFSIZE
- tweak the commit log and add more justification
- typo/whitespace fixes and other minor code style tweaks

Jason Wang (5):
  net: drop too large packet early
  net: hub: suppress warnings of no host network for qtest
  virtio-net-test: accept variable length argument in pci_test_start()
  virtio-net-test: remove unused macro
  virtio-net-test: add large tx buffer test

 include/net/net.h       |  6 ----
 net/hub.c               |  3 +-
 net/net.c               | 28 +++++++++++-------
 tests/virtio-net-test.c | 64 ++++++++++++++++++++++++++++++++++++-----
 4 files changed, 76 insertions(+), 25 deletions(-)

-- 
2.17.1

[Qemu-devel] [PATCH V5 for 3.1 1/5] net: drop too large packet early

[Jason Wang]

We try to detect and drop too large packet (>INT_MAX) in 1592a9947036
("net: ignore packet size greater than INT_MAX") during packet
delivering. Unfortunately, this is not sufficient as we may hit
another integer overflow when trying to queue such large packet in
qemu_net_queue_append_iov():

- size of the allocation may overflow on 32bit
- packet->size is integer which may overflow even on 64bit

Fixing this by moving the check to qemu_sendv_packet_async() which is
the entrance of all networking codes and reduce the limit to
NET_BUFSIZE to be more conservative. This works since:

- For the callers that call qemu_sendv_packet_async() directly, they
  only care about if zero is returned to determine whether to prevent
  the source from producing more packets. A callback will be triggered
  if peer can accept more then source could be enabled. This is
  usually used by high speed networking implementation like virtio-net
  or netmap.
- For the callers that call qemu_sendv_packet() that calls
  qemu_sendv_packet_async() indirectly, they often ignore the return
  value. In this case qemu will just the drop packets if peer can't
  receive.

Qemu will copy the packet if it was queued. So it was safe for both
kinds of the callers to assume the packet was sent.

Since we move the check from qemu_deliver_packet_iov() to
qemu_sendv_packet_async(), it would be safer to make
qemu_deliver_packet_iov() static to prevent any external user in the
future.

This is a revised patch of CVE-2018-17963.

Cc: qemu-stable@nongnu.org
Cc: Li Qiang <liq3ea@163.com>
Fixes: 1592a9947036 ("net: ignore packet size greater than INT_MAX")
Reported-by: Li Qiang <liq3ea@gmail.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 include/net/net.h |  6 ------
 net/net.c         | 28 +++++++++++++++++-----------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/include/net/net.h b/include/net/net.h
index 7936d53d2f..ec13702dbf 100644
--- a/include/net/net.h
+++ b/include/net/net.h
@@ -169,12 +169,6 @@ void qemu_check_nic_model(NICInfo *nd, const char *model);
 int qemu_find_nic_model(NICInfo *nd, const char * const *models,
                         const char *default_model);
 
-ssize_t qemu_deliver_packet_iov(NetClientState *sender,
-                            unsigned flags,
-                            const struct iovec *iov,
-                            int iovcnt,
-                            void *opaque);
-
 void print_net_client(Monitor *mon, NetClientState *nc);
 void hmp_info_network(Monitor *mon, const QDict *qdict);
 void net_socket_rs_init(SocketReadState *rs,
diff --git a/net/net.c b/net/net.c
index 07c194a8f6..1f7d626197 100644
--- a/net/net.c
+++ b/net/net.c
@@ -231,6 +231,11 @@ static void qemu_net_client_destructor(NetClientState *nc)
 {
     g_free(nc);
 }
+static ssize_t qemu_deliver_packet_iov(NetClientState *sender,
+                                       unsigned flags,
+                                       const struct iovec *iov,
+                                       int iovcnt,
+                                       void *opaque);
 
 static void qemu_net_client_setup(NetClientState *nc,
                                   NetClientInfo *info,
@@ -705,22 +710,18 @@ static ssize_t nc_sendv_compat(NetClientState *nc, const struct iovec *iov,
     return ret;
 }
 
-ssize_t qemu_deliver_packet_iov(NetClientState *sender,
-                                unsigned flags,
-                                const struct iovec *iov,
-                                int iovcnt,
-                                void *opaque)
+static ssize_t qemu_deliver_packet_iov(NetClientState *sender,
+                                       unsigned flags,
+                                       const struct iovec *iov,
+                                       int iovcnt,
+                                       void *opaque)
 {
     NetClientState *nc = opaque;
-    size_t size = iov_size(iov, iovcnt);
     int ret;
 
-    if (size > INT_MAX) {
-        return size;
-    }
 
     if (nc->link_down) {
-        return size;
+        return iov_size(iov, iovcnt);
     }
 
     if (nc->receive_disabled) {
@@ -745,10 +746,15 @@ ssize_t qemu_sendv_packet_async(NetClientState *sender,
                                 NetPacketSent *sent_cb)
 {
     NetQueue *queue;
+    size_t size = iov_size(iov, iovcnt);
     int ret;
 
+    if (size > NET_BUFSIZE) {
+        return size;
+    }
+
     if (sender->link_down || !sender->peer) {
-        return iov_size(iov, iovcnt);
+        return size;
     }
 
     /* Let filters handle the packet first */
-- 
2.17.1

[Qemu-devel] [PATCH V5 for 3.1 2/5] net: hub: suppress warnings of no host network for qtest

[Jason Wang]

If we want to qtest through hub, it would be much more simpler and
safer to configure the hub without host network. So silent this
warnings for qtest.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 net/hub.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/hub.c b/net/hub.c
index 78b671ed95..5795a678ed 100644
--- a/net/hub.c
+++ b/net/hub.c
@@ -20,6 +20,7 @@
 #include "hub.h"
 #include "qemu/iov.h"
 #include "qemu/error-report.h"
+#include "sysemu/qtest.h"
 
 /*
  * A hub broadcasts incoming packets to all its ports except the source port.
@@ -346,7 +347,7 @@ void net_hub_check_clients(void)
         if (has_host_dev && !has_nic) {
             warn_report("hub %d with no nics", hub->id);
         }
-        if (has_nic && !has_host_dev) {
+        if (has_nic && !has_host_dev && !qtest_enabled()) {
             warn_report("hub %d is not connected to host network", hub->id);
         }
     }
-- 
2.17.1

[Qemu-devel] [PATCH V5 for 3.1 3/5] virtio-net-test: accept variable length argument in pci_test_start()

[Jason Wang]

This allows flexibility to be reused for all kinds of command line
used by other tests.

Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 tests/virtio-net-test.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
index dcb87a8b6e..587a043e67 100644
--- a/tests/virtio-net-test.c
+++ b/tests/virtio-net-test.c
@@ -52,17 +52,21 @@ static QVirtioPCIDevice *virtio_net_pci_init(QPCIBus *bus, int slot)
     return dev;
 }
 
-static QOSState *pci_test_start(int socket)
+GCC_FMT_ATTR(1, 2)
+static QOSState *pci_test_start(const char *cmd, ...)
 {
     QOSState *qs;
+    va_list ap;
     const char *arch = qtest_get_arch();
-    const char *cmd = "-netdev socket,fd=%d,id=hs0 -device "
-                      "virtio-net-pci,netdev=hs0";
 
     if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
-        qs = qtest_pc_boot(cmd, socket);
+        va_start(ap, cmd);
+        qs = qtest_pc_vboot(cmd, ap);
+        va_end(ap);
     } else if (strcmp(arch, "ppc64") == 0) {
-        qs = qtest_spapr_boot(cmd, socket);
+        va_start(ap, cmd);
+        qs = qtest_spapr_vboot(cmd, ap);
+        va_end(ap);
     } else {
         g_printerr("virtio-net tests are only available on x86 or ppc64\n");
         exit(EXIT_FAILURE);
@@ -223,7 +227,8 @@ static void pci_basic(gconstpointer data)
     ret = socketpair(PF_UNIX, SOCK_STREAM, 0, sv);
     g_assert_cmpint(ret, !=, -1);
 
-    qs = pci_test_start(sv[1]);
+    qs = pci_test_start("-netdev socket,fd=%d,id=hs0 -device "
+                        "virtio-net-pci,netdev=hs0", sv[1]);
     dev = virtio_net_pci_init(qs->pcibus, PCI_SLOT);
 
     rx = (QVirtQueuePCI *)qvirtqueue_setup(&dev->vdev, qs->alloc, 0);
-- 
2.17.1

[Qemu-devel] [PATCH V5 for 3.1 4/5] virtio-net-test: remove unused macro

[Jason Wang]

Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 tests/virtio-net-test.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
index 587a043e67..bdd6af9999 100644
--- a/tests/virtio-net-test.c
+++ b/tests/virtio-net-test.c
@@ -24,7 +24,6 @@
 
 #define PCI_SLOT_HP             0x06
 #define PCI_SLOT                0x04
-#define PCI_FN                  0x00
 
 #define QVIRTIO_NET_TIMEOUT_US (30 * 1000 * 1000)
 #define VNET_HDR_SIZE sizeof(struct virtio_net_hdr_mrg_rxbuf)
-- 
2.17.1

[Qemu-devel] [PATCH V5 for 3.1 5/5] virtio-net-test: add large tx buffer test

[Jason Wang]

This test tries to build a packet whose size is greater than INT_MAX
which tries to trigger integer overflow in qemu_net_queue_append_iov()
which may result OOB.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 tests/virtio-net-test.c | 46 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
index bdd6af9999..e9783e6707 100644
--- a/tests/virtio-net-test.c
+++ b/tests/virtio-net-test.c
@@ -245,6 +245,48 @@ static void pci_basic(gconstpointer data)
     g_free(dev);
     qtest_shutdown(qs);
 }
+
+static void large_tx(gconstpointer data)
+{
+    QVirtioPCIDevice *dev;
+    QOSState *qs;
+    QVirtQueuePCI *tx, *rx;
+    QVirtQueue *vq;
+    uint64_t req_addr;
+    uint32_t free_head;
+    size_t alloc_size = (size_t)data / 64;
+    int i;
+
+    qs = pci_test_start("-netdev hubport,id=hp0,hubid=0 "
+                        "-device virtio-net-pci,netdev=hp0");
+    dev = virtio_net_pci_init(qs->pcibus, PCI_SLOT);
+
+    rx = (QVirtQueuePCI *)qvirtqueue_setup(&dev->vdev, qs->alloc, 0);
+    tx = (QVirtQueuePCI *)qvirtqueue_setup(&dev->vdev, qs->alloc, 1);
+
+    driver_init(&dev->vdev);
+    vq = &tx->vq;
+
+    /* Bypass the limitation by pointing several descriptors to a single
+     * smaller area */
+    req_addr = guest_alloc(qs->alloc, alloc_size);
+    free_head = qvirtqueue_add(vq, req_addr, alloc_size, false, true);
+
+    for (i = 0; i < 64; i++) {
+        qvirtqueue_add(vq, req_addr, alloc_size, false, i != 63);
+    }
+    qvirtqueue_kick(&dev->vdev, vq, free_head);
+
+    qvirtio_wait_used_elem(&dev->vdev, vq, free_head, NULL,
+                           QVIRTIO_NET_TIMEOUT_US);
+
+    qvirtqueue_cleanup(dev->vdev.bus, &tx->vq, qs->alloc);
+    qvirtqueue_cleanup(dev->vdev.bus, &rx->vq, qs->alloc);
+    qvirtio_pci_device_disable(dev);
+    g_free(dev->pdev);
+    g_free(dev);
+    qtest_shutdown(qs);
+}
 #endif
 
 static void hotplug(void)
@@ -270,6 +312,10 @@ int main(int argc, char **argv)
     qtest_add_data_func("/virtio/net/pci/basic", send_recv_test, pci_basic);
     qtest_add_data_func("/virtio/net/pci/rx_stop_cont",
                         stop_cont_test, pci_basic);
+    qtest_add_data_func("/virtio/net/pci/large_tx_uint_max",
+                        (gconstpointer)UINT_MAX, large_tx);
+    qtest_add_data_func("/virtio/net/pci/large_tx_net_bufsize",
+                        (gconstpointer)NET_BUFSIZE, large_tx);
 #endif
     qtest_add_func("/virtio/net/pci/hotplug", hotplug);
 
-- 
2.17.1

Re: [Qemu-devel] [PATCH V5 for 3.1 1/5] net: drop too large packet early

[Thomas Huth]

On 2018-12-04 04:53, Jason Wang wrote:
> We try to detect and drop too large packet (>INT_MAX) in 1592a9947036
> ("net: ignore packet size greater than INT_MAX") during packet
> delivering. Unfortunately, this is not sufficient as we may hit
> another integer overflow when trying to queue such large packet in
> qemu_net_queue_append_iov():
> 
> - size of the allocation may overflow on 32bit
> - packet->size is integer which may overflow even on 64bit
> 
> Fixing this by moving the check to qemu_sendv_packet_async() which is
> the entrance of all networking codes and reduce the limit to
> NET_BUFSIZE to be more conservative. This works since:
> 
> - For the callers that call qemu_sendv_packet_async() directly, they
>   only care about if zero is returned to determine whether to prevent
>   the source from producing more packets. A callback will be triggered
>   if peer can accept more then source could be enabled. This is
>   usually used by high speed networking implementation like virtio-net
>   or netmap.
> - For the callers that call qemu_sendv_packet() that calls
>   qemu_sendv_packet_async() indirectly, they often ignore the return
>   value. In this case qemu will just the drop packets if peer can't
>   receive.
> 
> Qemu will copy the packet if it was queued. So it was safe for both
> kinds of the callers to assume the packet was sent.
> 
> Since we move the check from qemu_deliver_packet_iov() to
> qemu_sendv_packet_async(), it would be safer to make
> qemu_deliver_packet_iov() static to prevent any external user in the
> future.
> 
> This is a revised patch of CVE-2018-17963.
> 
> Cc: qemu-stable@nongnu.org
> Cc: Li Qiang <liq3ea@163.com>
> Fixes: 1592a9947036 ("net: ignore packet size greater than INT_MAX")
> Reported-by: Li Qiang <liq3ea@gmail.com>
> Reviewed-by: Li Qiang <liq3ea@gmail.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  include/net/net.h |  6 ------
>  net/net.c         | 28 +++++++++++++++++-----------
>  2 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/include/net/net.h b/include/net/net.h
> index 7936d53d2f..ec13702dbf 100644
> --- a/include/net/net.h
> +++ b/include/net/net.h
> @@ -169,12 +169,6 @@ void qemu_check_nic_model(NICInfo *nd, const char *model);
>  int qemu_find_nic_model(NICInfo *nd, const char * const *models,
>                          const char *default_model);
>  
> -ssize_t qemu_deliver_packet_iov(NetClientState *sender,
> -                            unsigned flags,
> -                            const struct iovec *iov,
> -                            int iovcnt,
> -                            void *opaque);
> -
>  void print_net_client(Monitor *mon, NetClientState *nc);
>  void hmp_info_network(Monitor *mon, const QDict *qdict);
>  void net_socket_rs_init(SocketReadState *rs,
> diff --git a/net/net.c b/net/net.c
> index 07c194a8f6..1f7d626197 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -231,6 +231,11 @@ static void qemu_net_client_destructor(NetClientState *nc)
>  {
>      g_free(nc);
>  }
> +static ssize_t qemu_deliver_packet_iov(NetClientState *sender,
> +                                       unsigned flags,
> +                                       const struct iovec *iov,
> +                                       int iovcnt,
> +                                       void *opaque);
>  
>  static void qemu_net_client_setup(NetClientState *nc,
>                                    NetClientInfo *info,
> @@ -705,22 +710,18 @@ static ssize_t nc_sendv_compat(NetClientState *nc, const struct iovec *iov,
>      return ret;
>  }
>  
> -ssize_t qemu_deliver_packet_iov(NetClientState *sender,
> -                                unsigned flags,
> -                                const struct iovec *iov,
> -                                int iovcnt,
> -                                void *opaque)
> +static ssize_t qemu_deliver_packet_iov(NetClientState *sender,
> +                                       unsigned flags,
> +                                       const struct iovec *iov,
> +                                       int iovcnt,
> +                                       void *opaque)
>  {
>      NetClientState *nc = opaque;
> -    size_t size = iov_size(iov, iovcnt);
>      int ret;
>  
> -    if (size > INT_MAX) {
> -        return size;
> -    }
>  
>      if (nc->link_down) {
> -        return size;
> +        return iov_size(iov, iovcnt);
>      }
>  
>      if (nc->receive_disabled) {
> @@ -745,10 +746,15 @@ ssize_t qemu_sendv_packet_async(NetClientState *sender,
>                                  NetPacketSent *sent_cb)
>  {
>      NetQueue *queue;
> +    size_t size = iov_size(iov, iovcnt);
>      int ret;
>  
> +    if (size > NET_BUFSIZE) {
> +        return size;
> +    }
> +
>      if (sender->link_down || !sender->peer) {
> -        return iov_size(iov, iovcnt);
> +        return size;
>      }
>  
>      /* Let filters handle the packet first */
> 

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

Re: [Qemu-devel] [PATCH V5 for 3.1 2/5] net: hub: suppress warnings of no host network for qtest

[Thomas Huth]

On 2018-12-04 04:53, Jason Wang wrote:
> If we want to qtest through hub, it would be much more simpler and
> safer to configure the hub without host network. So silent this
> warnings for qtest.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  net/hub.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/hub.c b/net/hub.c
> index 78b671ed95..5795a678ed 100644
> --- a/net/hub.c
> +++ b/net/hub.c
> @@ -20,6 +20,7 @@
>  #include "hub.h"
>  #include "qemu/iov.h"
>  #include "qemu/error-report.h"
> +#include "sysemu/qtest.h"
>  
>  /*
>   * A hub broadcasts incoming packets to all its ports except the source port.
> @@ -346,7 +347,7 @@ void net_hub_check_clients(void)
>          if (has_host_dev && !has_nic) {
>              warn_report("hub %d with no nics", hub->id);
>          }
> -        if (has_nic && !has_host_dev) {
> +        if (has_nic && !has_host_dev && !qtest_enabled()) {
>              warn_report("hub %d is not connected to host network", hub->id);
>          }
>      }

Sounds fine to me.

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

Re: [Qemu-devel] [PATCH V5 for 3.1 5/5] virtio-net-test: add large tx buffer test

[Thomas Huth]

On 2018-12-04 04:53, Jason Wang wrote:
> This test tries to build a packet whose size is greater than INT_MAX
> which tries to trigger integer overflow in qemu_net_queue_append_iov()
> which may result OOB.
> 
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  tests/virtio-net-test.c | 46 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c
> index bdd6af9999..e9783e6707 100644
> --- a/tests/virtio-net-test.c
> +++ b/tests/virtio-net-test.c
> @@ -245,6 +245,48 @@ static void pci_basic(gconstpointer data)
>      g_free(dev);
>      qtest_shutdown(qs);
>  }
> +
> +static void large_tx(gconstpointer data)
> +{
> +    QVirtioPCIDevice *dev;
> +    QOSState *qs;
> +    QVirtQueuePCI *tx, *rx;
> +    QVirtQueue *vq;
> +    uint64_t req_addr;
> +    uint32_t free_head;
> +    size_t alloc_size = (size_t)data / 64;
> +    int i;
> +
> +    qs = pci_test_start("-netdev hubport,id=hp0,hubid=0 "
> +                        "-device virtio-net-pci,netdev=hp0");
> +    dev = virtio_net_pci_init(qs->pcibus, PCI_SLOT);
> +
> +    rx = (QVirtQueuePCI *)qvirtqueue_setup(&dev->vdev, qs->alloc, 0);
> +    tx = (QVirtQueuePCI *)qvirtqueue_setup(&dev->vdev, qs->alloc, 1);
> +
> +    driver_init(&dev->vdev);
> +    vq = &tx->vq;
> +
> +    /* Bypass the limitation by pointing several descriptors to a single
> +     * smaller area */
> +    req_addr = guest_alloc(qs->alloc, alloc_size);
> +    free_head = qvirtqueue_add(vq, req_addr, alloc_size, false, true);
> +
> +    for (i = 0; i < 64; i++) {
> +        qvirtqueue_add(vq, req_addr, alloc_size, false, i != 63);
> +    }
> +    qvirtqueue_kick(&dev->vdev, vq, free_head);
> +
> +    qvirtio_wait_used_elem(&dev->vdev, vq, free_head, NULL,
> +                           QVIRTIO_NET_TIMEOUT_US);
> +
> +    qvirtqueue_cleanup(dev->vdev.bus, &tx->vq, qs->alloc);
> +    qvirtqueue_cleanup(dev->vdev.bus, &rx->vq, qs->alloc);
> +    qvirtio_pci_device_disable(dev);
> +    g_free(dev->pdev);
> +    g_free(dev);
> +    qtest_shutdown(qs);
> +}
>  #endif
>  
>  static void hotplug(void)
> @@ -270,6 +312,10 @@ int main(int argc, char **argv)
>      qtest_add_data_func("/virtio/net/pci/basic", send_recv_test, pci_basic);
>      qtest_add_data_func("/virtio/net/pci/rx_stop_cont",
>                          stop_cont_test, pci_basic);
> +    qtest_add_data_func("/virtio/net/pci/large_tx_uint_max",
> +                        (gconstpointer)UINT_MAX, large_tx);
> +    qtest_add_data_func("/virtio/net/pci/large_tx_net_bufsize",
> +                        (gconstpointer)NET_BUFSIZE, large_tx);
>  #endif
>      qtest_add_func("/virtio/net/pci/hotplug", hotplug);
>  
> 

Looks reasonable to me. In case you respin again, it might maybe be a
good idea to set the RAM size of the guest ("-m 256" or something
similar), but AFAIK all off the guests where we run this test have
enough memory by default, so this should not be an issue.

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

Re: [Qemu-devel] [PATCH V5 for 3.1 0/5] Fix possible OOB during queuing packets

[Peter Maydell]

On Tue, 4 Dec 2018 at 03:54, Jason Wang <jasowang@redhat.com> wrote:
>
> Hi:
>
> This series tries to fix a possible OOB during queueing packets
> through qemu_net_queue_append_iov(). This could happen when it tries
> to queue a packet whose size is larger than INT_MAX which may lead
> integer overflow. We've fixed similar issue in the past during
> qemu_net_queue_deliver_iov() by ignoring large packets there. Let's
> just move the check earlier to qemu_sendv_packet_async() and reduce
> the limitation to NET_BUFSIZE. A simple qtest were also added this.
>
> Please review.
>
> Thanks
>
> Changes from V1:
> - slient compiling warnings
> Changes from V2:
> - annotate pci_test_start() with GCC_FMT_ATTR()
> - drop intermediate cmd string variable
> Changes from V4:
> - silent hub warning if qtest is enabled
> - make qemu_deliver_packet_iov() static
> - add one more test for packet size slightly greater than NET_BUFSIZE
> - tweak the commit log and add more justification
> - typo/whitespace fixes and other minor code style tweaks
>
> Jason Wang (5):
>   net: drop too large packet early
>   net: hub: suppress warnings of no host network for qtest
>   virtio-net-test: accept variable length argument in pci_test_start()
>   virtio-net-test: remove unused macro
>   virtio-net-test: add large tx buffer test

Applied, thanks.

-- PMM