Re: [Qemu-devel] net: vmxnet3: memory leakage issue

[P J P]

  Hello Jason,

+-- On Fri, 4 Dec 2015, Jason Wang wrote --+
| Better with "git send-email".

  Okay.

| What if guest deactivate the device before re-activate the device?
|Looks like it could be done through methods:
|
|1) VMXNET3_CMD_QUIESCE_DEV

  IIUC, it is used to pause the device when the receiver end is unable to 
keee-up with the incoming flow. After a brief period, the operation could be 
resumed again.

|2) VMXNET3_REG_DSAL

   Shared memory between a driver and the device appears to be set in two 
steps. Firs low address, followed by the high address(VMXNET3_REG_DSAH). I 
guess 's->device_active' needs to be enabled again while setting the higher 
part of the address.

|So looks like need to free both tx_pkt and rx_pkt during deactivating?

  I think freeing 'tx_pkt' & 'rx_pkt' during pause wouldn't be good. It needs 
to call something like - 'vmxnet3_resume_device'.

Please see the patch below for the above two cases, does it look okay?

===
diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
index 37373e5..4b2305b 100644
--- a/hw/net/vmxnet3.c
+++ b/hw/net/vmxnet3.c
@@ -1198,6 +1198,12 @@ static void vmxnet3_deactivate_device(VMXNET3State *s)
     s->device_active = false;
 }
 
+static void vmxnet3_resume_device(VMXNET3State *s)
+{
+    VMW_CBPRN("Resuming vmxnet3...");
+    s->device_active = true;
+}
+
 static void vmxnet3_reset(VMXNET3State *s)
 {
     VMW_CBPRN("Resetting vmxnet3...");
@@ -1627,8 +1633,13 @@ static void vmxnet3_handle_command(VMXNET3State *s, 
uint64_t cmd)
         break;
 
     case VMXNET3_CMD_QUIESCE_DEV:
-        VMW_CBPRN("Set: VMXNET3_CMD_QUIESCE_DEV - pause the device");
-        vmxnet3_deactivate_device(s);
+        if (s->device_active) {
+            VMW_CBPRN("Set: VMXNET3_CMD_QUIESCE_DEV - pause the device");
+            vmxnet3_deactivate_device(s);
+        } else {
+            VMW_CBPRN("Set: VMXNET3_CMD_QUIESCE_DEV - resume the device");
+            vmxnet3_resume_device(s);
+        }
         break;
 
     case VMXNET3_CMD_GET_CONF_INTR:
@@ -1756,6 +1767,9 @@ vmxnet3_io_bar1_write(void *opaque,
          * We already should have low address part.
          */
         s->drv_shmem = s->temp_shared_guest_driver_memory | (val << 32);
+        if (s->drv_shmem) {
+            s->device_active = true;
+        }
         break;
 
     /* Command */
===


Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F