Re: [Qemu-devel] [PATCH v4 29/30] vmxnet3: validate queues configuration coming from quest

[Dmitry Fleytman]

On Mar 31, 2014, at 18:48 PM, Dr. David Alan Gilbert <address@hidden> wrote:

* Michael S. Tsirkin (address@hidden) wrote:

From: Dmitry Fleytman <address@hidden>

CVE-2013-4544

Signed-off-by: Dmitry Fleytman <address@hidden>
Reported-by: Michael S. Tsirkin <address@hidden>
Signed-off-by: Michael S. Tsirkin <address@hidden>
---
hw/net/vmxnet3.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 8c6df05..0fa54e7 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -1336,6 +1336,17 @@ static void vmxnet3_validate_interrupts(VMXNET3State *s)
    }
}

+static void vmxnet3_validate_queues(VMXNET3State *s)
+{
+    if (s->txq_num > VMXNET3_DEVICE_MAX_TX_QUEUES) {
+        hw_error("Bad TX queues number: %d\n", s->txq_num);
+    }
+
+    if (s->rxq_num > VMXNET3_DEVICE_MAX_RX_QUEUES) {
+        hw_error("Bad RX queues number: %d\n", s->rxq_num);
+    }

Why isn't that >= ?
(I agree it matches the original assert).

       Vmxnet3RxqDescr rxq_descr[VMXNET3_DEVICE_MAX_RX_QUEUES];
       Vmxnet3TxqDescr txq_descr[VMXNET3_DEVICE_MAX_TX_QUEUES];

static void vmxnet3_fill_stats(VMXNET3State *s)
{
   int i;
   for (i = 0; i < s->txq_num; i++) {
       cpu_physical_memory_write(s->txq_descr[i].tx_stats_pa,
                                 &s->txq_descr[i].txq_stats,
                                 sizeof(s->txq_descr[i].txq_stats));
   }

so that looks like it's 0 indexed.

Dave

HI Dave, thanks for the review.

The verification is ok because s->txq_num and s->rxq_num are total number of queues, not a queue index.

Dmitry.

+}
+
static void vmxnet3_activate_device(VMXNET3State *s)
{
    int i;
@@ -1382,7 +1393,7 @@ static void vmxnet3_activate_device(VMXNET3State *s)
        VMXNET3_READ_DRV_SHARED8(s->drv_shmem, devRead.misc.numRxQueues);

    VMW_CFPRN("Number of TX/RX queues %u/%u", s->txq_num, s->rxq_num);
-    assert(s->txq_num <= VMXNET3_DEVICE_MAX_TX_QUEUES);
+    vmxnet3_validate_queues(s);

    qdescr_table_pa =
        VMXNET3_READ_DRV_SHARED64(s->drv_shmem, devRead.misc.queueDescPA);
-- 
MST

--
Dr. David Alan Gilbert / address@hidden / Manchester, UK