hw/net/vmxnet3: Do not abort QEMU if guest specified bad queue numbers

[PATCH] hw/net/vmxnet3: Do not abort QEMU if guest specified bad queue numbers

Posted by Thomas Huth 4 years, 6 months ago

QEMU should never terminate unexpectedly just because the guest is
doing something wrong like specifying wrong queue numbers. Let's
simply refuse to set the device active in this case.

Buglink: https://bugs.launchpad.net/qemu/+bug/1890160
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 hw/net/vmxnet3.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index f6bd8c53b1..41f796a247 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -1381,7 +1381,7 @@ static void vmxnet3_validate_interrupts(VMXNET3State *s)
     }
 }
 
-static void vmxnet3_validate_queues(VMXNET3State *s)
+static bool vmxnet3_validate_queues(VMXNET3State *s)
 {
     /*
     * txq_num and rxq_num are total number of queues
@@ -1390,12 +1390,18 @@ 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);
+        qemu_log_mask(LOG_GUEST_ERROR, "vmxnet3: Bad TX queues number: %d\n",
+                      s->txq_num);
+        return false;
     }
 
     if (s->rxq_num > VMXNET3_DEVICE_MAX_RX_QUEUES) {
-        hw_error("Bad RX queues number: %d\n", s->rxq_num);
+        qemu_log_mask(LOG_GUEST_ERROR, "vmxnet3: Bad RX queues number: %d\n",
+                      s->rxq_num);
+        return false;
     }
+
+    return true;
 }
 
 static void vmxnet3_activate_device(VMXNET3State *s)
@@ -1419,6 +1425,16 @@ static void vmxnet3_activate_device(VMXNET3State *s)
         return;
     }
 
+    s->txq_num =
+        VMXNET3_READ_DRV_SHARED8(d, s->drv_shmem, devRead.misc.numTxQueues);
+    s->rxq_num =
+        VMXNET3_READ_DRV_SHARED8(d, s->drv_shmem, devRead.misc.numRxQueues);
+
+    VMW_CFPRN("Number of TX/RX queues %u/%u", s->txq_num, s->rxq_num);
+    if (!vmxnet3_validate_queues(s)) {
+        return;
+    }
+
     vmxnet3_adjust_by_guest_type(s);
     vmxnet3_update_features(s);
     vmxnet3_update_pm_state(s);
@@ -1445,14 +1461,6 @@ static void vmxnet3_activate_device(VMXNET3State *s)
         VMXNET3_READ_DRV_SHARED8(d, s->drv_shmem, devRead.intrConf.autoMask);
     VMW_CFPRN("Automatic interrupt masking is %d", (int)s->auto_int_masking);
 
-    s->txq_num =
-        VMXNET3_READ_DRV_SHARED8(d, s->drv_shmem, devRead.misc.numTxQueues);
-    s->rxq_num =
-        VMXNET3_READ_DRV_SHARED8(d, s->drv_shmem, devRead.misc.numRxQueues);
-
-    VMW_CFPRN("Number of TX/RX queues %u/%u", s->txq_num, s->rxq_num);
-    vmxnet3_validate_queues(s);
-
     qdescr_table_pa =
         VMXNET3_READ_DRV_SHARED64(d, s->drv_shmem, devRead.misc.queueDescPA);
     VMW_CFPRN("TX queues descriptors table is at 0x%" PRIx64, qdescr_table_pa);
@@ -2404,7 +2412,9 @@ static int vmxnet3_post_load(void *opaque, int version_id)
         }
     }
 
-    vmxnet3_validate_queues(s);
+    if (!vmxnet3_validate_queues(s)) {
+        return -1;
+    }
     vmxnet3_validate_interrupts(s);
 
     return 0;
-- 
2.27.0

Re: [PATCH] hw/net/vmxnet3: Do not abort QEMU if guest specified bad queue numbers

Posted by Jason Wang 4 years, 6 months ago

在 2021/7/21 下午10:15, Thomas Huth 写道:
> QEMU should never terminate unexpectedly just because the guest is
> doing something wrong like specifying wrong queue numbers. Let's
> simply refuse to set the device active in this case.
>
> Buglink: https://bugs.launchpad.net/qemu/+bug/1890160
> Signed-off-by: Thomas Huth <thuth@redhat.com>


Applied.

Thanks


> ---
>   hw/net/vmxnet3.c | 34 ++++++++++++++++++++++------------
>   1 file changed, 22 insertions(+), 12 deletions(-)
>
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index f6bd8c53b1..41f796a247 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -1381,7 +1381,7 @@ static void vmxnet3_validate_interrupts(VMXNET3State *s)
>       }
>   }
>   
> -static void vmxnet3_validate_queues(VMXNET3State *s)
> +static bool vmxnet3_validate_queues(VMXNET3State *s)
>   {
>       /*
>       * txq_num and rxq_num are total number of queues
> @@ -1390,12 +1390,18 @@ 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);
> +        qemu_log_mask(LOG_GUEST_ERROR, "vmxnet3: Bad TX queues number: %d\n",
> +                      s->txq_num);
> +        return false;
>       }
>   
>       if (s->rxq_num > VMXNET3_DEVICE_MAX_RX_QUEUES) {
> -        hw_error("Bad RX queues number: %d\n", s->rxq_num);
> +        qemu_log_mask(LOG_GUEST_ERROR, "vmxnet3: Bad RX queues number: %d\n",
> +                      s->rxq_num);
> +        return false;
>       }
> +
> +    return true;
>   }
>   
>   static void vmxnet3_activate_device(VMXNET3State *s)
> @@ -1419,6 +1425,16 @@ static void vmxnet3_activate_device(VMXNET3State *s)
>           return;
>       }
>   
> +    s->txq_num =
> +        VMXNET3_READ_DRV_SHARED8(d, s->drv_shmem, devRead.misc.numTxQueues);
> +    s->rxq_num =
> +        VMXNET3_READ_DRV_SHARED8(d, s->drv_shmem, devRead.misc.numRxQueues);
> +
> +    VMW_CFPRN("Number of TX/RX queues %u/%u", s->txq_num, s->rxq_num);
> +    if (!vmxnet3_validate_queues(s)) {
> +        return;
> +    }
> +
>       vmxnet3_adjust_by_guest_type(s);
>       vmxnet3_update_features(s);
>       vmxnet3_update_pm_state(s);
> @@ -1445,14 +1461,6 @@ static void vmxnet3_activate_device(VMXNET3State *s)
>           VMXNET3_READ_DRV_SHARED8(d, s->drv_shmem, devRead.intrConf.autoMask);
>       VMW_CFPRN("Automatic interrupt masking is %d", (int)s->auto_int_masking);
>   
> -    s->txq_num =
> -        VMXNET3_READ_DRV_SHARED8(d, s->drv_shmem, devRead.misc.numTxQueues);
> -    s->rxq_num =
> -        VMXNET3_READ_DRV_SHARED8(d, s->drv_shmem, devRead.misc.numRxQueues);
> -
> -    VMW_CFPRN("Number of TX/RX queues %u/%u", s->txq_num, s->rxq_num);
> -    vmxnet3_validate_queues(s);
> -
>       qdescr_table_pa =
>           VMXNET3_READ_DRV_SHARED64(d, s->drv_shmem, devRead.misc.queueDescPA);
>       VMW_CFPRN("TX queues descriptors table is at 0x%" PRIx64, qdescr_table_pa);
> @@ -2404,7 +2412,9 @@ static int vmxnet3_post_load(void *opaque, int version_id)
>           }
>       }
>   
> -    vmxnet3_validate_queues(s);
> +    if (!vmxnet3_validate_queues(s)) {
> +        return -1;
> +    }
>       vmxnet3_validate_interrupts(s);
>   
>       return 0;