From 3f33dce627f419cbc77a9f6406b3353077697740 Mon Sep 17 00:00:00 2001

From: Xiao Wang <jasowang@redhat.com>

Date: Fri, 11 Jan 2019 07:59:00 +0000

Subject: [PATCH 05/11] net: drop too large packet early

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

RH-Author: Xiao Wang <jasowang@redhat.com>

Message-id: <20190111075904.2030-6-jasowang@redhat.com>

Patchwork-id: 83979

O-Subject: [RHEL8 qemu-kvm PATCH 5/9] net: drop too large packet early

Bugzilla: 1636784

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

RH-Acked-by: Philippe Mathieu-Daudé <philmd@redhat.com>

RH-Acked-by: Jens Freimann <jfreimann@redhat.com>

RH-Acked-by: Maxime Coquelin <maxime.coquelin@redhat.com>

RH-Acked-by: Michael S. Tsirkin <mst@redhat.com>

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>

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

Message-id: 20181204035347.6148-2-jasowang@redhat.com

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

(cherry picked from commit 25c01bd19d0e4b66f357618aeefda1ef7a41e21a)

Signed-off-by: Danilo C. L. de Paula <ddepaula@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 1f7341e..df4df25 100644

--- a/include/net/net.h

+++ b/include/net/net.h

@@ -170,12 +170,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 c991243..6e5c335 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 */

-- 

1.8.3.1