[PATCH] target/i386/hax-posix: fix two 'format-truncation' compile warnings

[pannengyuan]

From: Pan Nengyuan 

Fix compile warnings:
/mnt/sdb/qemu-new/qemu_test/qemu/target/i386/hax-posix.c:124:56: error: ‘%02d’ 
directive output may be truncated writing between 2 and 11 bytes into a region 
of size 3 [-Werror=format-truncation=]
 snprintf(name, sizeof HAX_VM_DEVFS, "/dev/hax_vm/vm%02d", vm_id);
^~~~
/mnt/sdb/qemu-new/qemu_test/qemu/target/i386/hax-posix.c:124:41: note: 
directive argument in the range [-2147483648, 64]
 snprintf(name, sizeof HAX_VM_DEVFS, "/dev/hax_vm/vm%02d", vm_id);
 ^~~~
In file included from /usr/include/stdio.h:873,
 from /mnt/sdb/qemu-new/qemu_test/qemu/include/qemu/osdep.h:99,
 from 
/mnt/sdb/qemu-new/qemu_test/qemu/target/i386/hax-posix.c:14:
/usr/include/bits/stdio2.h:67:10: note: ‘__builtin___snprintf_chk’ output 
between 17 and 26 bytes into a destination of size 17
   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
  ^~~~
__bos (__s), __fmt, __va_arg_pack ());
~
/mnt/sdb/qemu-new/qemu_test/qemu/target/i386/hax-posix.c: In function 
‘hax_vcpu_devfs_string’:
/mnt/sdb/qemu-new/qemu_test/qemu/target/i386/hax-posix.c:143:55: error: ‘%02d’ 
directive output may be truncated writing between 2 and 11 bytes into a region 
of size 10 [-Werror=format-truncation=]
 snprintf(name, sizeof HAX_VCPU_DEVFS, "/dev/hax_vm%02d/vcpu%02d",
   ^~~~
/mnt/sdb/qemu-new/qemu_test/qemu/target/i386/hax-posix.c:143:43: note: 
directive argument in the range [-2147483648, 64]
 snprintf(name, sizeof HAX_VCPU_DEVFS, "/dev/hax_vm%02d/vcpu%02d",
   ^~
/mnt/sdb/qemu-new/qemu_test/qemu/target/i386/hax-posix.c:143:43: note: 
directive argument in the range [-2147483648, 64]
In file included from /usr/include/stdio.h:873,
 from /mnt/sdb/qemu-new/qemu_test/qemu/include/qemu/osdep.h:99,
 from 
/mnt/sdb/qemu-new/qemu_test/qemu/target/i386/hax-posix.c:14:
/usr/include/bits/stdio2.h:67:10: note: ‘__builtin___snprintf_chk’ output 
between 21 and 39 bytes into a destination of size 21
   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
  ^~~~
__bos (__s), __fmt, __va_arg_pack ());

We know that we have checked the vm_id and vcpu_id in the first(less than 
0x40), it will never be truncated in snprintf().
Thus, this patch add an assertion to clear this false-positive warning.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 target/i386/hax-posix.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/target/i386/hax-posix.c b/target/i386/hax-posix.c
index a5426a6dac..197d5bc0f9 100644
--- a/target/i386/hax-posix.c
+++ b/target/i386/hax-posix.c
@@ -121,7 +121,8 @@ static char *hax_vm_devfs_string(int vm_id)
 return NULL;
 }
 
-snprintf(name, sizeof HAX_VM_DEVFS, "/dev/hax_vm/vm%02d", vm_id);
+int len = snprintf(name, sizeof HAX_VM_DEVFS, "/dev/hax_vm/vm%02d", vm_id);
+assert(len < sizeof HAX_VM_DEVFS);
 return name;
 }
 
@@ -140,8 +141,9 @@ static char *hax_vcpu_devfs_string(int vm_id, int vcpu_id)
 return NULL;
 }
 
-snprintf(name, sizeof HAX_VCPU_DEVFS, "/dev/hax_vm%02d/vcpu%02d",
- vm_id, vcpu_id);
+int len = snprintf(name, sizeof HAX_VCPU_DEVFS, "/dev/hax_vm%02d/vcpu%02d",
+   vm_id, vcpu_id);
+assert(len < sizeof HAX_VCPU_DEVFS);
 return name;
 }
 
-- 
2.18.2

[PATCH v2 0/2] delete virtio queues in vhost-user-blk-unrealize

[pannengyuan]

From: Pan Nengyuan 

This series patch fix memleaks when detaching vhost-user-blk device.
1. use old virtio_del_queue to fix memleaks, it's easier for stable branches to 
merge.
   As the discussion in 
https://lists.nongnu.org/archive/html/qemu-devel/2020-01/msg02903.html

2. convert virtio_del_queue to the new one(virtio_delete_queue).

v2->v1: rename vqs to vhost_vqs to avoid confusing with virtqs (suggented by 
Stefan Hajnoczi)

Pan Nengyuan (2):
  vhost-user-blk: delete virtioqueues in unrealize to fix memleaks
  vhost-use-blk: convert to new virtio_delete_queue

 hw/block/vhost-user-blk.c  | 23 +--
 include/hw/virtio/vhost-user-blk.h |  3 ++-
 2 files changed, 19 insertions(+), 7 deletions(-)

-- 
2.18.2

[PATCH v2 2/2] vhost-use-blk: convert to new virtio_delete_queue

[pannengyuan]

From: Pan Nengyuan 

use the new virtio_delete_queue function to cleanup.

Signed-off-by: Pan Nengyuan 
---
V2->V1:
- rename vqs to vhost_vqs to avoid confusing with virtqs (suggented by Stefan 
Hajnoczi)
---
 hw/block/vhost-user-blk.c  | 19 +++
 include/hw/virtio/vhost-user-blk.h |  3 ++-
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 2eba8b9db0..12925a47ec 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -306,7 +306,7 @@ static int vhost_user_blk_connect(DeviceState *dev)
 s->connected = true;
 
 s->dev.nvqs = s->num_queues;
-s->dev.vqs = s->vqs;
+s->dev.vqs = s->vhost_vqs;
 s->dev.vq_index = 0;
 s->dev.backend_features = 0;
 
@@ -420,13 +420,14 @@ static void vhost_user_blk_device_realize(DeviceState 
*dev, Error **errp)
 virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
 sizeof(struct virtio_blk_config));
 
+s->virtqs = g_new(VirtQueue *, s->num_queues);
 for (i = 0; i < s->num_queues; i++) {
-virtio_add_queue(vdev, s->queue_size,
- vhost_user_blk_handle_output);
+s->virtqs[i] = virtio_add_queue(vdev, s->queue_size,
+vhost_user_blk_handle_output);
 }
 
 s->inflight = g_new0(struct vhost_inflight, 1);
-s->vqs = g_new0(struct vhost_virtqueue, s->num_queues);
+s->vhost_vqs = g_new0(struct vhost_virtqueue, s->num_queues);
 s->watch = 0;
 s->connected = false;
 
@@ -458,11 +459,12 @@ reconnect:
 return;
 
 virtio_err:
-g_free(s->vqs);
+g_free(s->vhost_vqs);
 g_free(s->inflight);
 for (i = 0; i < s->num_queues; i++) {
-virtio_del_queue(vdev, i);
+virtio_delete_queue(s->virtqs[i]);
 }
+g_free(s->virtqs);
 virtio_cleanup(vdev);
 vhost_user_cleanup(>vhost_user);
 }
@@ -478,12 +480,13 @@ static void vhost_user_blk_device_unrealize(DeviceState 
*dev, Error **errp)
  NULL, NULL, NULL, false);
 vhost_dev_cleanup(>dev);
 vhost_dev_free_inflight(s->inflight);
-g_free(s->vqs);
+g_free(s->vhost_vqs);
 g_free(s->inflight);
 
 for (i = 0; i < s->num_queues; i++) {
-virtio_del_queue(vdev, i);
+virtio_delete_queue(s->virtqs[i]);
 }
+g_free(s->virtqs);
 virtio_cleanup(vdev);
 vhost_user_cleanup(>vhost_user);
 }
diff --git a/include/hw/virtio/vhost-user-blk.h 
b/include/hw/virtio/vhost-user-blk.h
index 108bfadeeb..05ea0ad183 100644
--- a/include/hw/virtio/vhost-user-blk.h
+++ b/include/hw/virtio/vhost-user-blk.h
@@ -36,7 +36,8 @@ typedef struct VHostUserBlk {
 struct vhost_dev dev;
 struct vhost_inflight *inflight;
 VhostUserState vhost_user;
-struct vhost_virtqueue *vqs;
+struct vhost_virtqueue *vhost_vqs;
+VirtQueue **virtqs;
 guint watch;
 bool connected;
 } VHostUserBlk;
-- 
2.18.2

[PATCH v2 1/2] vhost-user-blk: delete virtioqueues in unrealize to fix memleaks

[pannengyuan]

From: Pan Nengyuan 

virtio queues forgot to delete in unrealize, and aslo error path in
realize, this patch fix these memleaks, the leak stack is as follow:

Direct leak of 114688 byte(s) in 16 object(s) allocated from:
#0 0x7f24024fdbf0 in calloc (/lib64/libasan.so.3+0xcabf0)
#1 0x7f2401642015 in g_malloc0 (/lib64/libglib-2.0.so.0+0x50015)
#2 0x55ad175a6447 in virtio_add_queue /mnt/sdb/qemu/hw/virtio/virtio.c:2327
#3 0x55ad17570cf9 in vhost_user_blk_device_realize 
/mnt/sdb/qemu/hw/block/vhost-user-blk.c:419
#4 0x55ad175a3707 in virtio_device_realize 
/mnt/sdb/qemu/hw/virtio/virtio.c:3509
#5 0x55ad176ad0d1 in device_set_realized /mnt/sdb/qemu/hw/core/qdev.c:876
#6 0x55ad1781ff9d in property_set_bool /mnt/sdb/qemu/qom/object.c:2080
#7 0x55ad178245ae in object_property_set_qobject 
/mnt/sdb/qemu/qom/qom-qobject.c:26
#8 0x55ad17821eb4 in object_property_set_bool 
/mnt/sdb/qemu/qom/object.c:1338
#9 0x55ad177aeed7 in virtio_pci_realize 
/mnt/sdb/qemu/hw/virtio/virtio-pci.c:1801

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Reviewed-by: Stefan Hajnoczi 
---
v2->v1: There is no change in this patch(only change the patch2/2)
---
 hw/block/vhost-user-blk.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index d8c459c575..2eba8b9db0 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -460,6 +460,9 @@ reconnect:
 virtio_err:
 g_free(s->vqs);
 g_free(s->inflight);
+for (i = 0; i < s->num_queues; i++) {
+virtio_del_queue(vdev, i);
+}
 virtio_cleanup(vdev);
 vhost_user_cleanup(>vhost_user);
 }
@@ -468,6 +471,7 @@ static void vhost_user_blk_device_unrealize(DeviceState 
*dev, Error **errp)
 {
 VirtIODevice *vdev = VIRTIO_DEVICE(dev);
 VHostUserBlk *s = VHOST_USER_BLK(dev);
+int i;
 
 virtio_set_status(vdev, 0);
 qemu_chr_fe_set_handlers(>chardev,  NULL, NULL, NULL,
@@ -476,6 +480,10 @@ static void vhost_user_blk_device_unrealize(DeviceState 
*dev, Error **errp)
 vhost_dev_free_inflight(s->inflight);
 g_free(s->vqs);
 g_free(s->inflight);
+
+for (i = 0; i < s->num_queues; i++) {
+virtio_del_queue(vdev, i);
+}
 virtio_cleanup(vdev);
 vhost_user_cleanup(>vhost_user);
 }
-- 
2.18.2

[PATCH] migration/savevm: release gslist after dump_vmstate_json

[pannengyuan]

From: Pan Nengyuan 

'list' forgot to free at the end of dump_vmstate_json_to_file(), although it's 
called only once, but seems like a clean code.

Fix the leak as follow:
Direct leak of 16 byte(s) in 1 object(s) allocated from:
#0 0x7fb946abd768 in __interceptor_malloc (/lib64/libasan.so.5+0xef768)
#1 0x7fb945eca445 in g_malloc (/lib64/libglib-2.0.so.0+0x52445)
#2 0x7fb945ee2066 in g_slice_alloc (/lib64/libglib-2.0.so.0+0x6a066)
#3 0x7fb945ee3139 in g_slist_prepend (/lib64/libglib-2.0.so.0+0x6b139)
#4 0x5585db591581 in object_class_get_list_tramp 
/mnt/sdb/qemu-new/qemu/qom/object.c:1084
#5 0x5585db590f66 in object_class_foreach_tramp 
/mnt/sdb/qemu-new/qemu/qom/object.c:1028
#6 0x7fb945eb35f7 in g_hash_table_foreach (/lib64/libglib-2.0.so.0+0x3b5f7)
#7 0x5585db59110c in object_class_foreach 
/mnt/sdb/qemu-new/qemu/qom/object.c:1038
#8 0x5585db5916b6 in object_class_get_list 
/mnt/sdb/qemu-new/qemu/qom/object.c:1092
#9 0x5585db335ca0 in dump_vmstate_json_to_file 
/mnt/sdb/qemu-new/qemu/migration/savevm.c:638
#10 0x5585daa5bcbf in main /mnt/sdb/qemu-new/qemu/vl.c:4420
#11 0x7fb941204812 in __libc_start_main ../csu/libc-start.c:308
#12 0x5585da29420d in _start 
(/mnt/sdb/qemu-new/qemu/build/x86_64-softmmu/qemu-system-x86_64+0x27f020d)

Indirect leak of 7472 byte(s) in 467 object(s) allocated from:
#0 0x7fb946abd768 in __interceptor_malloc (/lib64/libasan.so.5+0xef768)
#1 0x7fb945eca445 in g_malloc (/lib64/libglib-2.0.so.0+0x52445)
#2 0x7fb945ee2066 in g_slice_alloc (/lib64/libglib-2.0.so.0+0x6a066)
#3 0x7fb945ee3139 in g_slist_prepend (/lib64/libglib-2.0.so.0+0x6b139)
#4 0x5585db591581 in object_class_get_list_tramp 
/mnt/sdb/qemu-new/qemu/qom/object.c:1084
#5 0x5585db590f66 in object_class_foreach_tramp 
/mnt/sdb/qemu-new/qemu/qom/object.c:1028
#6 0x7fb945eb35f7 in g_hash_table_foreach (/lib64/libglib-2.0.so.0+0x3b5f7)
#7 0x5585db59110c in object_class_foreach 
/mnt/sdb/qemu-new/qemu/qom/object.c:1038
#8 0x5585db5916b6 in object_class_get_list 
/mnt/sdb/qemu-new/qemu/qom/object.c:1092
#9 0x5585db335ca0 in dump_vmstate_json_to_file 
/mnt/sdb/qemu-new/qemu/migration/savevm.c:638
#10 0x5585daa5bcbf in main /mnt/sdb/qemu-new/qemu/vl.c:4420
#11 0x7fb941204812 in __libc_start_main ../csu/libc-start.c:308
#12 0x5585da29420d in _start 
(/mnt/sdb/qemu-new/qemu/build/x86_64-softmmu/qemu-system-x86_64+0x27f020d)

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 migration/savevm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/migration/savevm.c b/migration/savevm.c
index f19cb9ec7a..60e6ea8a8d 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -665,6 +665,7 @@ void dump_vmstate_json_to_file(FILE *out_file)
 }
 fprintf(out_file, "\n}\n");
 fclose(out_file);
+g_slist_free(list);
 }
 
 static uint32_t calculate_new_instance_id(const char *idstr)
-- 
2.18.2

[PATCH v2 2/2] hw: move timer_new from init() into realize() to avoid memleaks

[pannengyuan]

From: Pan Nengyuan 

There are some memleaks when we call 'device_list_properties'. This patch move 
timer_new from init into realize to fix it.
Meanwhile, do the null check in mos6522_reset() to avoid null deref if we move 
timer_new into realize().

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Reviewed-by: Philippe Mathieu-Daudé 
---
Changes v2 to v1:
- Send this patch in a series instead of a single patch but with wrong subject 
in v1.
---
 hw/arm/pxa2xx.c| 17 +++--
 hw/arm/spitz.c |  8 +++-
 hw/arm/strongarm.c | 18 --
 hw/misc/mos6522.c  | 14 --
 hw/timer/cadence_ttc.c | 16 +++-
 5 files changed, 53 insertions(+), 20 deletions(-)

diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
index b33f8f1351..56a36202d7 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -1134,18 +1134,22 @@ static void pxa2xx_rtc_init(Object *obj)
 s->last_rtcpicr = 0;
 s->last_hz = s->last_sw = s->last_pi = qemu_clock_get_ms(rtc_clock);
 
+sysbus_init_irq(dev, >rtc_irq);
+
+memory_region_init_io(>iomem, obj, _rtc_ops, s,
+  "pxa2xx-rtc", 0x1);
+sysbus_init_mmio(dev, >iomem);
+}
+
+static void pxa2xx_rtc_realize(DeviceState *dev, Error **errp)
+{
+PXA2xxRTCState *s = PXA2XX_RTC(dev);
 s->rtc_hz= timer_new_ms(rtc_clock, pxa2xx_rtc_hz_tick,s);
 s->rtc_rdal1 = timer_new_ms(rtc_clock, pxa2xx_rtc_rdal1_tick, s);
 s->rtc_rdal2 = timer_new_ms(rtc_clock, pxa2xx_rtc_rdal2_tick, s);
 s->rtc_swal1 = timer_new_ms(rtc_clock, pxa2xx_rtc_swal1_tick, s);
 s->rtc_swal2 = timer_new_ms(rtc_clock, pxa2xx_rtc_swal2_tick, s);
 s->rtc_pi= timer_new_ms(rtc_clock, pxa2xx_rtc_pi_tick,s);
-
-sysbus_init_irq(dev, >rtc_irq);
-
-memory_region_init_io(>iomem, obj, _rtc_ops, s,
-  "pxa2xx-rtc", 0x1);
-sysbus_init_mmio(dev, >iomem);
 }
 
 static int pxa2xx_rtc_pre_save(void *opaque)
@@ -1203,6 +1207,7 @@ static void pxa2xx_rtc_sysbus_class_init(ObjectClass 
*klass, void *data)
 
 dc->desc = "PXA2xx RTC Controller";
 dc->vmsd = _pxa2xx_rtc_regs;
+dc->realize = pxa2xx_rtc_realize;
 }
 
 static const TypeInfo pxa2xx_rtc_sysbus_info = {
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index e001088103..cbfa6934cf 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -524,11 +524,16 @@ static void spitz_keyboard_init(Object *obj)
 
 spitz_keyboard_pre_map(s);
 
-s->kbdtimer = timer_new_ns(QEMU_CLOCK_VIRTUAL, spitz_keyboard_tick, s);
 qdev_init_gpio_in(dev, spitz_keyboard_strobe, SPITZ_KEY_STROBE_NUM);
 qdev_init_gpio_out(dev, s->sense, SPITZ_KEY_SENSE_NUM);
 }
 
+static void spitz_keyboard_realize(DeviceState *dev, Error **errp)
+{
+SpitzKeyboardState *s = SPITZ_KEYBOARD(dev);
+s->kbdtimer = timer_new_ns(QEMU_CLOCK_VIRTUAL, spitz_keyboard_tick, s);
+}
+
 /* LCD backlight controller */
 
 #define LCDTG_RESCTL   0x00
@@ -1115,6 +1120,7 @@ static void spitz_keyboard_class_init(ObjectClass *klass, 
void *data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 
 dc->vmsd = _spitz_kbd;
+dc->realize = spitz_keyboard_realize;
 }
 
 static const TypeInfo spitz_keyboard_info = {
diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
index cd8a99aaf2..3010d765bb 100644
--- a/hw/arm/strongarm.c
+++ b/hw/arm/strongarm.c
@@ -399,9 +399,6 @@ static void strongarm_rtc_init(Object *obj)
 s->last_rcnr = (uint32_t) mktimegm();
 s->last_hz = qemu_clock_get_ms(rtc_clock);
 
-s->rtc_alarm = timer_new_ms(rtc_clock, strongarm_rtc_alarm_tick, s);
-s->rtc_hz = timer_new_ms(rtc_clock, strongarm_rtc_hz_tick, s);
-
 sysbus_init_irq(dev, >rtc_irq);
 sysbus_init_irq(dev, >rtc_hz_irq);
 
@@ -410,6 +407,13 @@ static void strongarm_rtc_init(Object *obj)
 sysbus_init_mmio(dev, >iomem);
 }
 
+static void strongarm_rtc_realize(DeviceState *dev, Error **errp)
+{
+StrongARMRTCState *s = STRONGARM_RTC(dev);
+s->rtc_alarm = timer_new_ms(rtc_clock, strongarm_rtc_alarm_tick, s);
+s->rtc_hz = timer_new_ms(rtc_clock, strongarm_rtc_hz_tick, s);
+}
+
 static int strongarm_rtc_pre_save(void *opaque)
 {
 StrongARMRTCState *s = opaque;
@@ -451,6 +455,7 @@ static void strongarm_rtc_sysbus_class_init(ObjectClass 
*klass, void *data)
 
 dc->desc = "StrongARM RTC Controller";
 dc->vmsd = _strongarm_rtc_regs;
+dc->realize = strongarm_rtc_realize;
 }
 
 static const TypeInfo strongarm_rtc_sysbus_info = {
@@ -1240,15 +1245,16 @@ static void strongarm_uart_init(Object *obj)
   "uart", 0x1);
 sysbus_init_mmio(dev, >iomem);
 sysbus_init_irq(dev, >irq);
-
-s->rx_timeout_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, 
strongarm_uart_rx_to, s);
-s->tx_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, strongarm_uart_tx, s);
 }
 
 static void strongarm_uart_realize(DeviceState *dev, Error **errp)
 {
 StrongARMUARTState *s = STRONGARM_UART(dev);
 
+s->rx_timeout_timer = 

[PATCH v2 1/2] s390x: fix memleaks in cpu_finalize

[pannengyuan]

From: Pan Nengyuan 

This patch fix memleaks when we call tests/qtest/cpu-plug-test on s390x. The 
leak stack is as follow:

Direct leak of 48 byte(s) in 1 object(s) allocated from:
#0 0x7fb43c7cd970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
#1 0x7fb43be2149d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
#2 0x558ba96da716 in timer_new_full 
/mnt/sdb/qemu-new/qemu/include/qemu/timer.h:530
#3 0x558ba96da716 in timer_new 
/mnt/sdb/qemu-new/qemu/include/qemu/timer.h:551
#4 0x558ba96da716 in timer_new_ns 
/mnt/sdb/qemu-new/qemu/include/qemu/timer.h:569
#5 0x558ba96da716 in s390_cpu_initfn 
/mnt/sdb/qemu-new/qemu/target/s390x/cpu.c:285
#6 0x558ba9c969ab in object_init_with_type 
/mnt/sdb/qemu-new/qemu/qom/object.c:372
#7 0x558ba9c9eb5f in object_initialize_with_type 
/mnt/sdb/qemu-new/qemu/qom/object.c:516
#8 0x558ba9c9f053 in object_new_with_type 
/mnt/sdb/qemu-new/qemu/qom/object.c:684
#9 0x558ba967ede6 in s390x_new_cpu 
/mnt/sdb/qemu-new/qemu/hw/s390x/s390-virtio-ccw.c:64
#10 0x558ba99764b3 in hmp_cpu_add 
/mnt/sdb/qemu-new/qemu/hw/core/machine-hmp-cmds.c:57
#11 0x558ba9b1c27f in handle_hmp_command 
/mnt/sdb/qemu-new/qemu/monitor/hmp.c:1082
#12 0x558ba96c1b02 in qmp_human_monitor_command 
/mnt/sdb/qemu-new/qemu/monitor/misc.c:142

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Cc: Richard Henderson 
Cc: Cornelia Huck 
---
Changes v2 to v1:
- Similarly to other cleanups, move timer_new into realize, then do
timer_del in unrealize.
---
 target/s390x/cpu.c | 22 ++
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index cf84d307c6..f18dbc6fe4 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -170,7 +170,12 @@ static void s390_cpu_realizefn(DeviceState *dev, Error 
**errp)
 S390CPUClass *scc = S390_CPU_GET_CLASS(dev);
 #if !defined(CONFIG_USER_ONLY)
 S390CPU *cpu = S390_CPU(dev);
+cpu->env.tod_timer =
+timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_tod_timer, cpu);
+cpu->env.cpu_timer =
+timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_cpu_timer, cpu);
 #endif
+
 Error *err = NULL;
 
 /* the model has to be realized before qemu_init_vcpu() due to kvm */
@@ -227,6 +232,16 @@ out:
 error_propagate(errp, err);
 }
 
+static void s390_cpu_unrealizefn(DeviceState *dev, Error **errp)
+{
+#if !defined(CONFIG_USER_ONLY)
+S390CPU *cpu = S390_CPU(dev);
+
+timer_del(cpu->env.tod_timer);
+timer_del(cpu->env.cpu_timer);
+#endif
+}
+
 static GuestPanicInformation *s390_cpu_get_crash_info(CPUState *cs)
 {
 GuestPanicInformation *panic_info;
@@ -279,10 +294,6 @@ static void s390_cpu_initfn(Object *obj)
 s390_cpu_get_crash_info_qom, NULL, NULL, NULL, NULL);
 s390_cpu_model_register_props(obj);
 #if !defined(CONFIG_USER_ONLY)
-cpu->env.tod_timer =
-timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_tod_timer, cpu);
-cpu->env.cpu_timer =
-timer_new_ns(QEMU_CLOCK_VIRTUAL, s390x_cpu_timer, cpu);
 s390_cpu_set_state(S390_CPU_STATE_STOPPED, cpu);
 #endif
 }
@@ -294,6 +305,8 @@ static void s390_cpu_finalize(Object *obj)
 
 qemu_unregister_reset(s390_cpu_machine_reset_cb, cpu);
 g_free(cpu->irqstate);
+timer_free(cpu->env.tod_timer);
+timer_free(cpu->env.cpu_timer);
 #endif
 }
 
@@ -453,6 +466,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
 
 device_class_set_parent_realize(dc, s390_cpu_realizefn,
 >parent_realize);
+dc->unrealize = s390_cpu_unrealizefn;
 device_class_set_props(dc, s390x_cpu_properties);
 dc->user_creatable = true;
 
-- 
2.21.0.windows.1

[PATCH v2 0/2] delay timer_new from init to realize to fix memleaks.

[pannengyuan]

From: Pan Nengyuan 

v1:
   - Delay timer_new() from init() to realize() to fix memleaks.
v2:
   - Similarly to other cleanups, move timer_new into realize in 
target/s390x/cpu.c (Suggested by Philippe Mathieu-Daudé).
   - Send these two patches as a series instead of sending each as a single 
patch but with wrong subject in v1.

Pan Nengyuan (2):
  s390x: fix memleaks in cpu_finalize
  hw: move timer_new from init() into realize() to avoid memleaks

 hw/arm/pxa2xx.c| 17 +++--
 hw/arm/spitz.c |  8 +++-
 hw/arm/strongarm.c | 18 --
 hw/misc/mos6522.c  | 14 --
 hw/timer/cadence_ttc.c | 16 +++-
 target/s390x/cpu.c | 22 ++
 6 files changed, 71 insertions(+), 24 deletions(-)

-- 
2.21.0.windows.1

[PATCH 2/2] hw: move timer_new from init() into realize() to avoid memleaks

[pannengyuan]

From: Pan Nengyuan 

There are some memleaks when we call 'device_list_properties'. This patch move 
timer_new from init into realize to fix it.
Meanwhile, do the null check in mos6522_reset() to avoid null deref if we move 
timer_new into realize().

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 hw/arm/pxa2xx.c| 17 +++--
 hw/arm/spitz.c |  8 +++-
 hw/arm/strongarm.c | 18 --
 hw/misc/mos6522.c  | 14 --
 hw/timer/cadence_ttc.c | 16 +++-
 5 files changed, 53 insertions(+), 20 deletions(-)

diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
index b33f8f1351..56a36202d7 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -1134,18 +1134,22 @@ static void pxa2xx_rtc_init(Object *obj)
 s->last_rtcpicr = 0;
 s->last_hz = s->last_sw = s->last_pi = qemu_clock_get_ms(rtc_clock);
 
+sysbus_init_irq(dev, >rtc_irq);
+
+memory_region_init_io(>iomem, obj, _rtc_ops, s,
+  "pxa2xx-rtc", 0x1);
+sysbus_init_mmio(dev, >iomem);
+}
+
+static void pxa2xx_rtc_realize(DeviceState *dev, Error **errp)
+{
+PXA2xxRTCState *s = PXA2XX_RTC(dev);
 s->rtc_hz= timer_new_ms(rtc_clock, pxa2xx_rtc_hz_tick,s);
 s->rtc_rdal1 = timer_new_ms(rtc_clock, pxa2xx_rtc_rdal1_tick, s);
 s->rtc_rdal2 = timer_new_ms(rtc_clock, pxa2xx_rtc_rdal2_tick, s);
 s->rtc_swal1 = timer_new_ms(rtc_clock, pxa2xx_rtc_swal1_tick, s);
 s->rtc_swal2 = timer_new_ms(rtc_clock, pxa2xx_rtc_swal2_tick, s);
 s->rtc_pi= timer_new_ms(rtc_clock, pxa2xx_rtc_pi_tick,s);
-
-sysbus_init_irq(dev, >rtc_irq);
-
-memory_region_init_io(>iomem, obj, _rtc_ops, s,
-  "pxa2xx-rtc", 0x1);
-sysbus_init_mmio(dev, >iomem);
 }
 
 static int pxa2xx_rtc_pre_save(void *opaque)
@@ -1203,6 +1207,7 @@ static void pxa2xx_rtc_sysbus_class_init(ObjectClass 
*klass, void *data)
 
 dc->desc = "PXA2xx RTC Controller";
 dc->vmsd = _pxa2xx_rtc_regs;
+dc->realize = pxa2xx_rtc_realize;
 }
 
 static const TypeInfo pxa2xx_rtc_sysbus_info = {
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index e001088103..cbfa6934cf 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -524,11 +524,16 @@ static void spitz_keyboard_init(Object *obj)
 
 spitz_keyboard_pre_map(s);
 
-s->kbdtimer = timer_new_ns(QEMU_CLOCK_VIRTUAL, spitz_keyboard_tick, s);
 qdev_init_gpio_in(dev, spitz_keyboard_strobe, SPITZ_KEY_STROBE_NUM);
 qdev_init_gpio_out(dev, s->sense, SPITZ_KEY_SENSE_NUM);
 }
 
+static void spitz_keyboard_realize(DeviceState *dev, Error **errp)
+{
+SpitzKeyboardState *s = SPITZ_KEYBOARD(dev);
+s->kbdtimer = timer_new_ns(QEMU_CLOCK_VIRTUAL, spitz_keyboard_tick, s);
+}
+
 /* LCD backlight controller */
 
 #define LCDTG_RESCTL   0x00
@@ -1115,6 +1120,7 @@ static void spitz_keyboard_class_init(ObjectClass *klass, 
void *data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 
 dc->vmsd = _spitz_kbd;
+dc->realize = spitz_keyboard_realize;
 }
 
 static const TypeInfo spitz_keyboard_info = {
diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
index cd8a99aaf2..3010d765bb 100644
--- a/hw/arm/strongarm.c
+++ b/hw/arm/strongarm.c
@@ -399,9 +399,6 @@ static void strongarm_rtc_init(Object *obj)
 s->last_rcnr = (uint32_t) mktimegm();
 s->last_hz = qemu_clock_get_ms(rtc_clock);
 
-s->rtc_alarm = timer_new_ms(rtc_clock, strongarm_rtc_alarm_tick, s);
-s->rtc_hz = timer_new_ms(rtc_clock, strongarm_rtc_hz_tick, s);
-
 sysbus_init_irq(dev, >rtc_irq);
 sysbus_init_irq(dev, >rtc_hz_irq);
 
@@ -410,6 +407,13 @@ static void strongarm_rtc_init(Object *obj)
 sysbus_init_mmio(dev, >iomem);
 }
 
+static void strongarm_rtc_realize(DeviceState *dev, Error **errp)
+{
+StrongARMRTCState *s = STRONGARM_RTC(dev);
+s->rtc_alarm = timer_new_ms(rtc_clock, strongarm_rtc_alarm_tick, s);
+s->rtc_hz = timer_new_ms(rtc_clock, strongarm_rtc_hz_tick, s);
+}
+
 static int strongarm_rtc_pre_save(void *opaque)
 {
 StrongARMRTCState *s = opaque;
@@ -451,6 +455,7 @@ static void strongarm_rtc_sysbus_class_init(ObjectClass 
*klass, void *data)
 
 dc->desc = "StrongARM RTC Controller";
 dc->vmsd = _strongarm_rtc_regs;
+dc->realize = strongarm_rtc_realize;
 }
 
 static const TypeInfo strongarm_rtc_sysbus_info = {
@@ -1240,15 +1245,16 @@ static void strongarm_uart_init(Object *obj)
   "uart", 0x1);
 sysbus_init_mmio(dev, >iomem);
 sysbus_init_irq(dev, >irq);
-
-s->rx_timeout_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, 
strongarm_uart_rx_to, s);
-s->tx_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, strongarm_uart_tx, s);
 }
 
 static void strongarm_uart_realize(DeviceState *dev, Error **errp)
 {
 StrongARMUARTState *s = STRONGARM_UART(dev);
 
+s->rx_timeout_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
+   strongarm_uart_rx_to,
+   s);
+s->tx_timer = 

[PATCH 1/2] s390x: fix memleaks in cpu_finalize

[pannengyuan]

From: Pan Nengyuan 

This patch fix memleaks when we call tests/qtest/cpu-plug-test on s390x. The 
leak stack is as follow:

Direct leak of 48 byte(s) in 1 object(s) allocated from:
#0 0x7fb43c7cd970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
#1 0x7fb43be2149d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
#2 0x558ba96da716 in timer_new_full 
/mnt/sdb/qemu-new/qemu/include/qemu/timer.h:530
#3 0x558ba96da716 in timer_new 
/mnt/sdb/qemu-new/qemu/include/qemu/timer.h:551
#4 0x558ba96da716 in timer_new_ns 
/mnt/sdb/qemu-new/qemu/include/qemu/timer.h:569
#5 0x558ba96da716 in s390_cpu_initfn 
/mnt/sdb/qemu-new/qemu/target/s390x/cpu.c:285
#6 0x558ba9c969ab in object_init_with_type 
/mnt/sdb/qemu-new/qemu/qom/object.c:372
#7 0x558ba9c9eb5f in object_initialize_with_type 
/mnt/sdb/qemu-new/qemu/qom/object.c:516
#8 0x558ba9c9f053 in object_new_with_type 
/mnt/sdb/qemu-new/qemu/qom/object.c:684
#9 0x558ba967ede6 in s390x_new_cpu 
/mnt/sdb/qemu-new/qemu/hw/s390x/s390-virtio-ccw.c:64
#10 0x558ba99764b3 in hmp_cpu_add 
/mnt/sdb/qemu-new/qemu/hw/core/machine-hmp-cmds.c:57
#11 0x558ba9b1c27f in handle_hmp_command 
/mnt/sdb/qemu-new/qemu/monitor/hmp.c:1082
#12 0x558ba96c1b02 in qmp_human_monitor_command 
/mnt/sdb/qemu-new/qemu/monitor/misc.c:142

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 target/s390x/cpu.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index cf84d307c6..fff793a4eb 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -294,6 +294,10 @@ static void s390_cpu_finalize(Object *obj)
 
 qemu_unregister_reset(s390_cpu_machine_reset_cb, cpu);
 g_free(cpu->irqstate);
+timer_del(cpu->env.tod_timer);
+timer_del(cpu->env.cpu_timer);
+timer_free(cpu->env.tod_timer);
+timer_free(cpu->env.cpu_timer);
 #endif
 }
 
-- 
2.18.2

[PATCH] ppc: free 'fdt' after reset the machine

[pannengyuan]

From: Pan Nengyuan 

'fdt' forgot to clean both e500 and pnv when we call 'system_reset' on ppc,
this patch fix it. The leak stacks are as follow:

Direct leak of 4194304 byte(s) in 4 object(s) allocated from:
#0 0x7fafe37dd970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
#1 0x7fafe2e3149d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
#2 0x561876f7f80d in create_device_tree 
/mnt/sdb/qemu-new/qemu/device_tree.c:40
#3 0x561876b7ac29 in ppce500_load_device_tree 
/mnt/sdb/qemu-new/qemu/hw/ppc/e500.c:364
#4 0x561876b7f437 in ppce500_reset_device_tree 
/mnt/sdb/qemu-new/qemu/hw/ppc/e500.c:617
#5 0x56187718b1ae in qemu_devices_reset 
/mnt/sdb/qemu-new/qemu/hw/core/reset.c:69
#6 0x561876f6938d in qemu_system_reset /mnt/sdb/qemu-new/qemu/vl.c:1412
#7 0x561876f6a25b in main_loop_should_exit /mnt/sdb/qemu-new/qemu/vl.c:1645
#8 0x561876f6a398 in main_loop /mnt/sdb/qemu-new/qemu/vl.c:1679
#9 0x561876f7da8e in main /mnt/sdb/qemu-new/qemu/vl.c:4438
#10 0x7fafde16b812 in __libc_start_main ../csu/libc-start.c:308
#11 0x5618765c055d in _start 
(/mnt/sdb/qemu-new/qemu/build/ppc64-softmmu/qemu-system-ppc64+0x2b1555d)

Direct leak of 1048576 byte(s) in 1 object(s) allocated from:
#0 0x7fc0a6f1b970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
#1 0x7fc0a656f49d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
#2 0x55eb05acd2ca in pnv_dt_create /mnt/sdb/qemu-new/qemu/hw/ppc/pnv.c:507
#3 0x55eb05ace5bf in pnv_reset /mnt/sdb/qemu-new/qemu/hw/ppc/pnv.c:578
#4 0x55eb05f2f395 in qemu_system_reset /mnt/sdb/qemu-new/qemu/vl.c:1410
#5 0x55eb05f43850 in main /mnt/sdb/qemu-new/qemu/vl.c:4403
#6 0x7fc0a18a9812 in __libc_start_main ../csu/libc-start.c:308
#7 0x55eb0558655d in _start 
(/mnt/sdb/qemu-new/qemu/build/ppc64-softmmu/qemu-system-ppc64+0x2b1555d)

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 hw/ppc/e500.c | 1 +
 hw/ppc/pnv.c  | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index 886442e54f..af537bba2b 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -594,6 +594,7 @@ done:
 cpu_physical_memory_write(addr, fdt, fdt_size);
 }
 ret = fdt_size;
+g_free(fdt);
 
 out:
 g_free(pci_map);
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 139c857b1e..e98038b809 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -582,6 +582,8 @@ static void pnv_reset(MachineState *machine)
 
 qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
 cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(fdt));
+
+g_free(fdt);
 }
 
 static ISABus *pnv_chip_power8_isa_create(PnvChip *chip, Error **errp)
-- 
2.18.2

[PATCH 0/2] delete virtio queues in vhost-user-blk-unrealize

[pannengyuan]

From: Pan Nengyuan 

This series patch fix memleaks when detaching vhost-user-blk device.
1. use old virtio_del_queue to fix memleaks, it's easier for stable branches to 
merge.
   As the discussion in 
https://lists.nongnu.org/archive/html/qemu-devel/2020-01/msg02903.html

2. convert virtio_del_queue to the new one(virtio_delete_queue).

Pan Nengyuan (2):
  vhost-user-blk: delete virtioqueues in unrealize to fix memleaks
  vhost-use-blk: convert to new virtio_delete_queue

 hw/block/vhost-user-blk.c  | 15 +--
 include/hw/virtio/vhost-user-blk.h |  1 +
 2 files changed, 14 insertions(+), 2 deletions(-)

-- 
2.21.0.windows.1

[PATCH 1/2] vhost-user-blk: delete virtioqueues in unrealize to fix memleaks

[pannengyuan]

From: Pan Nengyuan 

virtio queues forgot to delete in unrealize, and aslo error path in
realize, this patch fix these memleaks, the leak stack is as follow:

Direct leak of 114688 byte(s) in 16 object(s) allocated from:
#0 0x7f24024fdbf0 in calloc (/lib64/libasan.so.3+0xcabf0)
#1 0x7f2401642015 in g_malloc0 (/lib64/libglib-2.0.so.0+0x50015)
#2 0x55ad175a6447 in virtio_add_queue /mnt/sdb/qemu/hw/virtio/virtio.c:2327
#3 0x55ad17570cf9 in vhost_user_blk_device_realize 
/mnt/sdb/qemu/hw/block/vhost-user-blk.c:419
#4 0x55ad175a3707 in virtio_device_realize 
/mnt/sdb/qemu/hw/virtio/virtio.c:3509
#5 0x55ad176ad0d1 in device_set_realized /mnt/sdb/qemu/hw/core/qdev.c:876
#6 0x55ad1781ff9d in property_set_bool /mnt/sdb/qemu/qom/object.c:2080
#7 0x55ad178245ae in object_property_set_qobject 
/mnt/sdb/qemu/qom/qom-qobject.c:26
#8 0x55ad17821eb4 in object_property_set_bool 
/mnt/sdb/qemu/qom/object.c:1338
#9 0x55ad177aeed7 in virtio_pci_realize 
/mnt/sdb/qemu/hw/virtio/virtio-pci.c:1801

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 hw/block/vhost-user-blk.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index d8c459c575..2eba8b9db0 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -460,6 +460,9 @@ reconnect:
 virtio_err:
 g_free(s->vqs);
 g_free(s->inflight);
+for (i = 0; i < s->num_queues; i++) {
+virtio_del_queue(vdev, i);
+}
 virtio_cleanup(vdev);
 vhost_user_cleanup(>vhost_user);
 }
@@ -468,6 +471,7 @@ static void vhost_user_blk_device_unrealize(DeviceState 
*dev, Error **errp)
 {
 VirtIODevice *vdev = VIRTIO_DEVICE(dev);
 VHostUserBlk *s = VHOST_USER_BLK(dev);
+int i;
 
 virtio_set_status(vdev, 0);
 qemu_chr_fe_set_handlers(>chardev,  NULL, NULL, NULL,
@@ -476,6 +480,10 @@ static void vhost_user_blk_device_unrealize(DeviceState 
*dev, Error **errp)
 vhost_dev_free_inflight(s->inflight);
 g_free(s->vqs);
 g_free(s->inflight);
+
+for (i = 0; i < s->num_queues; i++) {
+virtio_del_queue(vdev, i);
+}
 virtio_cleanup(vdev);
 vhost_user_cleanup(>vhost_user);
 }
-- 
2.21.0.windows.1

[PATCH 2/2] vhost-use-blk: convert to new virtio_delete_queue

[pannengyuan]

From: Pan Nengyuan 

use the new virtio_delete_queue function to cleanup.

Signed-off-by: Pan Nengyuan 
---
 hw/block/vhost-user-blk.c  | 11 +++
 include/hw/virtio/vhost-user-blk.h |  1 +
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 2eba8b9db0..ed6a5cc03b 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -420,9 +420,10 @@ static void vhost_user_blk_device_realize(DeviceState 
*dev, Error **errp)
 virtio_init(vdev, "virtio-blk", VIRTIO_ID_BLOCK,
 sizeof(struct virtio_blk_config));
 
+s->virtqs = g_new0(VirtQueue *, s->num_queues);
 for (i = 0; i < s->num_queues; i++) {
-virtio_add_queue(vdev, s->queue_size,
- vhost_user_blk_handle_output);
+s->virtqs[i] = virtio_add_queue(vdev, s->queue_size,
+vhost_user_blk_handle_output);
 }
 
 s->inflight = g_new0(struct vhost_inflight, 1);
@@ -461,8 +462,9 @@ virtio_err:
 g_free(s->vqs);
 g_free(s->inflight);
 for (i = 0; i < s->num_queues; i++) {
-virtio_del_queue(vdev, i);
+virtio_delete_queue(s->virtqs[i]);
 }
+g_free(s->virtqs);
 virtio_cleanup(vdev);
 vhost_user_cleanup(>vhost_user);
 }
@@ -482,8 +484,9 @@ static void vhost_user_blk_device_unrealize(DeviceState 
*dev, Error **errp)
 g_free(s->inflight);
 
 for (i = 0; i < s->num_queues; i++) {
-virtio_del_queue(vdev, i);
+virtio_delete_queue(s->virtqs[i]);
 }
+g_free(s->virtqs);
 virtio_cleanup(vdev);
 vhost_user_cleanup(>vhost_user);
 }
diff --git a/include/hw/virtio/vhost-user-blk.h 
b/include/hw/virtio/vhost-user-blk.h
index 108bfadeeb..f68911f6f0 100644
--- a/include/hw/virtio/vhost-user-blk.h
+++ b/include/hw/virtio/vhost-user-blk.h
@@ -37,6 +37,7 @@ typedef struct VHostUserBlk {
 struct vhost_inflight *inflight;
 VhostUserState vhost_user;
 struct vhost_virtqueue *vqs;
+VirtQueue **virtqs;
 guint watch;
 bool connected;
 } VHostUserBlk;
-- 
2.21.0.windows.1

[PATCH] migration-test: fix some memleaks in migration-test

[pannengyuan]

From: Pan Nengyuan 

spotted by asan, 'check-qtest-aarch64' runs fail if sanitizers is enabled.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 tests/qtest/migration-test.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index cf27ebbc9d..2bb214c87f 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -498,11 +498,13 @@ static int test_migrate_start(QTestState **from, 
QTestState **to,
 const char *arch = qtest_get_arch();
 const char *machine_opts = NULL;
 const char *memory_size;
+int ret = 0;
 
 if (args->use_shmem) {
 if (!g_file_test("/dev/shm", G_FILE_TEST_IS_DIR)) {
 g_test_skip("/dev/shm is not supported");
-return -1;
+ret = -1;
+goto out;
 }
 }
 
@@ -611,8 +613,9 @@ static int test_migrate_start(QTestState **from, QTestState 
**to,
 g_free(shmem_path);
 }
 
+out:
 migrate_start_destroy(args);
-return 0;
+return ret;
 }
 
 static void test_migrate_end(QTestState *from, QTestState *to, bool test_dest)
@@ -1134,6 +1137,8 @@ static void test_validate_uuid(void)
 {
 MigrateStart *args = migrate_start_new();
 
+g_free(args->opts_source);
+g_free(args->opts_target);
 args->opts_source = g_strdup("-uuid ----");
 args->opts_target = g_strdup("-uuid ----");
 do_test_validate_uuid(args, false);
@@ -1143,6 +1148,8 @@ static void test_validate_uuid_error(void)
 {
 MigrateStart *args = migrate_start_new();
 
+g_free(args->opts_source);
+g_free(args->opts_target);
 args->opts_source = g_strdup("-uuid ----");
 args->opts_target = g_strdup("-uuid ----");
 args->hide_stderr = true;
@@ -1153,6 +1160,7 @@ static void test_validate_uuid_src_not_set(void)
 {
 MigrateStart *args = migrate_start_new();
 
+g_free(args->opts_target);
 args->opts_target = g_strdup("-uuid ----");
 args->hide_stderr = true;
 do_test_validate_uuid(args, false);
@@ -1162,6 +1170,7 @@ static void test_validate_uuid_dst_not_set(void)
 {
 MigrateStart *args = migrate_start_new();
 
+g_free(args->opts_source);
 args->opts_source = g_strdup("-uuid ----");
 args->hide_stderr = true;
 do_test_validate_uuid(args, false);
@@ -1379,6 +1388,7 @@ static void test_multifd_tcp_cancel(void)
 "  'arguments': { 'uri': 'tcp:127.0.0.1:0' }}");
 qobject_unref(rsp);
 
+g_free(uri);
 uri = migrate_get_socket_address(to2, "socket-address");
 
 wait_for_migration_status(from, "cancelled", NULL);
-- 
2.18.1

[PATCH 3/3] stellaris: delay timer_new to avoid memleaks

[pannengyuan]

From: Pan Nengyuan 

There is a memory leak when we call 'device_list_properties' with typename = 
stellaris-gptm. It's easy to reproduce as follow:

  virsh qemu-monitor-command vm1 --pretty '{"execute": 
"device-list-properties", "arguments": {"typename": "stellaris-gptm"}}'

This patch delay timer_new in realize to fix it.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Cc: qemu-...@nongnu.org
---
 hw/arm/stellaris.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index bb025e0bd0..221a78674e 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -347,11 +347,15 @@ static void stellaris_gptm_init(Object *obj)
 sysbus_init_mmio(sbd, >iomem);
 
 s->opaque[0] = s->opaque[1] = s;
+}
+
+static void stellaris_gptm_realize(DeviceState *dev, Error **errp)
+{
+gptm_state *s = STELLARIS_GPTM(dev);
 s->timer[0] = timer_new_ns(QEMU_CLOCK_VIRTUAL, gptm_tick, >opaque[0]);
 s->timer[1] = timer_new_ns(QEMU_CLOCK_VIRTUAL, gptm_tick, >opaque[1]);
 }
 
-
 /* System controller.  */
 
 typedef struct {
@@ -1536,6 +1540,7 @@ static void stellaris_gptm_class_init(ObjectClass *klass, 
void *data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 
 dc->vmsd = _stellaris_gptm;
+dc->realize = stellaris_gptm_realize;
 }
 
 static const TypeInfo stellaris_gptm_info = {
-- 
2.21.0.windows.1

[PATCH 1/3] armv7m_systick: delay timer_new to avoid memleaks

[pannengyuan]

From: Pan Nengyuan 

There is a memory leak when we call 'device_list_properties' with typename = 
armv7m_systick. It's easy to reproduce as follow:

  virsh qemu-monitor-command vm1 --pretty '{"execute": 
"device-list-properties", "arguments": {"typename": "armv7m_systick"}}'

This patch delay timer_new to fix this memleaks.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Cc: qemu-...@nongnu.org
---
 hw/timer/armv7m_systick.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/timer/armv7m_systick.c b/hw/timer/armv7m_systick.c
index 85d122dbcb..74c58bcf24 100644
--- a/hw/timer/armv7m_systick.c
+++ b/hw/timer/armv7m_systick.c
@@ -216,6 +216,11 @@ static void systick_instance_init(Object *obj)
 memory_region_init_io(>iomem, obj, _ops, s, "systick", 0xe0);
 sysbus_init_mmio(sbd, >iomem);
 sysbus_init_irq(sbd, >irq);
+}
+
+static void systick_realize(DeviceState *dev, Error **errp)
+{
+SysTickState *s = SYSTICK(dev);
 s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, systick_timer_tick, s);
 }
 
@@ -238,6 +243,7 @@ static void systick_class_init(ObjectClass *klass, void 
*data)
 
 dc->vmsd = _systick;
 dc->reset = systick_reset;
+dc->realize = systick_realize;
 }
 
 static const TypeInfo armv7m_systick_info = {
-- 
2.21.0.windows.1

[PATCH 2/3] stm32f2xx_timer: delay timer_new to avoid memleaks

[pannengyuan]

From: Pan Nengyuan 

There is a memory leak when we call 'device_list_properties' with typename = 
stm32f2xx_timer. It's easy to reproduce as follow:

virsh qemu-monitor-command vm1 --pretty '{"execute": 
"device-list-properties", "arguments": {"typename": "stm32f2xx_timer"}}'

This patch delay timer_new to fix this memleaks.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Cc: Alistair Francis 
---
 hw/timer/stm32f2xx_timer.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/timer/stm32f2xx_timer.c b/hw/timer/stm32f2xx_timer.c
index fb370ce0f0..06ec8a02c2 100644
--- a/hw/timer/stm32f2xx_timer.c
+++ b/hw/timer/stm32f2xx_timer.c
@@ -314,7 +314,11 @@ static void stm32f2xx_timer_init(Object *obj)
 memory_region_init_io(>iomem, obj, _timer_ops, s,
   "stm32f2xx_timer", 0x400);
 sysbus_init_mmio(SYS_BUS_DEVICE(obj), >iomem);
+}
 
+static void stm32f2xx_timer_realize(DeviceState *dev, Error **errp)
+{
+STM32F2XXTimerState *s = STM32F2XXTIMER(dev);
 s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, stm32f2xx_timer_interrupt, s);
 }
 
@@ -325,6 +329,7 @@ static void stm32f2xx_timer_class_init(ObjectClass *klass, 
void *data)
 dc->reset = stm32f2xx_timer_reset;
 device_class_set_props(dc, stm32f2xx_timer_properties);
 dc->vmsd = _stm32f2xx_timer;
+dc->realize = stm32f2xx_timer_realize;
 }
 
 static const TypeInfo stm32f2xx_timer_info = {
-- 
2.21.0.windows.1

[PATCH 0/3] delay timer_new to avoid memleaks

[pannengyuan]

From: Pan Nengyuan 

This series delay timer_new into realize() to fix some memleaks when we call 
'device-list-properties'.

Pan Nengyuan (3):
  armv7m_systick: delay timer_new to avoid memleaks
  stm32f2xx_timer: delay timer_new to avoid memleaks
  stellaris: delay timer_new to avoid memleaks

 hw/arm/stellaris.c | 7 ++-
 hw/timer/armv7m_systick.c  | 6 ++
 hw/timer/stm32f2xx_timer.c | 5 +
 3 files changed, 17 insertions(+), 1 deletion(-)

-- 
2.21.0.windows.1

[PATCH v2] pl031: add finalize function to avoid memleaks

[pannengyuan]

From: Pan Nengyuan 

There is a memory leak when we call 'device_list_properties' with
typename = pl031. It's easy to reproduce as follow:

  virsh qemu-monitor-command vm1 --pretty '{"execute": 
"device-list-properties", "arguments": {"typename": "pl031"}}'

The memory leak stack:
  Direct leak of 48 byte(s) in 1 object(s) allocated from:
#0 0x7f6e0925a970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
#1 0x7f6e06f4d49d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
#2 0x564a0f7654ea in timer_new_full /mnt/sdb/qemu/include/qemu/timer.h:530
#3 0x564a0f76555d in timer_new /mnt/sdb/qemu/include/qemu/timer.h:551
#4 0x564a0f765589 in timer_new_ns /mnt/sdb/qemu/include/qemu/timer.h:569
#5 0x564a0f76747d in pl031_init /mnt/sdb/qemu/hw/rtc/pl031.c:198
#6 0x564a0fd4a19d in object_init_with_type /mnt/sdb/qemu/qom/object.c:360
#7 0x564a0fd4b166 in object_initialize_with_type 
/mnt/sdb/qemu/qom/object.c:467
#8 0x564a0fd4c8e6 in object_new_with_type /mnt/sdb/qemu/qom/object.c:636
#9 0x564a0fd4c98e in object_new /mnt/sdb/qemu/qom/object.c:646
#10 0x564a0fc69d43 in qmp_device_list_properties 
/mnt/sdb/qemu/qom/qom-qmp-cmds.c:204
#11 0x564a0ef18e64 in qdev_device_help /mnt/sdb/qemu/qdev-monitor.c:278

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
Changes V2 to V1:
- Delay the timer_new until realize instead of putting it into instance_init, 
since the pl031 can't be hotplugged(suggested by Peter Maydell).
---
 hw/rtc/pl031.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/hw/rtc/pl031.c b/hw/rtc/pl031.c
index ae47f09635..0b9253eb30 100644
--- a/hw/rtc/pl031.c
+++ b/hw/rtc/pl031.c
@@ -190,7 +190,11 @@ static void pl031_init(Object *obj)
 qemu_get_timedate(, 0);
 s->tick_offset = mktimegm() -
 qemu_clock_get_ns(rtc_clock) / NANOSECONDS_PER_SECOND;
+}
 
+static void pl031_realize(DeviceState *dev, Error **errp)
+{
+PL031State *s = PL031(dev);
 s->timer = timer_new_ns(rtc_clock, pl031_interrupt, s);
 }
 
@@ -321,6 +325,7 @@ static void pl031_class_init(ObjectClass *klass, void *data)
 DeviceClass *dc = DEVICE_CLASS(klass);
 
 dc->vmsd = _pl031;
+dc->realize = pl031_realize;
 device_class_set_props(dc, pl031_properties);
 }
 
-- 
2.21.0.windows.1

[PATCH] pl031: add finalize function to avoid memleaks

[pannengyuan]

From: Pan Nengyuan 

There is a memory leak when we call 'device_list_properties' with
typename = pl031. It's easy to reproduce as follow:

  virsh qemu-monitor-command vm1 --pretty '{"execute": 
"device-list-properties", "arguments": {"typename": "pl031"}}'

The memory leak stack:
  Direct leak of 48 byte(s) in 1 object(s) allocated from:
#0 0x7f6e0925a970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
#1 0x7f6e06f4d49d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
#2 0x564a0f7654ea in timer_new_full /mnt/sdb/qemu/include/qemu/timer.h:530
#3 0x564a0f76555d in timer_new /mnt/sdb/qemu/include/qemu/timer.h:551
#4 0x564a0f765589 in timer_new_ns /mnt/sdb/qemu/include/qemu/timer.h:569
#5 0x564a0f76747d in pl031_init /mnt/sdb/qemu/hw/rtc/pl031.c:198
#6 0x564a0fd4a19d in object_init_with_type /mnt/sdb/qemu/qom/object.c:360
#7 0x564a0fd4b166 in object_initialize_with_type 
/mnt/sdb/qemu/qom/object.c:467
#8 0x564a0fd4c8e6 in object_new_with_type /mnt/sdb/qemu/qom/object.c:636
#9 0x564a0fd4c98e in object_new /mnt/sdb/qemu/qom/object.c:646
#10 0x564a0fc69d43 in qmp_device_list_properties 
/mnt/sdb/qemu/qom/qom-qmp-cmds.c:204
#11 0x564a0ef18e64 in qdev_device_help /mnt/sdb/qemu/qdev-monitor.c:278

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 hw/rtc/pl031.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/hw/rtc/pl031.c b/hw/rtc/pl031.c
index ae47f09635..50664ca000 100644
--- a/hw/rtc/pl031.c
+++ b/hw/rtc/pl031.c
@@ -194,6 +194,15 @@ static void pl031_init(Object *obj)
 s->timer = timer_new_ns(rtc_clock, pl031_interrupt, s);
 }
 
+static void pl031_finalize(Object *obj)
+{
+PL031State *s = PL031(obj);
+if (s->timer) {
+timer_del(s->timer);
+timer_free(s->timer);
+}
+}
+
 static int pl031_pre_save(void *opaque)
 {
 PL031State *s = opaque;
@@ -329,6 +338,7 @@ static const TypeInfo pl031_info = {
 .parent= TYPE_SYS_BUS_DEVICE,
 .instance_size = sizeof(PL031State),
 .instance_init = pl031_init,
+.instance_finalize = pl031_finalize,
 .class_init= pl031_class_init,
 };
 
-- 
2.21.0.windows.1

[PATCH] boot-order-test: fix memleaks in boot-order-test

[pannengyuan]

From: Pan Nengyuan 

It's not a big deal, but 'check qtest-ppc/ppc64' runs fail if sanitizers is 
enabled.
The memory leak stack is as follow:

Direct leak of 128 byte(s) in 4 object(s) allocated from:
#0 0x7f11756f5970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
#1 0x7f1174f2549d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
#2 0x556af05aa7da in mm_fw_cfg_init /mnt/sdb/qemu/tests/libqos/fw_cfg.c:119
#3 0x556af059f4f5 in read_boot_order_pmac 
/mnt/sdb/qemu/tests/boot-order-test.c:137
#4 0x556af059efe2 in test_a_boot_order 
/mnt/sdb/qemu/tests/boot-order-test.c:47
#5 0x556af059f2c0 in test_boot_orders 
/mnt/sdb/qemu/tests/boot-order-test.c:59
#6 0x556af059f52d in test_pmac_oldworld_boot_order 
/mnt/sdb/qemu/tests/boot-order-test.c:152
#7 0x7f1174f46cb9  (/lib64/libglib-2.0.so.0+0x73cb9)
#8 0x7f1174f46b73  (/lib64/libglib-2.0.so.0+0x73b73)
#9 0x7f1174f46b73  (/lib64/libglib-2.0.so.0+0x73b73)
#10 0x7f1174f46f71 in g_test_run_suite (/lib64/libglib-2.0.so.0+0x73f71)
#11 0x7f1174f46f94 in g_test_run (/lib64/libglib-2.0.so.0+0x73f94)

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 tests/qtest/boot-order-test.c | 6 +++---
 tests/qtest/libqos/fw_cfg.h   | 2 ++
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/boot-order-test.c b/tests/qtest/boot-order-test.c
index a725bce729..4241304ff5 100644
--- a/tests/qtest/boot-order-test.c
+++ b/tests/qtest/boot-order-test.c
@@ -134,7 +134,7 @@ static void test_prep_boot_order(void)
 
 static uint64_t read_boot_order_pmac(QTestState *qts)
 {
-QFWCFG *fw_cfg = mm_fw_cfg_init(qts, 0xf510);
+g_autoptr(QFWCFG) fw_cfg = mm_fw_cfg_init(qts, 0xf510);
 
 return qfw_cfg_get_u16(fw_cfg, FW_CFG_BOOT_DEVICE);
 }
@@ -159,7 +159,7 @@ static void test_pmac_newworld_boot_order(void)
 
 static uint64_t read_boot_order_sun4m(QTestState *qts)
 {
-QFWCFG *fw_cfg = mm_fw_cfg_init(qts, 0xd0510ULL);
+g_autoptr(QFWCFG) fw_cfg = mm_fw_cfg_init(qts, 0xd0510ULL);
 
 return qfw_cfg_get_u16(fw_cfg, FW_CFG_BOOT_DEVICE);
 }
@@ -171,7 +171,7 @@ static void test_sun4m_boot_order(void)
 
 static uint64_t read_boot_order_sun4u(QTestState *qts)
 {
-QFWCFG *fw_cfg = io_fw_cfg_init(qts, 0x510);
+g_autoptr(QFWCFG) fw_cfg = io_fw_cfg_init(qts, 0x510);
 
 return qfw_cfg_get_u16(fw_cfg, FW_CFG_BOOT_DEVICE);
 }
diff --git a/tests/qtest/libqos/fw_cfg.h b/tests/qtest/libqos/fw_cfg.h
index 13325cc4ff..c6a7cf8cf0 100644
--- a/tests/qtest/libqos/fw_cfg.h
+++ b/tests/qtest/libqos/fw_cfg.h
@@ -49,4 +49,6 @@ static inline void pc_fw_cfg_uninit(QFWCFG *fw_cfg)
 io_fw_cfg_uninit(fw_cfg);
 }
 
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(QFWCFG, mm_fw_cfg_uninit)
+
 #endif
-- 
2.21.0.windows.1

[PATCH] stm32f4xx_syscfg: remove redundant code to fix coverity warning

[pannengyuan]

From: Pan Nengyuan 

Fixes the coverity warning:
CID 91708242: (EVALUATION_ORDER)
50. write_write_typo: In "config = config = irq / 16", "config" is written 
twice with the same value.
50uint8_t config = config = irq / 16;

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 hw/misc/stm32f4xx_syscfg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/misc/stm32f4xx_syscfg.c b/hw/misc/stm32f4xx_syscfg.c
index dbcdca59f8..f960e4ea1e 100644
--- a/hw/misc/stm32f4xx_syscfg.c
+++ b/hw/misc/stm32f4xx_syscfg.c
@@ -47,7 +47,7 @@ static void stm32f4xx_syscfg_set_irq(void *opaque, int irq, 
int level)
 STM32F4xxSyscfgState *s = opaque;
 int icrreg = irq / 4;
 int startbit = (irq & 3) * 4;
-uint8_t config = config = irq / 16;
+uint8_t config = irq / 16;
 
 trace_stm32f4xx_syscfg_set_irq(irq / 16, irq % 16, level);
 
-- 
2.21.0.windows.1

[PATCH] backup-top: fix a memory leak in bdrv_backup_top_append()

[pannengyuan]

From: Pan Nengyuan 

top->opaque is aleardy malloced in bdrv_new_open_driver(), and then change
the pointer but without freeing it. It will cause a memory leak, the leak
stack is as follow:

Direct leak of 24 byte(s) in 1 object(s) allocated from:
  #0 0x7ff6f7be4970 (/lib64/libasan.so.5+0xef970)  ??:?
  #1 0x7ff6f723849d (/lib64/libglib-2.0.so.0+0x5249d)  ??:?
  #2 0x564c0d44caae (./x86_64-softmmu/qemu-system-x86_64+0x3b40aae)  
/mnt/sdb/qemu/block.c:1289
  #3 0x564c0d44dbaf (./x86_64-softmmu/qemu-system-x86_64+0x3b41baf)  
/mnt/sdb/qemu/block.c:1359
  #4 0x564c0d71618f (./x86_64-softmmu/qemu-system-x86_64+0x3e0a18f)  
/mnt/sdb/qemu/block/backup-top.c:190
  #5 0x564c0d7001be (./x86_64-softmmu/qemu-system-x86_64+0x3df41be)  
/mnt/sdb/qemu/block/backup.c:439
  #6 0x564c0c8ebef8 (./x86_64-softmmu/qemu-system-x86_64+0x2fdfef8)  
/mnt/sdb/qemu/blockdev.c:3580
  #7 0x564c0c8ed0cb (./x86_64-softmmu/qemu-system-x86_64+0x2fe10cb)  
/mnt/sdb/qemu/blockdev.c:3690
  #8 0x564c0c8ed177 (./x86_64-softmmu/qemu-system-x86_64+0x2fe1177)  
/mnt/sdb/qemu/blockdev.c:3704
  #9 0x564c0d316388 (./x86_64-softmmu/qemu-system-x86_64+0x3a0a388)  
/mnt/sdb/qemu/build/qapi/qapi-commands-block-core.c:439
  #10 0x564c0d7ff7fa (./x86_64-softmmu/qemu-system-x86_64+0x3ef37fa)  
/mnt/sdb/qemu/qapi/qmp-dispatch.c:132
  #11 0x564c0d7ffcb8 (./x86_64-softmmu/qemu-system-x86_64+0x3ef3cb8)  
/mnt/sdb/qemu/qapi/qmp-dispatch.c:175 (discriminator 4)
  #12 0x564c0d2704ef (./x86_64-softmmu/qemu-system-x86_64+0x39644ef)  
/mnt/sdb/qemu/monitor/qmp.c:145
  #13 0x564c0d2712de (./x86_64-softmmu/qemu-system-x86_64+0x39652de)  
/mnt/sdb/qemu/monitor/qmp.c:234 (discriminator 4)

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 block/backup-top.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/backup-top.c b/block/backup-top.c
index 818d3f26b4..d565f05520 100644
--- a/block/backup-top.c
+++ b/block/backup-top.c
@@ -196,6 +196,7 @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState 
*source,
 }
 
 top->total_sectors = source->total_sectors;
+g_free(top->opaque);
 top->opaque = state = g_new0(BDRVBackupTopState, 1);
 
 bdrv_ref(target);
-- 
2.21.0.windows.1

[PATCH 2/2] virtio-scsi: convert to new virtio_delete_queue

[pannengyuan]

From: Pan Nengyuan 

Use virtio_delete_queue to make it more clear.

Signed-off-by: Pan Nengyuan 
---
 hw/scsi/virtio-scsi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 858b3aaccb..d3af42ef92 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -945,10 +945,10 @@ void virtio_scsi_common_unrealize(DeviceState *dev)
 VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
 int i;
 
-virtio_del_queue(vdev, 0);
-virtio_del_queue(vdev, 1);
+virtio_delete_queue(vs->ctrl_vq);
+virtio_delete_queue(vs->event_vq);
 for (i = 0; i < vs->conf.num_queues; i++) {
-virtio_del_queue(vdev, i + 2);
+virtio_delete_queue(vs->cmd_vqs[i]);
 }
 g_free(vs->cmd_vqs);
 virtio_cleanup(vdev);
-- 
2.21.0.windows.1

[PATCH 1/2] virtio-scsi: delete vqs in unrealize to avoid memleaks

[pannengyuan]

From: Pan Nengyuan 

This patch fix memleaks when attaching/detaching virtio-scsi device, the
memory leak stack is as follow:

Direct leak of 21504 byte(s) in 3 object(s) allocated from:
  #0 0x7f491f2f2970 (/lib64/libasan.so.5+0xef970)  ??:?
  #1 0x7f491e94649d (/lib64/libglib-2.0.so.0+0x5249d)  ??:?
  #2 0x564d0f3919fa (./x86_64-softmmu/qemu-system-x86_64+0x2c3e9fa)  
/mnt/sdb/qemu/hw/virtio/virtio.c:2333
  #3 0x564d0f2eca55 (./x86_64-softmmu/qemu-system-x86_64+0x2b99a55)  
/mnt/sdb/qemu/hw/scsi/virtio-scsi.c:912
  #4 0x564d0f2ece7b (./x86_64-softmmu/qemu-system-x86_64+0x2b99e7b)  
/mnt/sdb/qemu/hw/scsi/virtio-scsi.c:924
  #5 0x564d0f39ee47 (./x86_64-softmmu/qemu-system-x86_64+0x2c4be47)  
/mnt/sdb/qemu/hw/virtio/virtio.c:3531
  #6 0x564d0f980224 (./x86_64-softmmu/qemu-system-x86_64+0x322d224)  
/mnt/sdb/qemu/hw/core/qdev.c:865

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 hw/scsi/virtio-scsi.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 4bc73a370e..858b3aaccb 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -943,7 +943,13 @@ void virtio_scsi_common_unrealize(DeviceState *dev)
 {
 VirtIODevice *vdev = VIRTIO_DEVICE(dev);
 VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
+int i;
 
+virtio_del_queue(vdev, 0);
+virtio_del_queue(vdev, 1);
+for (i = 0; i < vs->conf.num_queues; i++) {
+virtio_del_queue(vdev, i + 2);
+}
 g_free(vs->cmd_vqs);
 virtio_cleanup(vdev);
 }
-- 
2.21.0.windows.1

[PATCH 0/2] delete virtio queues in virtio_scsi_unrealize

[pannengyuan]

From: Pan Nengyuan 

This serie patch fix memleaks when detaching virtio-scsi device. 
1. use old virtio_del_queue to fix memleaks, it's easier for stable branches to 
merge.
   As the discussion in 
https://lists.nongnu.org/archive/html/qemu-devel/2020-01/msg02903.html

2. replace virtio_del_queue to virtio_delete_queue to make it more clear.

Pan Nengyuan (2):
  virtio-scsi: delete vqs in unrealize to avoid memleaks
  virtio-scsi: convert to new virtio_delete_queue

 hw/scsi/virtio-scsi.c | 6 ++
 1 file changed, 6 insertions(+)

-- 
2.21.0.windows.1

[PATCH v2 2/2] virtio-9p-device: convert to new virtio_delete_queue

[pannengyuan]

From: Pan Nengyuan 

Use virtio_delete_queue to make it more clear.

Signed-off-by: Pan Nengyuan 
---
Changes V2 to V1:
- replace virtio_del_queue to virtio_delete_queue to make it more clear.
---
 hw/9pfs/virtio-9p-device.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index 910dc5045e..b146387ae2 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -215,7 +215,7 @@ static void virtio_9p_device_unrealize(DeviceState *dev, 
Error **errp)
 V9fsVirtioState *v = VIRTIO_9P(dev);
 V9fsState *s = >state;
 
-virtio_del_queue(vdev, 0);
+virtio_delete_queue(v->vq);
 virtio_cleanup(vdev);
 v9fs_device_unrealize_common(s, errp);
 }
-- 
2.21.0.windows.1

[PATCH v2 0/2] fix memleaks in virtio_9p_device_unrealize

[pannengyuan]

From: Pan Nengyuan 

v1: fix memleaks in virtio_9p_device_unrealize
v2: split patch to make it easier for stable branches to merge

Pan Nengyuan (2):
  virtio-9p-device: fix memleak in virtio_9p_device_unrealize
  virtio-9p-device: convert to new virtio_delete_queue

 hw/9pfs/virtio-9p-device.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.21.0.windows.1

[PATCH v2 1/2] virtio-9p-device: fix memleak in virtio_9p_device_unrealize

[pannengyuan]

From: Pan Nengyuan 

v->vq forgot to cleanup in virtio_9p_device_unrealize, the memory leak
stack is as follow:

Direct leak of 14336 byte(s) in 2 object(s) allocated from:
  #0 0x7f819ae43970 (/lib64/libasan.so.5+0xef970)  ??:?
  #1 0x7f819872f49d (/lib64/libglib-2.0.so.0+0x5249d)  ??:?
  #2 0x55a3a58da624 (./x86_64-softmmu/qemu-system-x86_64+0x2c14624)  
/mnt/sdb/qemu/hw/virtio/virtio.c:2327
  #3 0x55a3a571bac7 (./x86_64-softmmu/qemu-system-x86_64+0x2a55ac7)  
/mnt/sdb/qemu/hw/9pfs/virtio-9p-device.c:209
  #4 0x55a3a58e7bc6 (./x86_64-softmmu/qemu-system-x86_64+0x2c21bc6)  
/mnt/sdb/qemu/hw/virtio/virtio.c:3504
  #5 0x55a3a5ebfb37 (./x86_64-softmmu/qemu-system-x86_64+0x31f9b37)  
/mnt/sdb/qemu/hw/core/qdev.c:876

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
Changes V2 to V1:
- use old function virtio_del_queue to make it easier for stable branch
to merge (suggested by Christian Schoenebeck)
---
 hw/9pfs/virtio-9p-device.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index b5a7c03f26..910dc5045e 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -215,6 +215,7 @@ static void virtio_9p_device_unrealize(DeviceState *dev, 
Error **errp)
 V9fsVirtioState *v = VIRTIO_9P(dev);
 V9fsState *s = >state;
 
+virtio_del_queue(vdev, 0);
 virtio_cleanup(vdev);
 v9fs_device_unrealize_common(s, errp);
 }
-- 
2.21.0.windows.1

[PATCH] block: fix memleaks in bdrv_refresh_filename

[pannengyuan]

From: Pan Nengyuan 

If we call the qmp 'query-block' while qemu is working on
'block-commit', it will cause memleaks, the memory leak stack is as
follow:

Indirect leak of 12360 byte(s) in 3 object(s) allocated from:
#0 0x7f80f0b6d970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
#1 0x7f80ee86049d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
#2 0x55ea95b5bb67 in qdict_new /mnt/sdb/qemu-4.2.0-rc0/qobject/qdict.c:29
#3 0x55ea956cd043 in bdrv_refresh_filename 
/mnt/sdb/qemu-4.2.0-rc0/block.c:6427
#4 0x55ea956cc950 in bdrv_refresh_filename 
/mnt/sdb/qemu-4.2.0-rc0/block.c:6399
#5 0x55ea956cc950 in bdrv_refresh_filename 
/mnt/sdb/qemu-4.2.0-rc0/block.c:6399
#6 0x55ea956cc950 in bdrv_refresh_filename 
/mnt/sdb/qemu-4.2.0-rc0/block.c:6399
#7 0x55ea958818ea in bdrv_block_device_info 
/mnt/sdb/qemu-4.2.0-rc0/block/qapi.c:56
#8 0x55ea958879de in bdrv_query_info 
/mnt/sdb/qemu-4.2.0-rc0/block/qapi.c:392
#9 0x55ea9588b58f in qmp_query_block 
/mnt/sdb/qemu-4.2.0-rc0/block/qapi.c:578
#10 0x55ea95567392 in qmp_marshal_query_block 
qapi/qapi-commands-block-core.c:95

Indirect leak of 4120 byte(s) in 1 object(s) allocated from:
#0 0x7f80f0b6d970 in __interceptor_calloc (/lib64/libasan.so.5+0xef970)
#1 0x7f80ee86049d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5249d)
#2 0x55ea95b5bb67 in qdict_new /mnt/sdb/qemu-4.2.0-rc0/qobject/qdict.c:29
#3 0x55ea956cd043 in bdrv_refresh_filename 
/mnt/sdb/qemu-4.2.0-rc0/block.c:6427
#4 0x55ea956cc950 in bdrv_refresh_filename 
/mnt/sdb/qemu-4.2.0-rc0/block.c:6399
#5 0x55ea956cc950 in bdrv_refresh_filename 
/mnt/sdb/qemu-4.2.0-rc0/block.c:6399
#6 0x55ea9569f301 in bdrv_backing_attach 
/mnt/sdb/qemu-4.2.0-rc0/block.c:1064
#7 0x55ea956a99dd in bdrv_replace_child_noperm 
/mnt/sdb/qemu-4.2.0-rc0/block.c:2283
#8 0x55ea956b9b53 in bdrv_replace_node /mnt/sdb/qemu-4.2.0-rc0/block.c:4196
#9 0x55ea956b9e49 in bdrv_append /mnt/sdb/qemu-4.2.0-rc0/block.c:4236
#10 0x55ea958c3472 in commit_start 
/mnt/sdb/qemu-4.2.0-rc0/block/commit.c:306
#11 0x55ea94b68ab0 in qmp_block_commit 
/mnt/sdb/qemu-4.2.0-rc0/blockdev.c:3459
#12 0x55ea9556a7a7 in qmp_marshal_block_commit 
qapi/qapi-commands-block-core.c:407

Fixes: bb808d5f5c0978828a974d547e6032402c339555
Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 block.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block.c b/block.c
index ecd09dbbfd..177eb8ade0 100644
--- a/block.c
+++ b/block.c
@@ -6419,6 +6419,7 @@ void bdrv_refresh_filename(BlockDriverState *bs)
 child->bs->exact_filename);
 pstrcpy(bs->filename, sizeof(bs->filename), child->bs->filename);
 
+qobject_unref(bs->full_open_options);
 bs->full_open_options = qobject_ref(child->bs->full_open_options);
 
 return;
-- 
2.21.0.windows.1

[PATCH v2] vhost-vsock: delete vqs in vhost_vsock_unrealize to avoid memleaks

[pannengyuan]

From: Pan Nengyuan 

Receive/transmit/event vqs forgot to cleanup in vhost_vsock_unrealize. This
patch save receive/transmit vq pointer in realize() and cleanup vqs
through those vq pointers in unrealize(). The leak stack is as follow:

Direct leak of 21504 byte(s) in 3 object(s) allocated from:
  #0 0x7f86a1356970 (/lib64/libasan.so.5+0xef970)  ??:?
  #1 0x7f86a09aa49d (/lib64/libglib-2.0.so.0+0x5249d)  ??:?
  #2 0x5604852f85ca (./x86_64-softmmu/qemu-system-x86_64+0x2c3e5ca)  
/mnt/sdb/qemu/hw/virtio/virtio.c:2333
  #3 0x560485356208 (./x86_64-softmmu/qemu-system-x86_64+0x2c9c208)  
/mnt/sdb/qemu/hw/virtio/vhost-vsock.c:339
  #4 0x560485305a17 (./x86_64-softmmu/qemu-system-x86_64+0x2c4ba17)  
/mnt/sdb/qemu/hw/virtio/virtio.c:3531
  #5 0x5604858e6b65 (./x86_64-softmmu/qemu-system-x86_64+0x322cb65)  
/mnt/sdb/qemu/hw/core/qdev.c:865
  #6 0x5604861e6c41 (./x86_64-softmmu/qemu-system-x86_64+0x3b2cc41)  
/mnt/sdb/qemu/qom/object.c:2102

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
Changes V2 to V1:
- delete virtqueues after vhost cleanup to avoid use-after-free
- aslo delete virtqueues in the error path of realize()
---
 hw/virtio/vhost-vsock.c | 12 ++--
 include/hw/virtio/vhost-vsock.h |  2 ++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index f5744363a8..b6cee479bb 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -335,8 +335,10 @@ static void vhost_vsock_device_realize(DeviceState *dev, 
Error **errp)
 sizeof(struct virtio_vsock_config));
 
 /* Receive and transmit queues belong to vhost */
-virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE, vhost_vsock_handle_output);
-virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE, vhost_vsock_handle_output);
+vsock->recv_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
+  vhost_vsock_handle_output);
+vsock->trans_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
+   vhost_vsock_handle_output);
 
 /* The event queue belongs to QEMU */
 vsock->event_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
@@ -363,6 +365,9 @@ static void vhost_vsock_device_realize(DeviceState *dev, 
Error **errp)
 err_vhost_dev:
 vhost_dev_cleanup(>vhost_dev);
 err_virtio:
+virtio_delete_queue(vsock->recv_vq);
+virtio_delete_queue(vsock->trans_vq);
+virtio_delete_queue(vsock->event_vq);
 virtio_cleanup(vdev);
 close(vhostfd);
 return;
@@ -379,6 +384,9 @@ static void vhost_vsock_device_unrealize(DeviceState *dev, 
Error **errp)
 vhost_vsock_set_status(vdev, 0);
 
 vhost_dev_cleanup(>vhost_dev);
+virtio_delete_queue(vsock->recv_vq);
+virtio_delete_queue(vsock->trans_vq);
+virtio_delete_queue(vsock->event_vq);
 virtio_cleanup(vdev);
 }
 
diff --git a/include/hw/virtio/vhost-vsock.h b/include/hw/virtio/vhost-vsock.h
index d509d67c4a..bc5a988ee5 100644
--- a/include/hw/virtio/vhost-vsock.h
+++ b/include/hw/virtio/vhost-vsock.h
@@ -33,6 +33,8 @@ typedef struct {
 struct vhost_virtqueue vhost_vqs[2];
 struct vhost_dev vhost_dev;
 VirtQueue *event_vq;
+VirtQueue *recv_vq;
+VirtQueue *trans_vq;
 QEMUTimer *post_load_timer;
 
 /*< public >*/
-- 
2.21.0.windows.1

[PATCH v2] scsi-disk: define props in scsi_block_disk to avoid memleaks

[pannengyuan]

From: Pan Nengyuan 

scsi_block_realize() use scsi_realize() to init some props, but
these props is not defined in scsi_block_disk_properties, so they will
not be freed.

This patch defines these prop in scsi_block_disk_properties and aslo
calls scsi_unrealize to avoid memleaks, the leak stack as
follow(it's easy to reproduce by attaching/detaching scsi-block-disks):

=
==qemu-system-x86_64==32195==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 57 byte(s) in 3 object(s) allocated from:
  #0 0x7f19f8bed768 (/lib64/libasan.so.5+0xef768)  ??:?
  #1 0x7f19f64d9445 (/lib64/libglib-2.0.so.0+0x52445)  ??:?
  #2 0x7f19f64f2d92 (/lib64/libglib-2.0.so.0+0x6bd92)  ??:?
  #3 0x55975366e596 (qemu-system-x86_64+0x35c0596)  
/mnt/sdb/qemu/hw/scsi/scsi-disk.c:2399
  #4 0x559753671201 (emu-system-x86_64+0x35c3201)  
/mnt/sdb/qemu/hw/scsi/scsi-disk.c:2681
  #5 0x559753687e3e (qemu-system-x86_64+0x35d9e3e)  
/mnt/sdb/qemu/hw/scsi/scsi-bus.c:58
  #6 0x55975368ac44 (qemu-system-x86_64+0x35dcc44)  
/mnt/sdb/qemu/hw/scsi/scsi-bus.c:216
  #7 0x5597532a7840 (qemu-system-x86_64+0x31f9840)  
/mnt/sdb/qemu/hw/core/qdev.c:876

Direct leak of 15 byte(s) in 3 object(s) allocated from:
  #0 0x7f19f8bed768 (/lib64/libasan.so.5+0xef768)  ??:?
  #1 0x7f19f64d9445 (/lib64/libglib-2.0.so.0+0x52445)  ??:?
  #2 0x7f19f64f2d92 (/lib64/libglib-2.0.so.0+0x6bd92)  ??:?
  #3 0x55975366e06f (qemu-system-x86_64+0x35c006f)  
/mnt/sdb/qemu/hw/scsi/scsi-disk.c:2388
  #4 0x559753671201 (qemu-system-x86_64+0x35c3201)  
/mnt/sdb/qemu/hw/scsi/scsi-disk.c:2681
  #5 0x559753687e3e (qemu-system-x86_64+0x35d9e3e)  
/mnt/sdb/qemu/hw/scsi/scsi-bus.c:58
  #6 0x55975368ac44 (qemu-system-x86_64+0x35dcc44)  
/mnt/sdb/qemu/hw/scsi/scsi-bus.c:216

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
Changes V2 to V1:
- move 'drive' to the front to keep the original props's order.
---
 hw/scsi/scsi-disk.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index e44c61eeb4..a1194b9fa7 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2981,7 +2981,6 @@ static const TypeInfo scsi_disk_base_info = {
 };
 
 #define DEFINE_SCSI_DISK_PROPERTIES()   \
-DEFINE_PROP_DRIVE_IOTHREAD("drive", SCSIDiskState, qdev.conf.blk),  \
 DEFINE_BLOCK_PROPERTIES_BASE(SCSIDiskState, qdev.conf), \
 DEFINE_BLOCK_ERROR_PROPERTIES(SCSIDiskState, qdev.conf),\
 DEFINE_PROP_STRING("ver", SCSIDiskState, version),  \
@@ -2992,6 +2991,7 @@ static const TypeInfo scsi_disk_base_info = {
 
 
 static Property scsi_hd_properties[] = {
+DEFINE_PROP_DRIVE_IOTHREAD("drive", SCSIDiskState, qdev.conf.blk),
 DEFINE_SCSI_DISK_PROPERTIES(),
 DEFINE_PROP_BIT("removable", SCSIDiskState, features,
 SCSI_DISK_F_REMOVABLE, false),
@@ -3047,6 +3047,7 @@ static const TypeInfo scsi_hd_info = {
 };
 
 static Property scsi_cd_properties[] = {
+DEFINE_PROP_DRIVE_IOTHREAD("drive", SCSIDiskState, qdev.conf.blk),
 DEFINE_SCSI_DISK_PROPERTIES(),
 DEFINE_PROP_UINT64("wwn", SCSIDiskState, qdev.wwn, 0),
 DEFINE_PROP_UINT64("port_wwn", SCSIDiskState, qdev.port_wwn, 0),
@@ -3079,9 +3080,8 @@ static const TypeInfo scsi_cd_info = {
 
 #ifdef __linux__
 static Property scsi_block_properties[] = {
-DEFINE_BLOCK_ERROR_PROPERTIES(SCSIDiskState, qdev.conf), \
+DEFINE_SCSI_DISK_PROPERTIES(),
 DEFINE_PROP_DRIVE("drive", SCSIDiskState, qdev.conf.blk),
-DEFINE_PROP_BOOL("share-rw", SCSIDiskState, qdev.conf.share_rw, false),
 DEFINE_PROP_UINT16("rotation_rate", SCSIDiskState, rotation_rate, 0),
 DEFINE_PROP_UINT64("max_unmap_size", SCSIDiskState, max_unmap_size,
DEFAULT_MAX_UNMAP_SIZE),
@@ -3099,6 +3099,7 @@ static void scsi_block_class_initfn(ObjectClass *klass, 
void *data)
 SCSIDiskClass *sdc = SCSI_DISK_BASE_CLASS(klass);
 
 sc->realize  = scsi_block_realize;
+sc->unrealize= scsi_unrealize;
 sc->alloc_req= scsi_block_new_request;
 sc->parse_cdb= scsi_block_parse_cdb;
 sdc->dma_readv   = scsi_block_dma_readv;
@@ -3118,6 +3119,7 @@ static const TypeInfo scsi_block_info = {
 #endif
 
 static Property scsi_disk_properties[] = {
+DEFINE_PROP_DRIVE_IOTHREAD("drive", SCSIDiskState, qdev.conf.blk),
 DEFINE_SCSI_DISK_PROPERTIES(),
 DEFINE_PROP_BIT("removable", SCSIDiskState, features,
 SCSI_DISK_F_REMOVABLE, false),
-- 
2.21.0.windows.1

[PATCH] vhost-vsock: delete vqs in vhost_vsock_unrealize to avoid memleaks

[pannengyuan]

From: Pan Nengyuan 

Receive/transmit/event vqs forgot to cleanup in vhost_vsock_unrealize. This
patch save receive/transmit vq pointer in realize() and cleanup vqs
through those vq pointers in unrealize(). The leak stack is as follow:

Direct leak of 21504 byte(s) in 3 object(s) allocated from:
  #0 0x7f86a1356970 (/lib64/libasan.so.5+0xef970)  ??:?
  #1 0x7f86a09aa49d (/lib64/libglib-2.0.so.0+0x5249d)  ??:?
  #2 0x5604852f85ca (./x86_64-softmmu/qemu-system-x86_64+0x2c3e5ca)  
/mnt/sdb/qemu/hw/virtio/virtio.c:2333
  #3 0x560485356208 (./x86_64-softmmu/qemu-system-x86_64+0x2c9c208)  
/mnt/sdb/qemu/hw/virtio/vhost-vsock.c:339
  #4 0x560485305a17 (./x86_64-softmmu/qemu-system-x86_64+0x2c4ba17)  
/mnt/sdb/qemu/hw/virtio/virtio.c:3531
  #5 0x5604858e6b65 (./x86_64-softmmu/qemu-system-x86_64+0x322cb65)  
/mnt/sdb/qemu/hw/core/qdev.c:865
  #6 0x5604861e6c41 (./x86_64-softmmu/qemu-system-x86_64+0x3b2cc41)  
/mnt/sdb/qemu/qom/object.c:2102

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 hw/virtio/vhost-vsock.c | 9 +++--
 include/hw/virtio/vhost-vsock.h | 2 ++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/vhost-vsock.c b/hw/virtio/vhost-vsock.c
index f5744363a8..896c0174c1 100644
--- a/hw/virtio/vhost-vsock.c
+++ b/hw/virtio/vhost-vsock.c
@@ -335,8 +335,10 @@ static void vhost_vsock_device_realize(DeviceState *dev, 
Error **errp)
 sizeof(struct virtio_vsock_config));
 
 /* Receive and transmit queues belong to vhost */
-virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE, vhost_vsock_handle_output);
-virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE, vhost_vsock_handle_output);
+vsock->recv_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
+  vhost_vsock_handle_output);
+vsock->trans_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
+   vhost_vsock_handle_output);
 
 /* The event queue belongs to QEMU */
 vsock->event_vq = virtio_add_queue(vdev, VHOST_VSOCK_QUEUE_SIZE,
@@ -378,6 +380,9 @@ static void vhost_vsock_device_unrealize(DeviceState *dev, 
Error **errp)
 /* This will stop vhost backend if appropriate. */
 vhost_vsock_set_status(vdev, 0);
 
+virtio_delete_queue(vsock->recv_vq);
+virtio_delete_queue(vsock->trans_vq);
+virtio_delete_queue(vsock->event_vq);
 vhost_dev_cleanup(>vhost_dev);
 virtio_cleanup(vdev);
 }
diff --git a/include/hw/virtio/vhost-vsock.h b/include/hw/virtio/vhost-vsock.h
index d509d67c4a..bc5a988ee5 100644
--- a/include/hw/virtio/vhost-vsock.h
+++ b/include/hw/virtio/vhost-vsock.h
@@ -33,6 +33,8 @@ typedef struct {
 struct vhost_virtqueue vhost_vqs[2];
 struct vhost_dev vhost_dev;
 VirtQueue *event_vq;
+VirtQueue *recv_vq;
+VirtQueue *trans_vq;
 QEMUTimer *post_load_timer;
 
 /*< public >*/
-- 
2.21.0.windows.1

[PATCH] virtio-9p-device: fix memleak in virtio_9p_device_unrealize

[pannengyuan]

From: Pan Nengyuan 

v->vq forgot to cleanup in virtio_9p_device_unrealize, the memory leak
stack is as follow:

Direct leak of 14336 byte(s) in 2 object(s) allocated from:
  #0 0x7f819ae43970 (/lib64/libasan.so.5+0xef970)  ??:?
  #1 0x7f819872f49d (/lib64/libglib-2.0.so.0+0x5249d)  ??:?
  #2 0x55a3a58da624 (./x86_64-softmmu/qemu-system-x86_64+0x2c14624)  
/mnt/sdb/qemu/hw/virtio/virtio.c:2327
  #3 0x55a3a571bac7 (./x86_64-softmmu/qemu-system-x86_64+0x2a55ac7)  
/mnt/sdb/qemu/hw/9pfs/virtio-9p-device.c:209
  #4 0x55a3a58e7bc6 (./x86_64-softmmu/qemu-system-x86_64+0x2c21bc6)  
/mnt/sdb/qemu/hw/virtio/virtio.c:3504
  #5 0x55a3a5ebfb37 (./x86_64-softmmu/qemu-system-x86_64+0x31f9b37)  
/mnt/sdb/qemu/hw/core/qdev.c:876

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 hw/9pfs/virtio-9p-device.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index b5a7c03f26..b146387ae2 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -215,6 +215,7 @@ static void virtio_9p_device_unrealize(DeviceState *dev, 
Error **errp)
 V9fsVirtioState *v = VIRTIO_9P(dev);
 V9fsState *s = >state;
 
+virtio_delete_queue(v->vq);
 virtio_cleanup(vdev);
 v9fs_device_unrealize_common(s, errp);
 }
-- 
2.21.0.windows.1

[PATCH] scsi-disk: define props in scsi_block_disk to avoid memleaks

[pannengyuan]

From: Pan Nengyuan 

scsi_block_realize() use scsi_realize() to init some props, but
these props is not defined in scsi_block_disk_properties, so they will
not be freed.

This patch defines these prop in scsi_block_disk_properties and aslo
calls scsi_unrealize to avoid memleaks, the leak stack as
follow(it's easy to reproduce by attaching/detaching scsi-block-disks):

=
==qemu-system-x86_64==32195==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 57 byte(s) in 3 object(s) allocated from:
  #0 0x7f19f8bed768 (/lib64/libasan.so.5+0xef768)  ??:?
  #1 0x7f19f64d9445 (/lib64/libglib-2.0.so.0+0x52445)  ??:?
  #2 0x7f19f64f2d92 (/lib64/libglib-2.0.so.0+0x6bd92)  ??:?
  #3 0x55975366e596 (qemu-system-x86_64+0x35c0596)  
/mnt/sdb/qemu/hw/scsi/scsi-disk.c:2399
  #4 0x559753671201 (emu-system-x86_64+0x35c3201)  
/mnt/sdb/qemu/hw/scsi/scsi-disk.c:2681
  #5 0x559753687e3e (qemu-system-x86_64+0x35d9e3e)  
/mnt/sdb/qemu/hw/scsi/scsi-bus.c:58
  #6 0x55975368ac44 (qemu-system-x86_64+0x35dcc44)  
/mnt/sdb/qemu/hw/scsi/scsi-bus.c:216
  #7 0x5597532a7840 (qemu-system-x86_64+0x31f9840)  
/mnt/sdb/qemu/hw/core/qdev.c:876

Direct leak of 15 byte(s) in 3 object(s) allocated from:
  #0 0x7f19f8bed768 (/lib64/libasan.so.5+0xef768)  ??:?
  #1 0x7f19f64d9445 (/lib64/libglib-2.0.so.0+0x52445)  ??:?
  #2 0x7f19f64f2d92 (/lib64/libglib-2.0.so.0+0x6bd92)  ??:?
  #3 0x55975366e06f (qemu-system-x86_64+0x35c006f)  
/mnt/sdb/qemu/hw/scsi/scsi-disk.c:2388
  #4 0x559753671201 (qemu-system-x86_64+0x35c3201)  
/mnt/sdb/qemu/hw/scsi/scsi-disk.c:2681
  #5 0x559753687e3e (qemu-system-x86_64+0x35d9e3e)  
/mnt/sdb/qemu/hw/scsi/scsi-bus.c:58
  #6 0x55975368ac44 (qemu-system-x86_64+0x35dcc44)  
/mnt/sdb/qemu/hw/scsi/scsi-bus.c:216

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 hw/scsi/scsi-disk.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index e44c61eeb4..caec99ae20 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2981,7 +2981,6 @@ static const TypeInfo scsi_disk_base_info = {
 };
 
 #define DEFINE_SCSI_DISK_PROPERTIES()   \
-DEFINE_PROP_DRIVE_IOTHREAD("drive", SCSIDiskState, qdev.conf.blk),  \
 DEFINE_BLOCK_PROPERTIES_BASE(SCSIDiskState, qdev.conf), \
 DEFINE_BLOCK_ERROR_PROPERTIES(SCSIDiskState, qdev.conf),\
 DEFINE_PROP_STRING("ver", SCSIDiskState, version),  \
@@ -2993,6 +2992,7 @@ static const TypeInfo scsi_disk_base_info = {
 
 static Property scsi_hd_properties[] = {
 DEFINE_SCSI_DISK_PROPERTIES(),
+DEFINE_PROP_DRIVE_IOTHREAD("drive", SCSIDiskState, qdev.conf.blk),
 DEFINE_PROP_BIT("removable", SCSIDiskState, features,
 SCSI_DISK_F_REMOVABLE, false),
 DEFINE_PROP_BIT("dpofua", SCSIDiskState, features,
@@ -3048,6 +3048,7 @@ static const TypeInfo scsi_hd_info = {
 
 static Property scsi_cd_properties[] = {
 DEFINE_SCSI_DISK_PROPERTIES(),
+DEFINE_PROP_DRIVE_IOTHREAD("drive", SCSIDiskState, qdev.conf.blk),
 DEFINE_PROP_UINT64("wwn", SCSIDiskState, qdev.wwn, 0),
 DEFINE_PROP_UINT64("port_wwn", SCSIDiskState, qdev.port_wwn, 0),
 DEFINE_PROP_UINT16("port_index", SCSIDiskState, port_index, 0),
@@ -3079,9 +3080,8 @@ static const TypeInfo scsi_cd_info = {
 
 #ifdef __linux__
 static Property scsi_block_properties[] = {
-DEFINE_BLOCK_ERROR_PROPERTIES(SCSIDiskState, qdev.conf), \
+DEFINE_SCSI_DISK_PROPERTIES(),
 DEFINE_PROP_DRIVE("drive", SCSIDiskState, qdev.conf.blk),
-DEFINE_PROP_BOOL("share-rw", SCSIDiskState, qdev.conf.share_rw, false),
 DEFINE_PROP_UINT16("rotation_rate", SCSIDiskState, rotation_rate, 0),
 DEFINE_PROP_UINT64("max_unmap_size", SCSIDiskState, max_unmap_size,
DEFAULT_MAX_UNMAP_SIZE),
@@ -3099,6 +3099,7 @@ static void scsi_block_class_initfn(ObjectClass *klass, 
void *data)
 SCSIDiskClass *sdc = SCSI_DISK_BASE_CLASS(klass);
 
 sc->realize  = scsi_block_realize;
+sc->unrealize= scsi_unrealize;
 sc->alloc_req= scsi_block_new_request;
 sc->parse_cdb= scsi_block_parse_cdb;
 sdc->dma_readv   = scsi_block_dma_readv;
@@ -3119,6 +3120,7 @@ static const TypeInfo scsi_block_info = {
 
 static Property scsi_disk_properties[] = {
 DEFINE_SCSI_DISK_PROPERTIES(),
+DEFINE_PROP_DRIVE_IOTHREAD("drive", SCSIDiskState, qdev.conf.blk),
 DEFINE_PROP_BIT("removable", SCSIDiskState, features,
 SCSI_DISK_F_REMOVABLE, false),
 DEFINE_PROP_BIT("dpofua", SCSIDiskState, features,
-- 
2.21.0.windows.1

[PATCH v2 1/2] vl: Don't mismatch g_strsplit()/g_free()

[pannengyuan]

From: Pan Nengyuan 

It's a mismatch between g_strsplit and g_free, it will cause a memory leak as 
follow:

[root@localhost]# ./aarch64-softmmu/qemu-system-aarch64 -accel help
Accelerators supported in QEMU binary:
tcg
kvm
=
==1207900==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 8 byte(s) in 2 object(s) allocated from:
#0 0xfffd700231cb in __interceptor_malloc (/lib64/libasan.so.4+0xd31cb)
#1 0xfffd6ec57163 in g_malloc (/lib64/libglib-2.0.so.0+0x57163)
#2 0xfffd6ec724d7 in g_strndup (/lib64/libglib-2.0.so.0+0x724d7)
#3 0xfffd6ec73d3f in g_strsplit (/lib64/libglib-2.0.so.0+0x73d3f)
#4 0xaaab66be5077 in main /mnt/sdc/qemu-master/qemu-4.2.0-rc0/vl.c:3517
#5 0xfffd6e140b9f in __libc_start_main (/lib64/libc.so.6+0x20b9f)
#6 0xaaab66bf0f53  (./build/aarch64-softmmu/qemu-system-aarch64+0x8a0f53)

Direct leak of 2 byte(s) in 2 object(s) allocated from:
#0 0xfffd700231cb in __interceptor_malloc (/lib64/libasan.so.4+0xd31cb)
#1 0xfffd6ec57163 in g_malloc (/lib64/libglib-2.0.so.0+0x57163)
#2 0xfffd6ec7243b in g_strdup (/lib64/libglib-2.0.so.0+0x7243b)
#3 0xfffd6ec73e6f in g_strsplit (/lib64/libglib-2.0.so.0+0x73e6f)
#4 0xaaab66be5077 in main /mnt/sdc/qemu-master/qemu-4.2.0-rc0/vl.c:3517
#5 0xfffd6e140b9f in __libc_start_main (/lib64/libc.so.6+0x20b9f)
#6 0xaaab66bf0f53  (./build/aarch64-softmmu/qemu-system-aarch64+0x8a0f53)

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
Changes v2 to v1:
- fix another on in qga/main.c
---
 vl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index 86474a55c9..2fa5cb3b9a 100644
--- a/vl.c
+++ b/vl.c
@@ -3476,7 +3476,7 @@ int main(int argc, char **argv, char **envp)
 gchar **optname = g_strsplit(typename,
  ACCEL_CLASS_SUFFIX, 
0);
 printf("%s\n", optname[0]);
-g_free(optname);
+g_strfreev(optname);
 }
 g_free(typename);
 }
-- 
2.21.0.windows.1

[PATCH v2 0/2] fix mismatches between g_strsplit and g_free

[pannengyuan]

From: Pan Nengyuan 

v1: fix a mismatch in vl.c
v2: fix mismatch in vl.c and qga/main.c

Pan Nengyuan (2):
  vl: Don't mismatch g_strsplit()/g_free()
  qga/main: Don't mismatch g_strsplit/g_free in split_list()

 qga/main.c | 2 +-
 vl.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.21.0.windows.1

[PATCH v2 2/2] qga/main: Don't mismatch g_strsplit/g_free in split_list()

[pannengyuan]

From: Pan Nengyuan 

fix a mismatch between g_strsplit and g_free

Reported-by: Laurent Vivier 
Signed-off-by: Pan Nengyuan 
---
Changes v2 to v1:
- fix a mismatch in qga/main.c
---
 qga/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/main.c b/qga/main.c
index c35c2a2120..a72ae074f4 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -933,7 +933,7 @@ static GList *split_list(const gchar *str, const gchar 
*delim)
 for (i = 0; strv[i]; i++) {
 list = g_list_prepend(list, strv[i]);
 }
-g_free(strv);
+g_strfreev(strv);
 
 return list;
 }
-- 
2.21.0.windows.1

[PATCH] vl: Don't mismatch g_strsplit()/g_free()

[pannengyuan]

From: Pan Nengyuan 

It's a mismatch between g_strsplit and g_free, it will cause a memory leak as 
follow:

[root@localhost]# ./aarch64-softmmu/qemu-system-aarch64 -accel help
Accelerators supported in QEMU binary:
tcg
kvm
=
==1207900==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 8 byte(s) in 2 object(s) allocated from:
#0 0xfffd700231cb in __interceptor_malloc (/lib64/libasan.so.4+0xd31cb)
#1 0xfffd6ec57163 in g_malloc (/lib64/libglib-2.0.so.0+0x57163)
#2 0xfffd6ec724d7 in g_strndup (/lib64/libglib-2.0.so.0+0x724d7)
#3 0xfffd6ec73d3f in g_strsplit (/lib64/libglib-2.0.so.0+0x73d3f)
#4 0xaaab66be5077 in main /mnt/sdc/qemu-master/qemu-4.2.0-rc0/vl.c:3517
#5 0xfffd6e140b9f in __libc_start_main (/lib64/libc.so.6+0x20b9f)
#6 0xaaab66bf0f53  (./build/aarch64-softmmu/qemu-system-aarch64+0x8a0f53)

Direct leak of 2 byte(s) in 2 object(s) allocated from:
#0 0xfffd700231cb in __interceptor_malloc (/lib64/libasan.so.4+0xd31cb)
#1 0xfffd6ec57163 in g_malloc (/lib64/libglib-2.0.so.0+0x57163)
#2 0xfffd6ec7243b in g_strdup (/lib64/libglib-2.0.so.0+0x7243b)
#3 0xfffd6ec73e6f in g_strsplit (/lib64/libglib-2.0.so.0+0x73e6f)
#4 0xaaab66be5077 in main /mnt/sdc/qemu-master/qemu-4.2.0-rc0/vl.c:3517
#5 0xfffd6e140b9f in __libc_start_main (/lib64/libc.so.6+0x20b9f)
#6 0xaaab66bf0f53  (./build/aarch64-softmmu/qemu-system-aarch64+0x8a0f53)

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 vl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/vl.c b/vl.c
index 86474a55c9..2fa5cb3b9a 100644
--- a/vl.c
+++ b/vl.c
@@ -3476,7 +3476,7 @@ int main(int argc, char **argv, char **envp)
 gchar **optname = g_strsplit(typename,
  ACCEL_CLASS_SUFFIX, 
0);
 printf("%s\n", optname[0]);
-g_free(optname);
+g_strfreev(optname);
 }
 g_free(typename);
 }
-- 
2.21.0.windows.1

[PATCH v2] nbd: fix uninitialized variable warning

[pannengyuan]

From: Pan Nengyuan 

Fixes:
/mnt/sdb/qemu/nbd/server.c: In function 'nbd_handle_request':
/mnt/sdb/qemu/nbd/server.c:2313:9: error: 'ret' may be used uninitialized in 
this function [-Werror=maybe-uninitialized]
 int ret;

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
Changes v2 to v1:
- change 'if(client->export_meta.bitmap)' into 'else' to fix uninitialized 
warning and clean up pointless code (suggested by Eric Blake)
---
 nbd/server.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index 24ebc1a805..87fcd2e7bf 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2384,20 +2384,12 @@ static coroutine_fn int nbd_handle_request(NBDClient 
*client,
!client->export_meta.bitmap,
NBD_META_ID_BASE_ALLOCATION,
errp);
-if (ret < 0) {
-return ret;
-}
-}
-
-if (client->export_meta.bitmap) {
+} else {  /* client->export_meta.bitmap */
 ret = nbd_co_send_bitmap(client, request->handle,
  client->exp->export_bitmap,
  request->from, request->len,
  dont_fragment,
  true, NBD_META_ID_DIRTY_BITMAP, errp);
-if (ret < 0) {
-return ret;
-}
 }
 
 return ret;
-- 
2.21.0.windows.1

[PATCH v2] arm/translate-a64: fix uninitialized variable warning

[pannengyuan]

From: Pan Nengyuan 

Fixes:
target/arm/translate-a64.c: In function 'disas_crypto_three_reg_sha512':
target/arm/translate-a64.c:13625:9: error: 'genfn' may be used uninitialized in 
this function [-Werror=maybe-uninitialized]
genfn(tcg_rd_ptr, tcg_rn_ptr, tcg_rm_ptr);
^
qemu/target/arm/translate-a64.c:13609:8: error: 'feature' may be used 
uninitialized in this function [-Werror=maybe-uninitialized]
if (!feature) {

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
Changes v2 to v1:
- add a default label to fix uninitialized warnings(suggested by Richard 
Henderson)
---
 target/arm/translate-a64.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index d4bebbe629..63a3d26687 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -13585,6 +13585,8 @@ static void disas_crypto_three_reg_sha512(DisasContext 
*s, uint32_t insn)
 feature = dc_isar_feature(aa64_sha3, s);
 genfn = NULL;
 break;
+default:
+g_assert_not_reached();
 }
 } else {
 switch (opcode) {
-- 
2.21.0.windows.1

[PATCH] arm/translate-a64: fix uninitialized variable warning

[pannengyuan]

From: Pan Nengyuan 

Fixes:
target/arm/translate-a64.c: In function 'disas_crypto_three_reg_sha512':
target/arm/translate-a64.c:13625:9: error: 'genfn' may be used uninitialized in 
this function [-Werror=maybe-uninitialized]
genfn(tcg_rd_ptr, tcg_rn_ptr, tcg_rm_ptr);
^
qemu/target/arm/translate-a64.c:13609:8: error: 'feature' may be used 
uninitialized in this function [-Werror=maybe-uninitialized]
if (!feature) {

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Cc: Peter Maydell  
---
 target/arm/translate-a64.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index d4bebbe629..211e3d813f 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -13564,8 +13564,8 @@ static void disas_crypto_three_reg_sha512(DisasContext 
*s, uint32_t insn)
 int rm = extract32(insn, 16, 5);
 int rn = extract32(insn, 5, 5);
 int rd = extract32(insn, 0, 5);
-bool feature;
-CryptoThreeOpFn *genfn;
+bool feature = false;
+CryptoThreeOpFn *genfn = NULL;
 
 if (o == 0) {
 switch (opcode) {
-- 
2.21.0.windows.1

[PATCH] nbd: fix uninitialized variable warning

[pannengyuan]

From: Pan Nengyuan 

Fixes:
/mnt/sdb/qemu/nbd/server.c: In function 'nbd_handle_request':
/mnt/sdb/qemu/nbd/server.c:2313:9: error: 'ret' may be used uninitialized in 
this function [-Werror=maybe-uninitialized]
 int ret;

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 nbd/server.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nbd/server.c b/nbd/server.c
index 24ebc1a805..7eb3de0842 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2310,7 +2310,7 @@ static coroutine_fn int nbd_handle_request(NBDClient 
*client,
NBDRequest *request,
uint8_t *data, Error **errp)
 {
-int ret;
+int ret = 0;
 int flags;
 NBDExport *exp = client->exp;
 char *msg;
-- 
2.21.0.windows.1

[PATCH] util/module: fix a memory leak

[pannengyuan]

From: Pan Nengyuan 

spotted by ASAN

Fixes: 81d8ccb1bea4fb9eaaf4c8e30bd4021180a9a39f
Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 util/module.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/util/module.c b/util/module.c
index e9fe3e5..8c5315a 100644
--- a/util/module.c
+++ b/util/module.c
@@ -214,6 +214,7 @@ bool module_load_one(const char *prefix, const char 
*lib_name)
 
 if (!success) {
 g_hash_table_remove(loaded_modules, module_name);
+g_free(module_name);
 }
 
 for (i = 0; i < n_dirs; i++) {
-- 
2.7.2.windows.1

[PATCH V2] vhost-user-test: fix a memory leak

[pannengyuan]

From: Pan Nengyuan 

Spotted by ASAN.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
Changes V2 to V1:
- use a "goto cleanup", instead of duplicating the "free" functions.
- free "dest_cmdline" at the end.
---
 tests/vhost-user-test.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 91ea373..dcb8617 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -717,7 +717,7 @@ static void test_migrate(void *obj, void *arg, 
QGuestAllocator *alloc)
 guint64 size;
 
 if (!wait_for_fds(s)) {
-return;
+goto cleanup;
 }
 
 size = get_log_size(s);
@@ -776,8 +776,11 @@ static void test_migrate(void *obj, void *arg, 
QGuestAllocator *alloc)
 g_source_unref(source);
 
 qtest_quit(to);
+
+ cleanup:
 test_server_free(dest);
 g_free(uri);
+g_string_free(dest_cmdline, true);
 }
 
 static void wait_for_rings_started(TestServer *s, size_t count)
-- 
2.7.2.windows.1

[PATCH] vhost-user-test: fix a memory leak

[pannengyuan]

From: Pan Nengyuan 

Spotted by ASAN.

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 tests/vhost-user-test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 91ea373..54be931 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -717,6 +717,8 @@ static void test_migrate(void *obj, void *arg, 
QGuestAllocator *alloc)
 guint64 size;
 
 if (!wait_for_fds(s)) {
+g_free(uri);
+test_server_free(dest);
 return;
 }
 
-- 
2.7.2.windows.1

[PATCH] riscv/sifive_u: fix a memory leak in soc_realize()

[pannengyuan]

From: Pan Nengyuan 

Fix a minor memory leak in riscv_sifive_u_soc_realize()

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
 hw/riscv/sifive_u.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 0140e95..0e12b3c 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -542,6 +542,7 @@ static void riscv_sifive_u_soc_realize(DeviceState *dev, 
Error **errp)
 SIFIVE_U_PLIC_CONTEXT_BASE,
 SIFIVE_U_PLIC_CONTEXT_STRIDE,
 memmap[SIFIVE_U_PLIC].size);
+g_free(plic_hart_config);
 sifive_uart_create(system_memory, memmap[SIFIVE_U_UART0].base,
 serial_hd(0), qdev_get_gpio_in(DEVICE(s->plic), SIFIVE_U_UART0_IRQ));
 sifive_uart_create(system_memory, memmap[SIFIVE_U_UART1].base,
-- 
2.7.2.windows.1

[PATCH v3 3/3] virtio-serial-bus: fix memory leak while attach virtio-serial-bus

[pannengyuan]

From: Pan Nengyuan 

ivqs/ovqs/c_ivq/c_ovq forgot to cleanup in
virtio_serial_device_unrealize, the memory leak stack is as below:

Direct leak of 1290240 byte(s) in 180 object(s) allocated from:
#0 0x7fc9bfc27560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
#1 0x7fc9bed6f015 in g_malloc0 (/usr/lib64/libglib-2.0.so.0+0x50015)
#2 0x5650e02b83e7 in virtio_add_queue hw/virtio/virtio.c:2327
#3 0x5650e02847b5 in virtio_serial_device_realize 
hw/char/virtio-serial-bus.c:1089
#4 0x5650e02b56a7 in virtio_device_realize hw/virtio/virtio.c:3504
#5 0x5650e03bf031 in device_set_realized hw/core/qdev.c:876
#6 0x5650e0531efd in property_set_bool qom/object.c:2080
#7 0x5650e053650e in object_property_set_qobject qom/qom-qobject.c:26
#8 0x5650e0533e14 in object_property_set_bool qom/object.c:1338
#9 0x5650e04c0e37 in virtio_pci_realize hw/virtio/virtio-pci.c:1801

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Reviewed-by: Laurent Vivier 
Cc: Laurent Vivier 
Cc: Amit Shah 
Cc: "Marc-André Lureau" 
Cc: Paolo Bonzini 
---
Changes v2 to v1:
- use virtio_delete_queue to cleanup vq through a vq pointer (suggested by
  Michael S. Tsirkin)
Changes v3 to v1:
- change virtio_delete_queue to virtio_queue_cleanup.
---
 hw/char/virtio-serial-bus.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 3325904..f63dc46 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -1126,9 +1126,17 @@ static void virtio_serial_device_unrealize(DeviceState 
*dev, Error **errp)
 {
 VirtIODevice *vdev = VIRTIO_DEVICE(dev);
 VirtIOSerial *vser = VIRTIO_SERIAL(dev);
+int i;
 
 QLIST_REMOVE(vser, next);
 
+virtio_queue_cleanup(vser->c_ivq);
+virtio_queue_cleanup(vser->c_ovq);
+for (i = 0; i < vser->bus.max_nr_ports; i++) {
+virtio_queue_cleanup(vser->ivqs[i]);
+virtio_queue_cleanup(vser->ovqs[i]);
+}
+
 g_free(vser->ivqs);
 g_free(vser->ovqs);
 g_free(vser->ports_map);
-- 
2.7.2.windows.1

[PATCH v3 2/3] virtio-balloon: fix memory leak while attach virtio-balloon device

[pannengyuan]

From: Pan Nengyuan 

ivq/dvq/svq/free_page_vq forgot to cleanup in
virtio_balloon_device_unrealize, the memory leak stack is as follow:

Direct leak of 14336 byte(s) in 2 object(s) allocated from:
#0 0x7f99fd9d8560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
#1 0x7f99fcb20015 in g_malloc0 (/usr/lib64/libglib-2.0.so.0+0x50015)
#2 0x557d90638437 in virtio_add_queue hw/virtio/virtio.c:2327
#3 0x557d9064401d in virtio_balloon_device_realize 
hw/virtio/virtio-balloon.c:793
#4 0x557d906356f7 in virtio_device_realize hw/virtio/virtio.c:3504
#5 0x557d9073f081 in device_set_realized hw/core/qdev.c:876
#6 0x557d908b1f4d in property_set_bool qom/object.c:2080
#7 0x557d908b655e in object_property_set_qobject qom/qom-qobject.c:26

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Cc: Amit Shah 
Reviewed-by: Laurent Vivier 
---
Changes v2 to v1:
- use virtio_delete_queue to cleanup vq through a vq pointer (suggested by
  Michael S. Tsirkin)
---
Changes v3 to v2:
- change virtio_delete_queue to virtio_queue_cleanup
---
 hw/virtio/virtio-balloon.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 40b04f5..681a2b2 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -831,6 +831,13 @@ static void virtio_balloon_device_unrealize(DeviceState 
*dev, Error **errp)
 }
 balloon_stats_destroy_timer(s);
 qemu_remove_balloon_handler(s);
+
+virtio_queue_cleanup(s->ivq);
+virtio_queue_cleanup(s->dvq);
+virtio_queue_cleanup(s->svq);
+if (s->free_page_vq) {
+virtio_queue_cleanup(s->free_page_vq);
+}
 virtio_cleanup(vdev);
 }
 
-- 
2.7.2.windows.1

[PATCH v3 0/3] virtio: fix memory leak in virtio-balloon/virtio-serial-bus

[pannengyuan]

From: Pan Nengyuan 

This series add a new function to cleanup vqueue through a vq pointer, and fix 
memory
leaks in virtio-balloon and virtio-serial-bus.

---
Changes v2 to v1:
- add a new function to cleanup vqueue through a vq pointer.
---
Changes v3 to v2:
- change function name from virtio_delete_queue to virtio_queue_cleanup.

Michael S. Tsirkin(1)
  virtio: add ability to delete vq through a pointer 

Pan Nengyuan (2):
  virtio-balloon: fix memory leak while attach virtio-balloon device
  virtio-serial-bus: fix memory leak while attach virtio-serial-bus

 hw/char/virtio-serial-bus.c |  8 
 hw/virtio/virtio-balloon.c  |  7 +++
 hw/virtio/virtio.c  | 16 +++-
 include/hw/virtio/virtio.h  |  2 ++
 4 files changed, 28 insertions(+), 5 deletions(-)

-- 
2.7.2.windows.1

[PATCH v3 1/3] virtio: add ability to delete vq through a pointer

[pannengyuan]

From: Michael S. Tsirkin  

Devices tend to maintain vq pointers, allow deleting them through a vq
pointer.

Signed-off-by: Michael S. Tsirkin 
Signed-off-by: Pan Nengyuan 
[PMM: change function name to virtio_queue_cleanup; set used_elems to NULL 
after free]
Cc: Amit Shah 
Reviewed-by: Pankaj Gupta 
Reviewed-by: Laurent Vivier 
---
Changes v2 to v1:
- use virtio_delete_queue to cleanup vq through a vq pointer
---
Changes v3 to v2:
- change function name from virtio_delete_queue to virtio_queue_cleanup
---
 hw/virtio/virtio.c | 16 +++-
 include/hw/virtio/virtio.h |  2 ++
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 04716b5..2743258 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2330,17 +2330,23 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int 
queue_size,
 return >vq[i];
 }
 
+void virtio_queue_cleanup(VirtQueue *vq)
+{
+vq->vring.num = 0;
+vq->vring.num_default = 0;
+vq->handle_output = NULL;
+vq->handle_aio_output = NULL;
+g_free(vq->used_elems);
+vq->used_elems = NULL;
+}
+
 void virtio_del_queue(VirtIODevice *vdev, int n)
 {
 if (n < 0 || n >= VIRTIO_QUEUE_MAX) {
 abort();
 }
 
-vdev->vq[n].vring.num = 0;
-vdev->vq[n].vring.num_default = 0;
-vdev->vq[n].handle_output = NULL;
-vdev->vq[n].handle_aio_output = NULL;
-g_free(vdev->vq[n].used_elems);
+virtio_queue_cleanup(>vq[n]);
 }
 
 static void virtio_set_isr(VirtIODevice *vdev, int value)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index c32a815..cc0b3f0 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -183,6 +183,8 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int 
queue_size,
 
 void virtio_del_queue(VirtIODevice *vdev, int n);
 
+void virtio_queue_cleanup(VirtQueue *vq);
+
 void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem,
 unsigned int len);
 void virtqueue_flush(VirtQueue *vq, unsigned int count);
-- 
2.7.2.windows.1

[PATCH v5 2/2] block/nbd: fix memory leak in nbd_open()

[pannengyuan]

From: Pan Nengyuan 

In currently implementation there will be a memory leak when
nbd_client_connect() returns error status. Here is an easy way to
reproduce:

1. run qemu-iotests as follow and check the result with asan:
./check -raw 143

Following is the asan output backtrack:
Direct leak of 40 byte(s) in 1 object(s) allocated from:
#0 0x7f629688a560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
#1 0x7f6295e7e015 in g_malloc0  (/usr/lib64/libglib-2.0.so.0+0x50015)
#2 0x56281dab4642 in qobject_input_start_struct  
/mnt/sdb/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:295
#3 0x56281dab1a04 in visit_start_struct  
/mnt/sdb/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:49
#4 0x56281dad1827 in visit_type_SocketAddress  qapi/qapi-visit-sockets.c:386
#5 0x56281da8062f in nbd_config   /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1716
#6 0x56281da8062f in nbd_process_options 
/mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1829
#7 0x56281da8062f in nbd_open /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873

Direct leak of 15 byte(s) in 1 object(s) allocated from:
#0 0x7f629688a3a0 in malloc (/usr/lib64/libasan.so.3+0xc73a0)
#1 0x7f6295e7dfbd in g_malloc (/usr/lib64/libglib-2.0.so.0+0x4ffbd)
#2 0x7f6295e96ace in g_strdup (/usr/lib64/libglib-2.0.so.0+0x68ace)
#3 0x56281da804ac in nbd_process_options 
/mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1834
#4 0x56281da804ac in nbd_open /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873

Indirect leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x7f629688a3a0 in malloc (/usr/lib64/libasan.so.3+0xc73a0)
#1 0x7f6295e7dfbd in g_malloc (/usr/lib64/libglib-2.0.so.0+0x4ffbd)
#2 0x7f6295e96ace in g_strdup (/usr/lib64/libglib-2.0.so.0+0x68ace)
#3 0x56281dab41a3 in qobject_input_type_str_keyval 
/mnt/sdb/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:536
#4 0x56281dab2ee9 in visit_type_str 
/mnt/sdb/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:297
#5 0x56281dad0fa1 in visit_type_UnixSocketAddress_members 
qapi/qapi-visit-sockets.c:141
#6 0x56281dad17b6 in visit_type_SocketAddress_members 
qapi/qapi-visit-sockets.c:366
#7 0x56281dad186a in visit_type_SocketAddress qapi/qapi-visit-sockets.c:393
#8 0x56281da8062f in nbd_config /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1716
#9 0x56281da8062f in nbd_process_options 
/mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1829
#10 0x56281da8062f in nbd_open /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873

Fixes: 8f071c9db506e03ab
Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Cc: qemu-stable 
Cc: Vladimir Sementsov-Ogievskiy 
---
Changes v2 to v1:
- add a new function to do the common cleanups (suggested by Stefano
  Garzarella).
---
Changes v3 to v2:
- split in two patches(suggested by Stefano Garzarella)
---
Changes v4 to v3:
- replace function name from nbd_free_bdrvstate_prop to
  nbd_clear_bdrvstate and add Fixes tag.
---
Changes v5 to v4:
- correct the wrong email address
---
 block/nbd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/nbd.c b/block/nbd.c
index 8b4a65a..9062409 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1891,6 +1891,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options, 
int flags,
 
 ret = nbd_client_connect(bs, errp);
 if (ret < 0) {
+nbd_clear_bdrvstate(s);
 return ret;
 }
 /* successfully connected */
-- 
2.7.2.windows.1

[PATCH v5 1/2] block/nbd: extract the common cleanup code

[pannengyuan]

From: Pan Nengyuan 

The BDRVNBDState cleanup code is common in two places, add
nbd_clear_bdrvstate() function to do these cleanups.

Signed-off-by: Stefano Garzarella 
Signed-off-by: Pan Nengyuan 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
v3:
- new patch, split form 2/2 patch (suggested by Stefano Garzarella)
Changes v4 to v3:
- replace function name from nbd_free_bdrvstate_prop to
  nbd_clear_bdrvstate and set cleared fields to NULL (suggested by Eric
  Blake)
- remove static function prototype. (suggested by Eric Blake)
---
Changes v5 to v4:
- correct the wrong email address
---
 block/nbd.c | 25 +++--
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 1239761..8b4a65a 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -94,6 +94,19 @@ typedef struct BDRVNBDState {
 
 static int nbd_client_connect(BlockDriverState *bs, Error **errp);
 
+void nbd_clear_bdrvstate(BDRVNBDState *s)
+{
+object_unref(OBJECT(s->tlscreds));
+qapi_free_SocketAddress(s->saddr);
+s->saddr = NULL;
+g_free(s->export);
+s->export = NULL;
+g_free(s->tlscredsid);
+s->tlscredsid = NULL;
+g_free(s->x_dirty_bitmap);
+s->x_dirty_bitmap = NULL;
+}
+
 static void nbd_channel_error(BDRVNBDState *s, int ret)
 {
 if (ret == -EIO) {
@@ -1855,10 +1868,7 @@ static int nbd_process_options(BlockDriverState *bs, 
QDict *options,
 
  error:
 if (ret < 0) {
-object_unref(OBJECT(s->tlscreds));
-qapi_free_SocketAddress(s->saddr);
-g_free(s->export);
-g_free(s->tlscredsid);
+nbd_clear_bdrvstate(s);
 }
 qemu_opts_del(opts);
 return ret;
@@ -1937,12 +1947,7 @@ static void nbd_close(BlockDriverState *bs)
 BDRVNBDState *s = bs->opaque;
 
 nbd_client_close(bs);
-
-object_unref(OBJECT(s->tlscreds));
-qapi_free_SocketAddress(s->saddr);
-g_free(s->export);
-g_free(s->tlscredsid);
-g_free(s->x_dirty_bitmap);
+nbd_clear_bdrvstate(s);
 }
 
 static int64_t nbd_getlength(BlockDriverState *bs)
-- 
2.7.2.windows.1

[PATCH v5 0/2] block/nbd: fix memory leak in nbd_open

[pannengyuan]

From: Pan Nengyuan 

This series add a new function to do the common cleanups, and fix a memory
leak in nbd_open when nbd_client_connect returns error status.

---
Changes v2 to v1:
- add a new function to do the common cleanups (suggested by Stefano 
Garzarella).
---
Changes v3 to v2:
- split in two patches(suggested by Stefano Garzarella)
---
Changes v4 to v3:
- replace function name from nbd_free_bdrvstate_prop to nbd_clear_bdrvstate and 
add Fixes tag(suggested by Eric Blake).
- remove static function prototype. (suggested by Eric Blake)
---
Changes v5 to v4:
- correct the wrong email address

Pan Nengyuan (2):
  block/nbd: extract the common cleanup code
  block/nbd: fix memory leak in nbd_open()

 block/nbd.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

-- 
2.7.2.windows.1

[PATCH v4 1/2] block/nbd: extract the common cleanup code

[pannengyuan]

From: Pan Nengyuan 

The BDRVNBDState cleanup code is common in two places, add
nbd_clear_bdrvstate() function to do these cleanups.

Signed-off-by: Stefano Garzarella 
Signed-off-by: Pan Nengyuan 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
---
v3:
- new patch, split form 2/2 patch (suggested by Stefano Garzarella)
Changes v4 to v3:
- replace function name from nbd_free_bdrvstate_prop to
  nbd_clear_bdrvstate and set cleared fields to NULL (suggested by Eric
  Blake)
- remove static function prototype. (suggested by Eric Blake)
---
 block/nbd.c | 25 +++--
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 1239761..8b4a65a 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -94,6 +94,19 @@ typedef struct BDRVNBDState {
 
 static int nbd_client_connect(BlockDriverState *bs, Error **errp);
 
+void nbd_clear_bdrvstate(BDRVNBDState *s)
+{
+object_unref(OBJECT(s->tlscreds));
+qapi_free_SocketAddress(s->saddr);
+s->saddr = NULL;
+g_free(s->export);
+s->export = NULL;
+g_free(s->tlscredsid);
+s->tlscredsid = NULL;
+g_free(s->x_dirty_bitmap);
+s->x_dirty_bitmap = NULL;
+}
+
 static void nbd_channel_error(BDRVNBDState *s, int ret)
 {
 if (ret == -EIO) {
@@ -1855,10 +1868,7 @@ static int nbd_process_options(BlockDriverState *bs, 
QDict *options,
 
  error:
 if (ret < 0) {
-object_unref(OBJECT(s->tlscreds));
-qapi_free_SocketAddress(s->saddr);
-g_free(s->export);
-g_free(s->tlscredsid);
+nbd_clear_bdrvstate(s);
 }
 qemu_opts_del(opts);
 return ret;
@@ -1937,12 +1947,7 @@ static void nbd_close(BlockDriverState *bs)
 BDRVNBDState *s = bs->opaque;
 
 nbd_client_close(bs);
-
-object_unref(OBJECT(s->tlscreds));
-qapi_free_SocketAddress(s->saddr);
-g_free(s->export);
-g_free(s->tlscredsid);
-g_free(s->x_dirty_bitmap);
+nbd_clear_bdrvstate(s);
 }
 
 static int64_t nbd_getlength(BlockDriverState *bs)
-- 
2.7.2.windows.1

[PATCH v4 2/2] block/nbd: fix memory leak in nbd_open()

[pannengyuan]

From: Pan Nengyuan 

In currently implementation there will be a memory leak when
nbd_client_connect() returns error status. Here is an easy way to
reproduce:

1. run qemu-iotests as follow and check the result with asan:
./check -raw 143

Following is the asan output backtrack:
Direct leak of 40 byte(s) in 1 object(s) allocated from:
#0 0x7f629688a560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
#1 0x7f6295e7e015 in g_malloc0  (/usr/lib64/libglib-2.0.so.0+0x50015)
#2 0x56281dab4642 in qobject_input_start_struct  
/mnt/sdb/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:295
#3 0x56281dab1a04 in visit_start_struct  
/mnt/sdb/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:49
#4 0x56281dad1827 in visit_type_SocketAddress  qapi/qapi-visit-sockets.c:386
#5 0x56281da8062f in nbd_config   /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1716
#6 0x56281da8062f in nbd_process_options 
/mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1829
#7 0x56281da8062f in nbd_open /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873

Direct leak of 15 byte(s) in 1 object(s) allocated from:
#0 0x7f629688a3a0 in malloc (/usr/lib64/libasan.so.3+0xc73a0)
#1 0x7f6295e7dfbd in g_malloc (/usr/lib64/libglib-2.0.so.0+0x4ffbd)
#2 0x7f6295e96ace in g_strdup (/usr/lib64/libglib-2.0.so.0+0x68ace)
#3 0x56281da804ac in nbd_process_options 
/mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1834
#4 0x56281da804ac in nbd_open /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873

Indirect leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x7f629688a3a0 in malloc (/usr/lib64/libasan.so.3+0xc73a0)
#1 0x7f6295e7dfbd in g_malloc (/usr/lib64/libglib-2.0.so.0+0x4ffbd)
#2 0x7f6295e96ace in g_strdup (/usr/lib64/libglib-2.0.so.0+0x68ace)
#3 0x56281dab41a3 in qobject_input_type_str_keyval 
/mnt/sdb/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:536
#4 0x56281dab2ee9 in visit_type_str 
/mnt/sdb/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:297
#5 0x56281dad0fa1 in visit_type_UnixSocketAddress_members 
qapi/qapi-visit-sockets.c:141
#6 0x56281dad17b6 in visit_type_SocketAddress_members 
qapi/qapi-visit-sockets.c:366
#7 0x56281dad186a in visit_type_SocketAddress qapi/qapi-visit-sockets.c:393
#8 0x56281da8062f in nbd_config /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1716
#9 0x56281da8062f in nbd_process_options 
/mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1829
#10 0x56281da8062f in nbd_open /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873

Fixes: 8f071c9db506e03ab
Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Cc: qemu-stable 
Cc: Vladimir Sementsov-Ogievskiy 
---
Changes v2 to v1:
- add a new function to do the common cleanups (suggested by Stefano
  Garzarella).
---
Changes v3 to v2:
- split in two patches(suggested by Stefano Garzarella)
---
Changes v4 to v3:
- replace function name from nbd_free_bdrvstate_prop to
  nbd_clear_bdrvstate and add Fixes tag.
---
 block/nbd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/nbd.c b/block/nbd.c
index 8b4a65a..9062409 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1891,6 +1891,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options, 
int flags,
 
 ret = nbd_client_connect(bs, errp);
 if (ret < 0) {
+nbd_clear_bdrvstate(s);
 return ret;
 }
 /* successfully connected */
-- 
2.7.2.windows.1

[PATCH v4 0/2] block/nbd: fix memory leak in nbd_open

[pannengyuan]

From: Pan Nengyuan 

This series add a new function to do the common cleanups, and fix a memory
leak in nbd_open when nbd_client_connect returns error status.

---
Changes v2 to v1:
- add a new function to do the common cleanups (suggested by Stefano 
Garzarella).
---
Changes v3 to v2:
- split in two patches(suggested by Stefano Garzarella)
---
Changes v4 to v3:
- replace function name from nbd_free_bdrvstate_prop to nbd_clear_bdrvstate and 
add Fixes tag(suggested by Eric Blake).
- remove static function prototype. (suggested by Eric Blake)

Pan Nengyuan (2):
  block/nbd: extract the common cleanup code
  block/nbd: fix memory leak in nbd_open()

 block/nbd.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

-- 
2.7.2.windows.1

[PATCH v2 2/3] virtio-balloon: fix memory leak while attach virtio-balloon device

[pannengyuan]

From: Pan Nengyuan 

ivq/dvq/svq/free_page_vq is forgot to cleanup in
virtio_balloon_device_unrealize, the memory leak stack is as follow:

Direct leak of 14336 byte(s) in 2 object(s) allocated from:
#0 0x7f99fd9d8560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
#1 0x7f99fcb20015 in g_malloc0 (/usr/lib64/libglib-2.0.so.0+0x50015)
#2 0x557d90638437 in virtio_add_queue hw/virtio/virtio.c:2327
#3 0x557d9064401d in virtio_balloon_device_realize 
hw/virtio/virtio-balloon.c:793
#4 0x557d906356f7 in virtio_device_realize hw/virtio/virtio.c:3504
#5 0x557d9073f081 in device_set_realized hw/core/qdev.c:876
#6 0x557d908b1f4d in property_set_bool qom/object.c:2080
#7 0x557d908b655e in object_property_set_qobject qom/qom-qobject.c:26

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
---
Change v2 to v1:
- use virtio_delete_queue to cleanup vq through a vq pointer (suggested by
  Michael S. Tsirkin)
---
 hw/virtio/virtio-balloon.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 40b04f5..57f3b9f 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -831,6 +831,13 @@ static void virtio_balloon_device_unrealize(DeviceState 
*dev, Error **errp)
 }
 balloon_stats_destroy_timer(s);
 qemu_remove_balloon_handler(s);
+
+virtio_delete_queue(s->ivq);
+virtio_delete_queue(s->dvq);
+virtio_delete_queue(s->svq);
+if (s->free_page_vq) {
+virtio_delete_queue(s->free_page_vq);
+}
 virtio_cleanup(vdev);
 }
 
-- 
2.7.2.windows.1

[PATCH v2 1/3] virtio: add ability to delete vq through a pointer

[pannengyuan]

From: Pan Nengyuan 

Devices tend to maintain vq pointers, allow deleting them trough a vq pointer.

Signed-off-by: Michael S. Tsirkin 
Signed-off-by: Pan Nengyuan 
---
Changes v2 to v1:
- add a new function virtio_delete_queue to cleanup vq through a vq pointer
---
 hw/virtio/virtio.c | 16 +++-
 include/hw/virtio/virtio.h |  2 ++
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 04716b5..6de3cfd 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -2330,17 +2330,23 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int 
queue_size,
 return >vq[i];
 }
 
+void virtio_delete_queue(VirtQueue *vq)
+{
+vq->vring.num = 0;
+vq->vring.num_default = 0;
+vq->handle_output = NULL;
+vq->handle_aio_output = NULL;
+g_free(vq->used_elems);
+vq->used_elems = NULL;
+}
+
 void virtio_del_queue(VirtIODevice *vdev, int n)
 {
 if (n < 0 || n >= VIRTIO_QUEUE_MAX) {
 abort();
 }
 
-vdev->vq[n].vring.num = 0;
-vdev->vq[n].vring.num_default = 0;
-vdev->vq[n].handle_output = NULL;
-vdev->vq[n].handle_aio_output = NULL;
-g_free(vdev->vq[n].used_elems);
+virtio_delete_queue(>vq[n]);
 }
 
 static void virtio_set_isr(VirtIODevice *vdev, int value)
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index c32a815..e18756d 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -183,6 +183,8 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int 
queue_size,
 
 void virtio_del_queue(VirtIODevice *vdev, int n);
 
+void virtio_delete_queue(VirtQueue *vq);
+
 void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem,
 unsigned int len);
 void virtqueue_flush(VirtQueue *vq, unsigned int count);
-- 
2.7.2.windows.1

[PATCH v2 3/3] virtio-serial-bus: fix memory leak while attach virtio-serial-bus

[pannengyuan]

From: Pan Nengyuan 

ivqs/ovqs/c_ivq/c_ovq is forgot to cleanup in
virtio_serial_device_unrealize, the memory leak stack is as bellow:

Direct leak of 1290240 byte(s) in 180 object(s) allocated from:
#0 0x7fc9bfc27560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
#1 0x7fc9bed6f015 in g_malloc0 (/usr/lib64/libglib-2.0.so.0+0x50015)
#2 0x5650e02b83e7 in virtio_add_queue hw/virtio/virtio.c:2327
#3 0x5650e02847b5 in virtio_serial_device_realize 
hw/char/virtio-serial-bus.c:1089
#4 0x5650e02b56a7 in virtio_device_realize hw/virtio/virtio.c:3504
#5 0x5650e03bf031 in device_set_realized hw/core/qdev.c:876
#6 0x5650e0531efd in property_set_bool qom/object.c:2080
#7 0x5650e053650e in object_property_set_qobject qom/qom-qobject.c:26
#8 0x5650e0533e14 in object_property_set_bool qom/object.c:1338
#9 0x5650e04c0e37 in virtio_pci_realize hw/virtio/virtio-pci.c:1801

Reported-by: Euler Robot 
Signed-off-by: Pan Nengyuan 
Cc: Laurent Vivier 
Cc: Amit Shah 
Cc: "Marc-Andr?? Lureau" 
Cc: Paolo Bonzini 
---
Changes v2 to v1:
- use virtio_delete_queue to cleanup vq through a vq pointer (suggested by
  Michael S. Tsirkin)
---
 hw/char/virtio-serial-bus.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 3325904..e1cbce3 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -1126,9 +1126,17 @@ static void virtio_serial_device_unrealize(DeviceState 
*dev, Error **errp)
 {
 VirtIODevice *vdev = VIRTIO_DEVICE(dev);
 VirtIOSerial *vser = VIRTIO_SERIAL(dev);
+int i;
 
 QLIST_REMOVE(vser, next);
 
+virtio_delete_queue(vser->c_ivq);
+virtio_delete_queue(vser->c_ovq);
+for (i = 0; i < vser->bus.max_nr_ports; i++) {
+virtio_delete_queue(vser->ivqs[i]);
+virtio_delete_queue(vser->ovqs[i]);
+}
+
 g_free(vser->ivqs);
 g_free(vser->ovqs);
 g_free(vser->ports_map);
-- 
2.7.2.windows.1

Re: for 4.2 ??? Re: [PATCH V3 2/2] block/nbd: fix memory leak in nbd_open()

[pannengyuan]

On 2019/12/4 2:54, Eric Blake wrote:
> On 12/3/19 11:52 AM, Vladimir Sementsov-Ogievskiy wrote:
>> It's just a memory leak, but it's a regression in 4.2.
>>
>> Should we take it into 4.2?
> 
> Sorry, I was on holiday and then jury service, so I missed any chance at
> getting this into -rc3.  The memory leak only happens on failure, and
> you'd have to be pretty desperate to purposefully attempt to open a lot
> of NBD devices where you know you'll get a failure just to trigger
> enough of a leak to cause the OOM-killer to target qemu.  So I'm fine if
> this is deferred to 5.0, and just cc's qemu-stable (now done).
> 
> I'll queue this through my NBD tree for 5.0.
> 
>>
>>
>> 29.11.2019 10:25, pannengy...@huawei.com wrote:
>>> From: PanNengyuan 
> 
>>>
>>> Reported-by: Euler Robot 
>>> Signed-off-by: PanNengyuan 
> 
> I'm not one to tell you that your name is written incorrectly, but it
> does look odd to have a single word rather than a space between two
> capitalized portions.  If that's really how you want your S-o-b and
> authorship to appear, I'm happy to preserve it; but you may want to
> consider updating your git settings, and posting a v4 with an updated
> spelling if you would prefer something different.  (It is also
> acceptable to use UTF-8 characters; some people like listing an S-o-b in
> both native characters and a Westernized variant).
> 

Thanks for your advice, I will update my git settings.

>>
>> May add:
>>
>> Fixes: 8f071c9db506e03ab
> 
> Yes, information like that helps in deciding how long the problem has
> been present (in this case, it is indeed a regression added in 4.2, even
> if minor in nature).
> 

ok, I will add it next time.

Re: [PATCH V3 1/2] block/nbd: extract the common cleanup code

[pannengyuan]

On 2019/12/4 3:00, Eric Blake wrote:
> On 11/29/19 1:25 AM, pannengy...@huawei.com wrote:
>> From: PanNengyuan 
>>
>> The BDRVNBDState cleanup code is common in two places, add
>> nbd_free_bdrvstate_prop() function to do these cleanups (suggested by
>> Stefano Garzarella).
>>
>> Signed-off-by: PanNengyuan 
>> ---
>>   block/nbd.c | 23 +--
>>   1 file changed, 13 insertions(+), 10 deletions(-)
>>
>> diff --git a/block/nbd.c b/block/nbd.c
>> index 1239761..5805979 100644
>> --- a/block/nbd.c
>> +++ b/block/nbd.c
>> @@ -94,6 +94,8 @@ typedef struct BDRVNBDState {
>>     static int nbd_client_connect(BlockDriverState *bs, Error **errp);
>>   +static void nbd_free_bdrvstate_prop(BDRVNBDState *s);
>> +
> 
> Why do you need a static function prototype?  Just implement the
> function prior to its first use, then you won't need a forward declaration.

Yes, It's not necessary. I will change it.

> 
>>   static void nbd_channel_error(BDRVNBDState *s, int ret)
>>   {
>>   if (ret == -EIO) {
>> @@ -1486,6 +1488,15 @@ static int nbd_client_connect(BlockDriverState
>> *bs, Error **errp)
>>   }
>>   }
>>   +static void nbd_free_bdrvstate_prop(BDRVNBDState *s)
>> +{
>> +    object_unref(OBJECT(s->tlscreds));
>> +    qapi_free_SocketAddress(s->saddr);
>> +    g_free(s->export);
>> +    g_free(s->tlscredsid);
>> +    g_free(s->x_dirty_bitmap);
>> +}
> 
> In fact, it appears that you did just that, as the first use...
> 
> Bike-shedding: the name 'nbd_free_bdrvstate_prop' doesn't seem right to
> me - when I see a function with 'free' in the name taking a single
> pointer, I assume that the given pointer (here, BDRVNBDState *s) is
> freed - but your function does NOT free then incoming pointer.  Rather,
> you are clearing out the contents within a pre-allocated object which
> remains allocated.  What's more, since the object remains allocated, I'm
> surprised that you are not setting fields to NULL to prevent
> use-after-free bugs.
> 
> Either this function should also free s (in which case naming it merely
> 'nbd_free_bdrvstate' might be better), or you should consider naming it
> 'nbd_clear_bdrvstate' and assigning cleared fields to NULL.
> 

thanks, 'nbd_clear_bdrvstate' seems nice. I will replace the name and
set fields to NULL in next version.

>> +
>>   /*
>>    * Parse nbd_open options
>>    */
>> @@ -1855,10 +1866,7 @@ static int nbd_process_options(BlockDriverState
>> *bs, QDict *options,
>>      error:
>>   if (ret < 0) {
>> -    object_unref(OBJECT(s->tlscreds));
>> -    qapi_free_SocketAddress(s->saddr);
>> -    g_free(s->export);
>> -    g_free(s->tlscredsid);
>> +    nbd_free_bdrvstate_prop(s);
> 
> ...is here.
> 
>>   }
>>   qemu_opts_del(opts);
>>   return ret;
>> @@ -1937,12 +1945,7 @@ static void nbd_close(BlockDriverState *bs)
>>   BDRVNBDState *s = bs->opaque;
>>     nbd_client_close(bs);
>> -
>> -    object_unref(OBJECT(s->tlscreds));
>> -    qapi_free_SocketAddress(s->saddr);
>> -    g_free(s->export);
>> -    g_free(s->tlscredsid);
>> -    g_free(s->x_dirty_bitmap);
>> +    nbd_free_bdrvstate_prop(s);
>>   }
>>     static int64_t nbd_getlength(BlockDriverState *bs)
>>
> 

Re: [PATCH V3 1/2] block/nbd: extract the common cleanup code

[pannengyuan]

On 2019/12/4 1:38, Vladimir Sementsov-Ogievskiy wrote:
> Hi!
> 
> First, please, when sending more than one patch, create a cover-letter. Also,
> summarize (in cover letter) what was changed since previous version.
In previous version, I only send one patch(2/2 in this version), so I
only add a change summarize in 2/2 patch in this version. should I add a
summarize in 1/2 patch too if 1/2 patch is a new one?

> 
> 29.11.2019 10:25, pannengy...@huawei.com wrote:
>> From: PanNengyuan 
> 
> Strange line. Check you git preferences. Such line appears (and make sense)
> when you are sending patches authored by someone else.. But here is your name,
> the same as in email's From:.

Thanks for your reminding. I will correct it in next version.

> 
>>
>> The BDRVNBDState cleanup code is common in two places, add
>> nbd_free_bdrvstate_prop() function to do these cleanups (suggested by
>> Stefano Garzarella).
>>
>> Signed-off-by: PanNengyuan 
> 
> Reviewed-by: Vladimir Sementsov-Ogievskiy 
> 
>> ---
>>   block/nbd.c | 23 +--
>>   1 file changed, 13 insertions(+), 10 deletions(-)
>>
>> diff --git a/block/nbd.c b/block/nbd.c
>> index 1239761..5805979 100644
>> --- a/block/nbd.c
>> +++ b/block/nbd.c
>> @@ -94,6 +94,8 @@ typedef struct BDRVNBDState {
>>   
>>   static int nbd_client_connect(BlockDriverState *bs, Error **errp);
>>   
>> +static void nbd_free_bdrvstate_prop(BDRVNBDState *s);
>> +
>>   static void nbd_channel_error(BDRVNBDState *s, int ret)
>>   {
>>   if (ret == -EIO) {
>> @@ -1486,6 +1488,15 @@ static int nbd_client_connect(BlockDriverState *bs, 
>> Error **errp)
>>   }
>>   }
>>   
>> +static void nbd_free_bdrvstate_prop(BDRVNBDState *s)
>> +{
>> +object_unref(OBJECT(s->tlscreds));
>> +qapi_free_SocketAddress(s->saddr);
>> +g_free(s->export);
>> +g_free(s->tlscredsid);
>> +g_free(s->x_dirty_bitmap);
>> +}
>> +
>>   /*
>>* Parse nbd_open options
>>*/
>> @@ -1855,10 +1866,7 @@ static int nbd_process_options(BlockDriverState *bs, 
>> QDict *options,
>>   
>>error:
>>   if (ret < 0) {
>> -object_unref(OBJECT(s->tlscreds));
>> -qapi_free_SocketAddress(s->saddr);
>> -g_free(s->export);
>> -g_free(s->tlscredsid);
>> +nbd_free_bdrvstate_prop(s);
>>   }
>>   qemu_opts_del(opts);
>>   return ret;
>> @@ -1937,12 +1945,7 @@ static void nbd_close(BlockDriverState *bs)
>>   BDRVNBDState *s = bs->opaque;
>>   
>>   nbd_client_close(bs);
>> -
>> -object_unref(OBJECT(s->tlscreds));
>> -qapi_free_SocketAddress(s->saddr);
>> -g_free(s->export);
>> -g_free(s->tlscredsid);
>> -g_free(s->x_dirty_bitmap);
>> +nbd_free_bdrvstate_prop(s);
>>   }
>>   
>>   static int64_t nbd_getlength(BlockDriverState *bs)
>>
> 
> 

Re: [PATCH] virtio-serial-bus: fix memory leak while attach virtio-serial-bus

[pannengyuan]

On 2019/12/3 16:32, Laurent Vivier wrote:
> On 03/12/2019 01:53, pannengyuan wrote:
>>
>>
>> On 2019/12/2 21:58, Laurent Vivier wrote:
>>> On 02/12/2019 12:15, pannengy...@huawei.com wrote:
>>>> From: PanNengyuan 
>>>>
>>>> ivqs/ovqs/c_ivq/c_ovq is forgot to cleanup in
>>>> virtio_serial_device_unrealize, the memory leak stack is as bellow:
>>>>
>>>> Direct leak of 1290240 byte(s) in 180 object(s) allocated from:
>>>> #0 0x7fc9bfc27560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
>>>> #1 0x7fc9bed6f015 in g_malloc0 (/usr/lib64/libglib-2.0.so.0+0x50015)
>>>> #2 0x5650e02b83e7 in virtio_add_queue 
>>>> /mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio.c:2327
>>>> #3 0x5650e02847b5 in virtio_serial_device_realize 
>>>> /mnt/sdb/qemu-4.2.0-rc0/hw/char/virtio-serial-bus.c:1089
>>>> #4 0x5650e02b56a7 in virtio_device_realize 
>>>> /mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio.c:3504
>>>> #5 0x5650e03bf031 in device_set_realized 
>>>> /mnt/sdb/qemu-4.2.0-rc0/hw/core/qdev.c:876
>>>> #6 0x5650e0531efd in property_set_bool 
>>>> /mnt/sdb/qemu-4.2.0-rc0/qom/object.c:2080
>>>> #7 0x5650e053650e in object_property_set_qobject 
>>>> /mnt/sdb/qemu-4.2.0-rc0/qom/qom-qobject.c:26
>>>> #8 0x5650e0533e14 in object_property_set_bool 
>>>> /mnt/sdb/qemu-4.2.0-rc0/qom/object.c:1338
>>>> #9 0x5650e04c0e37 in virtio_pci_realize 
>>>> /mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio-pci.c:1801
>>>>
>>>> Reported-by: Euler Robot 
>>>> Signed-off-by: PanNengyuan 
>>>> ---
>>>>  hw/char/virtio-serial-bus.c | 6 ++
>>>>  1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
>>>> index 3325904..da9019a 100644
>>>> --- a/hw/char/virtio-serial-bus.c
>>>> +++ b/hw/char/virtio-serial-bus.c
>>>> @@ -1126,9 +1126,15 @@ static void 
>>>> virtio_serial_device_unrealize(DeviceState *dev, Error **errp)
>>>>  {
>>>>  VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>>>>  VirtIOSerial *vser = VIRTIO_SERIAL(dev);
>>>> +int i;
>>>>  
>>>>  QLIST_REMOVE(vser, next);
>>>>  
>>>> +for (i = 0; i <= vser->bus.max_nr_ports; i++) {
>>>> +virtio_del_queue(vdev, 2 * i);
>>>> +virtio_del_queue(vdev, 2 * i + 1);
>>>> +}
>>>> +
>>>
>>> According to virtio_serial_device_realize() and the number of
>>> virtio_add_queue(), I think you have more queues to delete:
>>>
>>>   4 + 2 * vser->bus.max_nr_ports
>>>
>>> (for vser->ivqs[0], vser->ovqs[0], vser->c_ivq, vser->c_ovq,
>>> vser->ivqs[i], vser->ovqs[i]).
>>>
>>> Thanks,
>>> Laurent
>>>
>>>
>> Thanks, but I think the queues is correct, the queues in
>> virtio_serial_device_realize is as follow:
>>
>> // here is 2
>> vser->ivqs[0] = virtio_add_queue(vdev, 128, handle_input);
>> vser->ovqs[0] = virtio_add_queue(vdev, 128, handle_output);
>>
>> // here is 2
>> vser->c_ivq = virtio_add_queue(vdev, 32, control_in);
>> vser->c_ovq = virtio_add_queue(vdev, 32, control_out);
>>
>> // here 2 * (max_nr_ports - 1)  - i is from 1 to max_nr_ports - 1
>> for (i = 1; i < vser->bus.max_nr_ports; i++) {
>> vser->ivqs[i] = virtio_add_queue(vdev, 128, handle_input);
>> vser->ovqs[i] = virtio_add_queue(vdev, 128, handle_output);
>> }
>>
>> so the total queues number is:  2 * (vser->bus.max_nr_ports + 1)
>>
> 
> Yes, you're right. A comment in the code would have helped or written
> clearly like:
> 
> for (i = 0; i < 2 * (vser->bus.max_nr_ports + 1); i++) {
> virtio_del_queue(vdev, i);
> }
> 
> Thanks,
> Laurent
> 
> 
yes, it would be helpful, Michael S. Tsirkin posted another way to make
it more clear, I will reuse it in next version.

Thanks.
Nengyuan Pan.

Re: [PATCH] virtio-serial-bus: fix memory leak while attach virtio-serial-bus

[pannengyuan]

On 2019/12/3 13:37, Michael S. Tsirkin wrote:
> On Tue, Dec 03, 2019 at 08:53:42AM +0800, pannengyuan wrote:
>>
>>
>> On 2019/12/2 21:58, Laurent Vivier wrote:
>>> On 02/12/2019 12:15, pannengy...@huawei.com wrote:
>>>> From: PanNengyuan 
>>>>
>>>> ivqs/ovqs/c_ivq/c_ovq is forgot to cleanup in
>>>> virtio_serial_device_unrealize, the memory leak stack is as bellow:
>>>>
>>>> Direct leak of 1290240 byte(s) in 180 object(s) allocated from:
>>>> #0 0x7fc9bfc27560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
>>>> #1 0x7fc9bed6f015 in g_malloc0 (/usr/lib64/libglib-2.0.so.0+0x50015)
>>>> #2 0x5650e02b83e7 in virtio_add_queue 
>>>> /mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio.c:2327
>>>> #3 0x5650e02847b5 in virtio_serial_device_realize 
>>>> /mnt/sdb/qemu-4.2.0-rc0/hw/char/virtio-serial-bus.c:1089
>>>> #4 0x5650e02b56a7 in virtio_device_realize 
>>>> /mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio.c:3504
>>>> #5 0x5650e03bf031 in device_set_realized 
>>>> /mnt/sdb/qemu-4.2.0-rc0/hw/core/qdev.c:876
>>>> #6 0x5650e0531efd in property_set_bool 
>>>> /mnt/sdb/qemu-4.2.0-rc0/qom/object.c:2080
>>>> #7 0x5650e053650e in object_property_set_qobject 
>>>> /mnt/sdb/qemu-4.2.0-rc0/qom/qom-qobject.c:26
>>>> #8 0x5650e0533e14 in object_property_set_bool 
>>>> /mnt/sdb/qemu-4.2.0-rc0/qom/object.c:1338
>>>> #9 0x5650e04c0e37 in virtio_pci_realize 
>>>> /mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio-pci.c:1801
>>>>
>>>> Reported-by: Euler Robot 
>>>> Signed-off-by: PanNengyuan 
>>>> ---
>>>>  hw/char/virtio-serial-bus.c | 6 ++
>>>>  1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
>>>> index 3325904..da9019a 100644
>>>> --- a/hw/char/virtio-serial-bus.c
>>>> +++ b/hw/char/virtio-serial-bus.c
>>>> @@ -1126,9 +1126,15 @@ static void 
>>>> virtio_serial_device_unrealize(DeviceState *dev, Error **errp)
>>>>  {
>>>>  VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>>>>  VirtIOSerial *vser = VIRTIO_SERIAL(dev);
>>>> +int i;
>>>>  
>>>>  QLIST_REMOVE(vser, next);
>>>>  
>>>> +for (i = 0; i <= vser->bus.max_nr_ports; i++) {
>>>> +virtio_del_queue(vdev, 2 * i);
>>>> +virtio_del_queue(vdev, 2 * i + 1);
>>>> +}
>>>> +
>>>
>>> According to virtio_serial_device_realize() and the number of
>>> virtio_add_queue(), I think you have more queues to delete:
>>>
>>>   4 + 2 * vser->bus.max_nr_ports
>>>
>>> (for vser->ivqs[0], vser->ovqs[0], vser->c_ivq, vser->c_ovq,
>>> vser->ivqs[i], vser->ovqs[i]).
>>>
>>> Thanks,
>>> Laurent
>>>
>>>
>> Thanks, but I think the queues is correct, the queues in
>> virtio_serial_device_realize is as follow:
>>
>> // here is 2
>> vser->ivqs[0] = virtio_add_queue(vdev, 128, handle_input);
>> vser->ovqs[0] = virtio_add_queue(vdev, 128, handle_output);
>>
>> // here is 2
>> vser->c_ivq = virtio_add_queue(vdev, 32, control_in);
>> vser->c_ovq = virtio_add_queue(vdev, 32, control_out);
>>
>> // here 2 * (max_nr_ports - 1)  - i is from 1 to max_nr_ports - 1
>> for (i = 1; i < vser->bus.max_nr_ports; i++) {
>> vser->ivqs[i] = virtio_add_queue(vdev, 128, handle_input);
>> vser->ovqs[i] = virtio_add_queue(vdev, 128, handle_output);
>> }
>>
>> so the total queues number is:  2 * (vser->bus.max_nr_ports + 1)
> 
> Rather than worry about this, I posted a patch adding virtio_delete_queue.
> How about reusing that, and just using ivqs/ovqs pointers?
> 
Ok, I will reuse it in next version.

Thanks.

Re: [PATCH] virtio-balloon: fix memory leak while attach virtio-balloon device

[pannengyuan]

On 2019/12/3 13:34, Michael S. Tsirkin wrote:
> On Tue, Dec 03, 2019 at 09:44:19AM +0800, pannengy...@huawei.com wrote:
>> From: PanNengyuan 
>>
>> ivq/dvq/svq/free_page_vq is forgot to cleanup in
>> virtio_balloon_device_unrealize, the memory leak stack is as follow:
>>
>> Direct leak of 14336 byte(s) in 2 object(s) allocated from:
>> #0 0x7f99fd9d8560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
>> #1 0x7f99fcb20015 in g_malloc0 (/usr/lib64/libglib-2.0.so.0+0x50015)
>> #2 0x557d90638437 in virtio_add_queue 
>> /mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio.c:2327
>> #3 0x557d9064401d in virtio_balloon_device_realize 
>> /mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio-balloon.c:793
>> #4 0x557d906356f7 in virtio_device_realize 
>> /mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio.c:3504
>> #5 0x557d9073f081 in device_set_realized 
>> /mnt/sdb/qemu-4.2.0-rc0/hw/core/qdev.c:876
>> #6 0x557d908b1f4d in property_set_bool 
>> /mnt/sdb/qemu-4.2.0-rc0/qom/object.c:2080
>> #7 0x557d908b655e in object_property_set_qobject 
>> /mnt/sdb/qemu-4.2.0-rc0/qom/qom-qobject.c:26
>>
>> Reported-by: Euler Robot 
>> Signed-off-by: PanNengyuan 
>> ---
>>  hw/virtio/virtio-balloon.c | 7 +++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
>> index 40b04f5..5329c65 100644
>> --- a/hw/virtio/virtio-balloon.c
>> +++ b/hw/virtio/virtio-balloon.c
>> @@ -831,6 +831,13 @@ static void virtio_balloon_device_unrealize(DeviceState 
>> *dev, Error **errp)
>>  }
>>  balloon_stats_destroy_timer(s);
>>  qemu_remove_balloon_handler(s);
>> +
>> +virtio_del_queue(vdev, 0);
>> +virtio_del_queue(vdev, 1);
>> +virtio_del_queue(vdev, 2);
>> +if (s->free_page_vq) {
>> +virtio_del_queue(vdev, 3);
>> +}
>>  virtio_cleanup(vdev);
>>  }
> 
> Hmm ok, but how about just doing it through a vq pointer then?
> Seems cleaner. E.g. use patch below and add your on top
> using the new virtio_delete_queue?
> 

ok, It seems more cleaner, I will send a new version later.

Thanks.

> -->
> virtio: add ability to delete vq through a pointer
> 
> Devices tend to maintain vq pointers, allow deleting them like this.
> 
> Signed-off-by: Michael S. Tsirkin 
> 
> --
> 
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index c32a815303..e18756d50d 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -183,6 +183,8 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int 
> queue_size,
>  
>  void virtio_del_queue(VirtIODevice *vdev, int n);
>  
> +void virtio_delete_queue(VirtQueue *vq);
> +
>  void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem,
>  unsigned int len);
>  void virtqueue_flush(VirtQueue *vq, unsigned int count);
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 04716b5f6c..31dd140990 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -2330,17 +2330,22 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int 
> queue_size,
>  return >vq[i];
>  }
>  
> +void virtio_delete_queue(VirtQueue *vq)
> +{
> +vq->vring.num = 0;
> +vq->vring.num_default = 0;
> +vq->handle_output = NULL;
> +vq->handle_aio_output = NULL;
> +g_free(vq->used_elems);
> +}
> +
>  void virtio_del_queue(VirtIODevice *vdev, int n)
>  {
>  if (n < 0 || n >= VIRTIO_QUEUE_MAX) {
>  abort();
>  }
>  
> -vdev->vq[n].vring.num = 0;
> -vdev->vq[n].vring.num_default = 0;
> -vdev->vq[n].handle_output = NULL;
> -vdev->vq[n].handle_aio_output = NULL;
> -g_free(vdev->vq[n].used_elems);
> +virtio_delete_queue(>vq[n]);
>  }
>  
>  static void virtio_set_isr(VirtIODevice *vdev, int value)
> 
> 
> .
> 

[PATCH] virtio-balloon: fix memory leak while attach virtio-balloon device

[pannengyuan]

From: PanNengyuan 

ivq/dvq/svq/free_page_vq is forgot to cleanup in
virtio_balloon_device_unrealize, the memory leak stack is as follow:

Direct leak of 14336 byte(s) in 2 object(s) allocated from:
#0 0x7f99fd9d8560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
#1 0x7f99fcb20015 in g_malloc0 (/usr/lib64/libglib-2.0.so.0+0x50015)
#2 0x557d90638437 in virtio_add_queue 
/mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio.c:2327
#3 0x557d9064401d in virtio_balloon_device_realize 
/mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio-balloon.c:793
#4 0x557d906356f7 in virtio_device_realize 
/mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio.c:3504
#5 0x557d9073f081 in device_set_realized 
/mnt/sdb/qemu-4.2.0-rc0/hw/core/qdev.c:876
#6 0x557d908b1f4d in property_set_bool 
/mnt/sdb/qemu-4.2.0-rc0/qom/object.c:2080
#7 0x557d908b655e in object_property_set_qobject 
/mnt/sdb/qemu-4.2.0-rc0/qom/qom-qobject.c:26

Reported-by: Euler Robot 
Signed-off-by: PanNengyuan 
---
 hw/virtio/virtio-balloon.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 40b04f5..5329c65 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -831,6 +831,13 @@ static void virtio_balloon_device_unrealize(DeviceState 
*dev, Error **errp)
 }
 balloon_stats_destroy_timer(s);
 qemu_remove_balloon_handler(s);
+
+virtio_del_queue(vdev, 0);
+virtio_del_queue(vdev, 1);
+virtio_del_queue(vdev, 2);
+if (s->free_page_vq) {
+virtio_del_queue(vdev, 3);
+}
 virtio_cleanup(vdev);
 }
 
-- 
2.7.2.windows.1

Re: [PATCH] virtio-serial-bus: fix memory leak while attach virtio-serial-bus

[pannengyuan]

On 2019/12/2 21:58, Laurent Vivier wrote:
> On 02/12/2019 12:15, pannengy...@huawei.com wrote:
>> From: PanNengyuan 
>>
>> ivqs/ovqs/c_ivq/c_ovq is forgot to cleanup in
>> virtio_serial_device_unrealize, the memory leak stack is as bellow:
>>
>> Direct leak of 1290240 byte(s) in 180 object(s) allocated from:
>> #0 0x7fc9bfc27560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
>> #1 0x7fc9bed6f015 in g_malloc0 (/usr/lib64/libglib-2.0.so.0+0x50015)
>> #2 0x5650e02b83e7 in virtio_add_queue 
>> /mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio.c:2327
>> #3 0x5650e02847b5 in virtio_serial_device_realize 
>> /mnt/sdb/qemu-4.2.0-rc0/hw/char/virtio-serial-bus.c:1089
>> #4 0x5650e02b56a7 in virtio_device_realize 
>> /mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio.c:3504
>> #5 0x5650e03bf031 in device_set_realized 
>> /mnt/sdb/qemu-4.2.0-rc0/hw/core/qdev.c:876
>> #6 0x5650e0531efd in property_set_bool 
>> /mnt/sdb/qemu-4.2.0-rc0/qom/object.c:2080
>> #7 0x5650e053650e in object_property_set_qobject 
>> /mnt/sdb/qemu-4.2.0-rc0/qom/qom-qobject.c:26
>> #8 0x5650e0533e14 in object_property_set_bool 
>> /mnt/sdb/qemu-4.2.0-rc0/qom/object.c:1338
>> #9 0x5650e04c0e37 in virtio_pci_realize 
>> /mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio-pci.c:1801
>>
>> Reported-by: Euler Robot 
>> Signed-off-by: PanNengyuan 
>> ---
>>  hw/char/virtio-serial-bus.c | 6 ++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
>> index 3325904..da9019a 100644
>> --- a/hw/char/virtio-serial-bus.c
>> +++ b/hw/char/virtio-serial-bus.c
>> @@ -1126,9 +1126,15 @@ static void 
>> virtio_serial_device_unrealize(DeviceState *dev, Error **errp)
>>  {
>>  VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>>  VirtIOSerial *vser = VIRTIO_SERIAL(dev);
>> +int i;
>>  
>>  QLIST_REMOVE(vser, next);
>>  
>> +for (i = 0; i <= vser->bus.max_nr_ports; i++) {
>> +virtio_del_queue(vdev, 2 * i);
>> +virtio_del_queue(vdev, 2 * i + 1);
>> +}
>> +
> 
> According to virtio_serial_device_realize() and the number of
> virtio_add_queue(), I think you have more queues to delete:
> 
>   4 + 2 * vser->bus.max_nr_ports
> 
> (for vser->ivqs[0], vser->ovqs[0], vser->c_ivq, vser->c_ovq,
> vser->ivqs[i], vser->ovqs[i]).
> 
> Thanks,
> Laurent
> 
> 
Thanks, but I think the queues is correct, the queues in
virtio_serial_device_realize is as follow:

// here is 2
vser->ivqs[0] = virtio_add_queue(vdev, 128, handle_input);
vser->ovqs[0] = virtio_add_queue(vdev, 128, handle_output);

// here is 2
vser->c_ivq = virtio_add_queue(vdev, 32, control_in);
vser->c_ovq = virtio_add_queue(vdev, 32, control_out);

// here 2 * (max_nr_ports - 1)  - i is from 1 to max_nr_ports - 1
for (i = 1; i < vser->bus.max_nr_ports; i++) {
vser->ivqs[i] = virtio_add_queue(vdev, 128, handle_input);
vser->ovqs[i] = virtio_add_queue(vdev, 128, handle_output);
}

so the total queues number is:  2 * (vser->bus.max_nr_ports + 1)

[PATCH] virtio-serial-bus: fix memory leak while attach virtio-serial-bus

[pannengyuan]

From: PanNengyuan 

ivqs/ovqs/c_ivq/c_ovq is forgot to cleanup in
virtio_serial_device_unrealize, the memory leak stack is as bellow:

Direct leak of 1290240 byte(s) in 180 object(s) allocated from:
#0 0x7fc9bfc27560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
#1 0x7fc9bed6f015 in g_malloc0 (/usr/lib64/libglib-2.0.so.0+0x50015)
#2 0x5650e02b83e7 in virtio_add_queue 
/mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio.c:2327
#3 0x5650e02847b5 in virtio_serial_device_realize 
/mnt/sdb/qemu-4.2.0-rc0/hw/char/virtio-serial-bus.c:1089
#4 0x5650e02b56a7 in virtio_device_realize 
/mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio.c:3504
#5 0x5650e03bf031 in device_set_realized 
/mnt/sdb/qemu-4.2.0-rc0/hw/core/qdev.c:876
#6 0x5650e0531efd in property_set_bool 
/mnt/sdb/qemu-4.2.0-rc0/qom/object.c:2080
#7 0x5650e053650e in object_property_set_qobject 
/mnt/sdb/qemu-4.2.0-rc0/qom/qom-qobject.c:26
#8 0x5650e0533e14 in object_property_set_bool 
/mnt/sdb/qemu-4.2.0-rc0/qom/object.c:1338
#9 0x5650e04c0e37 in virtio_pci_realize 
/mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio-pci.c:1801

Reported-by: Euler Robot 
Signed-off-by: PanNengyuan 
---
 hw/char/virtio-serial-bus.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 3325904..da9019a 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -1126,9 +1126,15 @@ static void virtio_serial_device_unrealize(DeviceState 
*dev, Error **errp)
 {
 VirtIODevice *vdev = VIRTIO_DEVICE(dev);
 VirtIOSerial *vser = VIRTIO_SERIAL(dev);
+int i;
 
 QLIST_REMOVE(vser, next);
 
+for (i = 0; i <= vser->bus.max_nr_ports; i++) {
+virtio_del_queue(vdev, 2 * i);
+virtio_del_queue(vdev, 2 * i + 1);
+}
+
 g_free(vser->ivqs);
 g_free(vser->ovqs);
 g_free(vser->ports_map);
-- 
2.7.2.windows.1

[PATCH] virtio-input: fix memory leak in virtio_input_device_unrealize()

[pannengyuan]

From: PanNengyuan 

vdev->vq[i] is forgot to cleanup in
virtio_input_device_unrealize, the memory leak stack is as bellow:

Direct leak of 3584 byte(s) in 1 object(s) allocated from:
#0 0x7f84a49f6560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
#1 0x7f84a3b3e015 in g_malloc0 (/usr/lib64/libglib-2.0.so.0+0x50015)
#2 0x559c0f0b33e7 in virtio_add_queue 
/mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio.c:2327
#3 0x559c0f205c24 in virtio_input_device_realize 
/mnt/sdb/qemu-4.2.0-rc0/hw/input/virtio-input.c:262
#4 0x559c0f0b06a7 in virtio_device_realize 
/mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio.c:3504
#5 0x559c0f1ba031 in device_set_realized  
/mnt/sdb/qemu-4.2.0-rc0/hw/core/qdev.c:876
#6 0x559c0f32cedd in property_set_bool 
/mnt/sdb/qemu-4.2.0-rc0/qom/object.c:2080
#7 0x559c0f3314ee in object_property_set_qobject 
/mnt/sdb/qemu-4.2.0-rc0/qom/qom-qobject.c:26

Direct leak of 3584 byte(s) in 1 object(s) allocated from:
#0 0x7f84a49f6560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
#1 0x7f84a3b3e015 in g_malloc0 (/usr/lib64/libglib-2.0.so.0+0x50015)
#2 0x559c0f0b33e7 in virtio_add_queue 
/mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio.c:2327
#3 0x559c0f205c3f in virtio_input_device_realize 
/mnt/sdb/qemu-4.2.0-rc0/hw/input/virtio-input.c:263
#4 0x559c0f0b06a7 in virtio_device_realize 
/mnt/sdb/qemu-4.2.0-rc0/hw/virtio/virtio.c:3504
#5 0x559c0f1ba031 in device_set_realized 
/mnt/sdb/qemu-4.2.0-rc0/hw/core/qdev.c:876
#6 0x559c0f32cedd in property_set_bool 
/mnt/sdb/qemu-4.2.0-rc0/qom/object.c:2080
#7 0x559c0f3314ee in object_property_set_qobject 
/mnt/sdb/qemu-4.2.0-rc0/qom/qom-qobject.c:26

Reported-by: Euler Robot 
Signed-off-by: PanNengyuan 
---
 hw/input/virtio-input.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/input/virtio-input.c b/hw/input/virtio-input.c
index 51617a5..da94da4 100644
--- a/hw/input/virtio-input.c
+++ b/hw/input/virtio-input.c
@@ -288,6 +288,9 @@ static void virtio_input_device_unrealize(DeviceState *dev, 
Error **errp)
 return;
 }
 }
+
+virtio_del_queue(vdev, 0);
+virtio_del_queue(vdev, 1);
 virtio_cleanup(vdev);
 }
 
-- 
2.7.2.windows.1

[PATCH V3 1/2] block/nbd: extract the common cleanup code

[pannengyuan]

From: PanNengyuan 

The BDRVNBDState cleanup code is common in two places, add
nbd_free_bdrvstate_prop() function to do these cleanups (suggested by
Stefano Garzarella).

Signed-off-by: PanNengyuan 
---
 block/nbd.c | 23 +--
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 1239761..5805979 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -94,6 +94,8 @@ typedef struct BDRVNBDState {
 
 static int nbd_client_connect(BlockDriverState *bs, Error **errp);
 
+static void nbd_free_bdrvstate_prop(BDRVNBDState *s);
+
 static void nbd_channel_error(BDRVNBDState *s, int ret)
 {
 if (ret == -EIO) {
@@ -1486,6 +1488,15 @@ static int nbd_client_connect(BlockDriverState *bs, 
Error **errp)
 }
 }
 
+static void nbd_free_bdrvstate_prop(BDRVNBDState *s)
+{
+object_unref(OBJECT(s->tlscreds));
+qapi_free_SocketAddress(s->saddr);
+g_free(s->export);
+g_free(s->tlscredsid);
+g_free(s->x_dirty_bitmap);
+}
+
 /*
  * Parse nbd_open options
  */
@@ -1855,10 +1866,7 @@ static int nbd_process_options(BlockDriverState *bs, 
QDict *options,
 
  error:
 if (ret < 0) {
-object_unref(OBJECT(s->tlscreds));
-qapi_free_SocketAddress(s->saddr);
-g_free(s->export);
-g_free(s->tlscredsid);
+nbd_free_bdrvstate_prop(s);
 }
 qemu_opts_del(opts);
 return ret;
@@ -1937,12 +1945,7 @@ static void nbd_close(BlockDriverState *bs)
 BDRVNBDState *s = bs->opaque;
 
 nbd_client_close(bs);
-
-object_unref(OBJECT(s->tlscreds));
-qapi_free_SocketAddress(s->saddr);
-g_free(s->export);
-g_free(s->tlscredsid);
-g_free(s->x_dirty_bitmap);
+nbd_free_bdrvstate_prop(s);
 }
 
 static int64_t nbd_getlength(BlockDriverState *bs)
-- 
2.7.2.windows.1

[PATCH V3 2/2] block/nbd: fix memory leak in nbd_open()

[pannengyuan]

From: PanNengyuan 

In currently implementation there will be a memory leak when
nbd_client_connect() returns error status. Here is an easy way to
reproduce:

1. run qemu-iotests as follow and check the result with asan:
./check -raw 143

Following is the asan output backtrack:
Direct leak of 40 byte(s) in 1 object(s) allocated from:
#0 0x7f629688a560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
#1 0x7f6295e7e015 in g_malloc0  (/usr/lib64/libglib-2.0.so.0+0x50015)
#2 0x56281dab4642 in qobject_input_start_struct  
/mnt/sdb/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:295
#3 0x56281dab1a04 in visit_start_struct  
/mnt/sdb/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:49
#4 0x56281dad1827 in visit_type_SocketAddress  qapi/qapi-visit-sockets.c:386
#5 0x56281da8062f in nbd_config   /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1716
#6 0x56281da8062f in nbd_process_options 
/mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1829
#7 0x56281da8062f in nbd_open /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873

Direct leak of 15 byte(s) in 1 object(s) allocated from:
#0 0x7f629688a3a0 in malloc (/usr/lib64/libasan.so.3+0xc73a0)
#1 0x7f6295e7dfbd in g_malloc (/usr/lib64/libglib-2.0.so.0+0x4ffbd)
#2 0x7f6295e96ace in g_strdup (/usr/lib64/libglib-2.0.so.0+0x68ace)
#3 0x56281da804ac in nbd_process_options 
/mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1834
#4 0x56281da804ac in nbd_open /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873

Indirect leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x7f629688a3a0 in malloc (/usr/lib64/libasan.so.3+0xc73a0)
#1 0x7f6295e7dfbd in g_malloc (/usr/lib64/libglib-2.0.so.0+0x4ffbd)
#2 0x7f6295e96ace in g_strdup (/usr/lib64/libglib-2.0.so.0+0x68ace)
#3 0x56281dab41a3 in qobject_input_type_str_keyval 
/mnt/sdb/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:536
#4 0x56281dab2ee9 in visit_type_str 
/mnt/sdb/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:297
#5 0x56281dad0fa1 in visit_type_UnixSocketAddress_members 
qapi/qapi-visit-sockets.c:141
#6 0x56281dad17b6 in visit_type_SocketAddress_members 
qapi/qapi-visit-sockets.c:366
#7 0x56281dad186a in visit_type_SocketAddress qapi/qapi-visit-sockets.c:393
#8 0x56281da8062f in nbd_config /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1716
#9 0x56281da8062f in nbd_process_options 
/mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1829
#10 0x56281da8062f in nbd_open /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873

Reported-by: Euler Robot 
Signed-off-by: PanNengyuan 
---
Changes v2 to v1:
- add a new function to do the common cleanups (suggested by Stefano
  Garzarella).
---
Changes v3 to v2:
- split in two patches(suggested by Stefano Garzarella)
---
 block/nbd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/nbd.c b/block/nbd.c
index 5805979..09d6925 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1889,6 +1889,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options, 
int flags,
 
 ret = nbd_client_connect(bs, errp);
 if (ret < 0) {
+nbd_free_bdrvstate_prop(s);
 return ret;
 }
 /* successfully connected */
-- 
2.7.2.windows.1

Re: [PATCH V2] block/nbd: fix memory leak in nbd_open()

[pannengyuan]

On 2019/11/28 21:36, Stefano Garzarella wrote:
> On Thu, Nov 28, 2019 at 08:09:31PM +0800, pannengy...@huawei.com wrote:
>> From: PanNengyuan 
>>
>> In currently implementation there will be a memory leak when
>> nbd_client_connect() returns error status. Here is an easy way to
>> reproduce:
>>
>> 1. run qemu-iotests as follow and check the result with asan:
>> ./check -raw 143
>>
>> Following is the asan output backtrack:
>> Direct leak of 40 byte(s) in 1 object(s) allocated from:
>> #0 0x7f629688a560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
>> #1 0x7f6295e7e015 in g_malloc0  (/usr/lib64/libglib-2.0.so.0+0x50015)
>> #2 0x56281dab4642 in qobject_input_start_struct  
>> /mnt/sdb/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:295
>> #3 0x56281dab1a04 in visit_start_struct  
>> /mnt/sdb/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:49
>> #4 0x56281dad1827 in visit_type_SocketAddress  
>> qapi/qapi-visit-sockets.c:386
>> #5 0x56281da8062f in nbd_config   
>> /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1716
>> #6 0x56281da8062f in nbd_process_options  
>> /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1829
>> #7 0x56281da8062f in nbd_open  /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873
>>
>> Direct leak of 15 byte(s) in 1 object(s) allocated from:
>> #0 0x7f629688a3a0 in malloc (/usr/lib64/libasan.so.3+0xc73a0)
>> #1 0x7f6295e7dfbd in g_malloc (/usr/lib64/libglib-2.0.so.0+0x4ffbd)
>> #2 0x7f6295e96ace in g_strdup (/usr/lib64/libglib-2.0.so.0+0x68ace)
>> #3 0x56281da804ac in nbd_process_options 
>> /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1834
>> #4 0x56281da804ac in nbd_open /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873
>>
>> Indirect leak of 24 byte(s) in 1 object(s) allocated from:
>> #0 0x7f629688a3a0 in malloc (/usr/lib64/libasan.so.3+0xc73a0)
>> #1 0x7f6295e7dfbd in g_malloc (/usr/lib64/libglib-2.0.so.0+0x4ffbd)
>> #2 0x7f6295e96ace in g_strdup (/usr/lib64/libglib-2.0.so.0+0x68ace)
>> #3 0x56281dab41a3 in qobject_input_type_str_keyval  
>> /mnt/sdb/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:536
>> #4 0x56281dab2ee9 in visit_type_str  
>> /mnt/sdb/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:297
>> #5 0x56281dad0fa1 in visit_type_UnixSocketAddress_members 
>> qapi/qapi-visit-sockets.c:141
>> #6 0x56281dad17b6 in visit_type_SocketAddress_members  
>> qapi/qapi-visit-sockets.c:366
>> #7 0x56281dad186a in visit_type_SocketAddress 
>> qapi/qapi-visit-sockets.c:393
>> #8 0x56281da8062f in nbd_config /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1716
>> #9 0x56281da8062f in nbd_process_options 
>> /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1829
>> #10 0x56281da8062f in nbd_open /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873
>>
>> Reported-by: Euler Robot 
>> Signed-off-by: PanNengyuan 
>> ---
>> Changes v2 to v1:
>> - add a new function to do the common cleanups (suggested by Stefano 
>> Garzarella).
>> ---
>>  block/nbd.c | 26 --
>>  1 file changed, 16 insertions(+), 10 deletions(-)
>>
>> diff --git a/block/nbd.c b/block/nbd.c
>> index 1239761..f8aa2a8 100644
>> --- a/block/nbd.c
>> +++ b/block/nbd.c
>> @@ -94,6 +94,8 @@ typedef struct BDRVNBDState {
>>  
>>  static int nbd_client_connect(BlockDriverState *bs, Error **errp);
>>  
>> +static void nbd_free_bdrvstate_prop(BDRVNBDState *s);
>> +
>>  static void nbd_channel_error(BDRVNBDState *s, int ret)
>>  {
>>  if (ret == -EIO) {
>> @@ -1486,6 +1488,17 @@ static int nbd_client_connect(BlockDriverState *bs, 
>> Error **errp)
>>  }
>>  }
>>  
>> +static void nbd_free_bdrvstate_prop(BDRVNBDState *s)
>> +{
>> +object_unref(OBJECT(s->tlscreds));
>> +qapi_free_SocketAddress(s->saddr);
>> +g_free(s->export);
>> +g_free(s->tlscredsid);
>> +if (s->x_dirty_bitmap) {
>^ it is not needed, g_free() handles NULL pointers.
>> +g_free(s->x_dirty_bitmap);
>> +}
>> +}
>> +
> 
> Please, split this patch in two patches:
> - the first patch where you add this function and use it in
>   nbd_process_options() and nbd_close()
> - the second patch where you fix the leak in nbd_open()
> 
> Thanks,
> Stefano

Thanks, I will change and split it in next version.

> 
>>  /*
>>   * Parse nbd_open options
>>   */
>> @@ -1855,10 +1868,7 @@ static int nbd_process_options(BlockDriverState *bs, 
>> QDict *options,
>>  
>>   error:
>>  if (ret < 0) 

[PATCH V2] block/nbd: fix memory leak in nbd_open()

[pannengyuan]

From: PanNengyuan 

In currently implementation there will be a memory leak when
nbd_client_connect() returns error status. Here is an easy way to
reproduce:

1. run qemu-iotests as follow and check the result with asan:
./check -raw 143

Following is the asan output backtrack:
Direct leak of 40 byte(s) in 1 object(s) allocated from:
#0 0x7f629688a560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
#1 0x7f6295e7e015 in g_malloc0  (/usr/lib64/libglib-2.0.so.0+0x50015)
#2 0x56281dab4642 in qobject_input_start_struct  
/mnt/sdb/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:295
#3 0x56281dab1a04 in visit_start_struct  
/mnt/sdb/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:49
#4 0x56281dad1827 in visit_type_SocketAddress  qapi/qapi-visit-sockets.c:386
#5 0x56281da8062f in nbd_config   /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1716
#6 0x56281da8062f in nbd_process_options  
/mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1829
#7 0x56281da8062f in nbd_open  /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873

Direct leak of 15 byte(s) in 1 object(s) allocated from:
#0 0x7f629688a3a0 in malloc (/usr/lib64/libasan.so.3+0xc73a0)
#1 0x7f6295e7dfbd in g_malloc (/usr/lib64/libglib-2.0.so.0+0x4ffbd)
#2 0x7f6295e96ace in g_strdup (/usr/lib64/libglib-2.0.so.0+0x68ace)
#3 0x56281da804ac in nbd_process_options 
/mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1834
#4 0x56281da804ac in nbd_open /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873

Indirect leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x7f629688a3a0 in malloc (/usr/lib64/libasan.so.3+0xc73a0)
#1 0x7f6295e7dfbd in g_malloc (/usr/lib64/libglib-2.0.so.0+0x4ffbd)
#2 0x7f6295e96ace in g_strdup (/usr/lib64/libglib-2.0.so.0+0x68ace)
#3 0x56281dab41a3 in qobject_input_type_str_keyval  
/mnt/sdb/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:536
#4 0x56281dab2ee9 in visit_type_str  
/mnt/sdb/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:297
#5 0x56281dad0fa1 in visit_type_UnixSocketAddress_members 
qapi/qapi-visit-sockets.c:141
#6 0x56281dad17b6 in visit_type_SocketAddress_members  
qapi/qapi-visit-sockets.c:366
#7 0x56281dad186a in visit_type_SocketAddress qapi/qapi-visit-sockets.c:393
#8 0x56281da8062f in nbd_config /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1716
#9 0x56281da8062f in nbd_process_options 
/mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1829
#10 0x56281da8062f in nbd_open /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873

Reported-by: Euler Robot 
Signed-off-by: PanNengyuan 
---
Changes v2 to v1:
- add a new function to do the common cleanups (suggested by Stefano 
Garzarella).
---
 block/nbd.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 1239761..f8aa2a8 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -94,6 +94,8 @@ typedef struct BDRVNBDState {
 
 static int nbd_client_connect(BlockDriverState *bs, Error **errp);
 
+static void nbd_free_bdrvstate_prop(BDRVNBDState *s);
+
 static void nbd_channel_error(BDRVNBDState *s, int ret)
 {
 if (ret == -EIO) {
@@ -1486,6 +1488,17 @@ static int nbd_client_connect(BlockDriverState *bs, 
Error **errp)
 }
 }
 
+static void nbd_free_bdrvstate_prop(BDRVNBDState *s)
+{
+object_unref(OBJECT(s->tlscreds));
+qapi_free_SocketAddress(s->saddr);
+g_free(s->export);
+g_free(s->tlscredsid);
+if (s->x_dirty_bitmap) {
+g_free(s->x_dirty_bitmap);
+}
+}
+
 /*
  * Parse nbd_open options
  */
@@ -1855,10 +1868,7 @@ static int nbd_process_options(BlockDriverState *bs, 
QDict *options,
 
  error:
 if (ret < 0) {
-object_unref(OBJECT(s->tlscreds));
-qapi_free_SocketAddress(s->saddr);
-g_free(s->export);
-g_free(s->tlscredsid);
+nbd_free_bdrvstate_prop(s);
 }
 qemu_opts_del(opts);
 return ret;
@@ -1881,6 +1891,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options, 
int flags,
 
 ret = nbd_client_connect(bs, errp);
 if (ret < 0) {
+nbd_free_bdrvstate_prop(s);
 return ret;
 }
 /* successfully connected */
@@ -1937,12 +1948,7 @@ static void nbd_close(BlockDriverState *bs)
 BDRVNBDState *s = bs->opaque;
 
 nbd_client_close(bs);
-
-object_unref(OBJECT(s->tlscreds));
-qapi_free_SocketAddress(s->saddr);
-g_free(s->export);
-g_free(s->tlscredsid);
-g_free(s->x_dirty_bitmap);
+nbd_free_bdrvstate_prop(s);
 }
 
 static int64_t nbd_getlength(BlockDriverState *bs)
-- 
2.7.2.windows.1

Re: [PATCH] block/nbd: fix memory leak in nbd_open()

[pannengyuan]

Thanks for you suggestion, I'd be glad to do it, I will send a new
version later.

Cheers.

On 2019/11/28 18:41, Stefano Garzarella wrote:
> On Thu, Nov 28, 2019 at 06:32:49PM +0800, pannengyuan wrote:
>> Hi,
>> I think it's a better way, you can implement this new function before
>> this patch.
> 
> If you want to do it, so you can send everything together, for me there's
> no problem, it was just a suggestion.
> 
> If you don't have time, I can do it.
> 
> Cheers,
> Stefano
> 
>>
>> Thanks.
>>
>> On 2019/11/28 17:01, Stefano Garzarella wrote:
>>> On Thu, Nov 28, 2019 at 04:40:10PM +0800, pannengy...@huawei.com wrote:
>>>
>>> Hi,
>>> I don't know nbd code very well, the patch LGTM, but just a comment
>>> below:
>>>
>>>> From: PanNengyuan 
>>>>
>>>> In currently implementation there will be a memory leak when
>>>> nbd_client_connect() returns error status. Here is an easy way to 
>>>> reproduce:
>>>>
>>>> 1. run qemu-iotests as follow and check the result with asan:
>>>> ./check -raw 143
>>>>
>>>> Following is the asan output backtrack:
>>>> Direct leak of 40 byte(s) in 1 object(s) allocated from:
>>>> #0 0x7f629688a560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
>>>> #1 0x7f6295e7e015 in g_malloc0  (/usr/lib64/libglib-2.0.so.0+0x50015)
>>>> #2 0x56281dab4642 in qobject_input_start_struct  
>>>> /mnt/sdb/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:295
>>>> #3 0x56281dab1a04 in visit_start_struct  
>>>> /mnt/sdb/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:49
>>>> #4 0x56281dad1827 in visit_type_SocketAddress  
>>>> qapi/qapi-visit-sockets.c:386
>>>> #5 0x56281da8062f in nbd_config  
>>>> /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1716
>>>> #6 0x56281da8062f in nbd_process_options  
>>>> /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1829
>>>> #7 0x56281da8062f in nbd_open  /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873
>>>>
>>>> Direct leak of 15 byte(s) in 1 object(s) allocated from:
>>>> #0 0x7f629688a3a0 in malloc (/usr/lib64/libasan.so.3+0xc73a0)
>>>> #1 0x7f6295e7dfbd in g_malloc  (/usr/lib64/libglib-2.0.so.0+0x4ffbd)
>>>> #2 0x7f6295e96ace in g_strdup  (/usr/lib64/libglib-2.0.so.0+0x68ace)
>>>> #3 0x56281da804ac in nbd_process_options 
>>>> /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1834
>>>> #4 0x56281da804ac in nbd_open  /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873
>>>>
>>>> Indirect leak of 24 byte(s) in 1 object(s) allocated from:
>>>> #0 0x7f629688a3a0 in malloc (/usr/lib64/libasan.so.3+0xc73a0)
>>>> #1 0x7f6295e7dfbd in g_malloc (/usr/lib64/libglib-2.0.so.0+0x4ffbd)
>>>> #2 0x7f6295e96ace in g_strdup (/usr/lib64/libglib-2.0.so.0+0x68ace)
>>>> #3 0x56281dab41a3 in qobject_input_type_str_keyval   
>>>> /mnt/sdb/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:536
>>>> #4 0x56281dab2ee9 in visit_type_str   
>>>> /mnt/sdb/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:297
>>>>     #5 0x56281dad0fa1 in visit_type_UnixSocketAddress_members  
>>>> qapi/qapi-visit-sockets.c:141
>>>> #6 0x56281dad17b6 in visit_type_SocketAddress_members  
>>>> qapi/qapi-visit-sockets.c:366
>>>> #7 0x56281dad186a in visit_type_SocketAddress 
>>>> qapi/qapi-visit-sockets.c:393
>>>> #8 0x56281da8062f in nbd_config   
>>>> /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1716
>>>> #9 0x56281da8062f in nbd_process_options   
>>>> /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1829
>>>> #10 0x56281da8062f in nbd_open  
>>>> /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873
>>>>
>>>> Reported-by: Euler Robot 
>>>> Signed-off-by: PanNengyuan 
>>>> ---
>>>>  block/nbd.c | 5 +
>>>>  1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/block/nbd.c b/block/nbd.c
>>>> index 1239761..bc40a25 100644
>>>> --- a/block/nbd.c
>>>> +++ b/block/nbd.c
>>>> @@ -1881,6 +1881,11 @@ static int nbd_open(BlockDriverState *bs, QDict 
>>>> *options, int flags,
>>>>  
>>>>  ret = nbd_client_connect(bs, errp);
>>>>  if (ret < 0) {
>>>> +object_unref(OBJECT(s->tlscreds));
>>>> +qapi_free_SocketAddress(s->saddr);
>>>> +g_free(s->export);
>>>> +g_free(s->tlscredsid);
>>>> +g_free(s->x_dirty_bitmap);
>>>
>>> Since with this patch we are doing these cleanups in 3 places (here,
>>> nbd_close(), and nbd_process_options()), should be better to add a new
>>> function to do these cleanups?
>>>
>>> Maybe I'd create a series adding a patch before this one, implementing this
>>> new function, and change this patch calling it.
>>>
>>> Thanks,
>>> Stefano
>>>
>>>
>>> .
>>>
>>
> 

Re: [PATCH] block/nbd: fix memory leak in nbd_open()

[pannengyuan]

Hi,
I think it's a better way, you can implement this new function before
this patch.

Thanks.

On 2019/11/28 17:01, Stefano Garzarella wrote:
> On Thu, Nov 28, 2019 at 04:40:10PM +0800, pannengy...@huawei.com wrote:
> 
> Hi,
> I don't know nbd code very well, the patch LGTM, but just a comment
> below:
> 
>> From: PanNengyuan 
>>
>> In currently implementation there will be a memory leak when
>> nbd_client_connect() returns error status. Here is an easy way to reproduce:
>>
>> 1. run qemu-iotests as follow and check the result with asan:
>> ./check -raw 143
>>
>> Following is the asan output backtrack:
>> Direct leak of 40 byte(s) in 1 object(s) allocated from:
>> #0 0x7f629688a560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
>> #1 0x7f6295e7e015 in g_malloc0  (/usr/lib64/libglib-2.0.so.0+0x50015)
>> #2 0x56281dab4642 in qobject_input_start_struct  
>> /mnt/sdb/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:295
>> #3 0x56281dab1a04 in visit_start_struct  
>> /mnt/sdb/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:49
>> #4 0x56281dad1827 in visit_type_SocketAddress  
>> qapi/qapi-visit-sockets.c:386
>> #5 0x56281da8062f in nbd_config  /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1716
>> #6 0x56281da8062f in nbd_process_options  
>> /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1829
>> #7 0x56281da8062f in nbd_open  /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873
>>
>> Direct leak of 15 byte(s) in 1 object(s) allocated from:
>> #0 0x7f629688a3a0 in malloc (/usr/lib64/libasan.so.3+0xc73a0)
>> #1 0x7f6295e7dfbd in g_malloc  (/usr/lib64/libglib-2.0.so.0+0x4ffbd)
>> #2 0x7f6295e96ace in g_strdup  (/usr/lib64/libglib-2.0.so.0+0x68ace)
>> #3 0x56281da804ac in nbd_process_options 
>> /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1834
>> #4 0x56281da804ac in nbd_open  /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873
>>
>> Indirect leak of 24 byte(s) in 1 object(s) allocated from:
>> #0 0x7f629688a3a0 in malloc (/usr/lib64/libasan.so.3+0xc73a0)
>> #1 0x7f6295e7dfbd in g_malloc (/usr/lib64/libglib-2.0.so.0+0x4ffbd)
>> #2 0x7f6295e96ace in g_strdup (/usr/lib64/libglib-2.0.so.0+0x68ace)
>> #3 0x56281dab41a3 in qobject_input_type_str_keyval   
>> /mnt/sdb/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:536
>> #4 0x56281dab2ee9 in visit_type_str   
>> /mnt/sdb/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:297
>> #5 0x56281dad0fa1 in visit_type_UnixSocketAddress_members  
>> qapi/qapi-visit-sockets.c:141
>> #6 0x56281dad17b6 in visit_type_SocketAddress_members  
>> qapi/qapi-visit-sockets.c:366
>> #7 0x56281dad186a in visit_type_SocketAddress 
>> qapi/qapi-visit-sockets.c:393
>> #8 0x56281da8062f in nbd_config   
>> /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1716
>> #9 0x56281da8062f in nbd_process_options   
>> /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1829
>> #10 0x56281da8062f in nbd_open  /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873
>>
>> Reported-by: Euler Robot 
>> Signed-off-by: PanNengyuan 
>> ---
>>  block/nbd.c | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/block/nbd.c b/block/nbd.c
>> index 1239761..bc40a25 100644
>> --- a/block/nbd.c
>> +++ b/block/nbd.c
>> @@ -1881,6 +1881,11 @@ static int nbd_open(BlockDriverState *bs, QDict 
>> *options, int flags,
>>  
>>  ret = nbd_client_connect(bs, errp);
>>  if (ret < 0) {
>> +object_unref(OBJECT(s->tlscreds));
>> +qapi_free_SocketAddress(s->saddr);
>> +g_free(s->export);
>> +g_free(s->tlscredsid);
>> +g_free(s->x_dirty_bitmap);
> 
> Since with this patch we are doing these cleanups in 3 places (here,
> nbd_close(), and nbd_process_options()), should be better to add a new
> function to do these cleanups?
> 
> Maybe I'd create a series adding a patch before this one, implementing this
> new function, and change this patch calling it.
> 
> Thanks,
> Stefano
> 
> 
> .
> 

[PATCH] block/nbd: fix memory leak in nbd_open()

[pannengyuan]

From: PanNengyuan 

In currently implementation there will be a memory leak when
nbd_client_connect() returns error status. Here is an easy way to reproduce:

1. run qemu-iotests as follow and check the result with asan:
./check -raw 143

Following is the asan output backtrack:
Direct leak of 40 byte(s) in 1 object(s) allocated from:
#0 0x7f629688a560 in calloc (/usr/lib64/libasan.so.3+0xc7560)
#1 0x7f6295e7e015 in g_malloc0  (/usr/lib64/libglib-2.0.so.0+0x50015)
#2 0x56281dab4642 in qobject_input_start_struct  
/mnt/sdb/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:295
#3 0x56281dab1a04 in visit_start_struct  
/mnt/sdb/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:49
#4 0x56281dad1827 in visit_type_SocketAddress  qapi/qapi-visit-sockets.c:386
#5 0x56281da8062f in nbd_config  /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1716
#6 0x56281da8062f in nbd_process_options  
/mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1829
#7 0x56281da8062f in nbd_open  /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873

Direct leak of 15 byte(s) in 1 object(s) allocated from:
#0 0x7f629688a3a0 in malloc (/usr/lib64/libasan.so.3+0xc73a0)
#1 0x7f6295e7dfbd in g_malloc  (/usr/lib64/libglib-2.0.so.0+0x4ffbd)
#2 0x7f6295e96ace in g_strdup  (/usr/lib64/libglib-2.0.so.0+0x68ace)
#3 0x56281da804ac in nbd_process_options 
/mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1834
#4 0x56281da804ac in nbd_open  /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873

Indirect leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x7f629688a3a0 in malloc (/usr/lib64/libasan.so.3+0xc73a0)
#1 0x7f6295e7dfbd in g_malloc (/usr/lib64/libglib-2.0.so.0+0x4ffbd)
#2 0x7f6295e96ace in g_strdup (/usr/lib64/libglib-2.0.so.0+0x68ace)
#3 0x56281dab41a3 in qobject_input_type_str_keyval   
/mnt/sdb/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:536
#4 0x56281dab2ee9 in visit_type_str   
/mnt/sdb/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:297
#5 0x56281dad0fa1 in visit_type_UnixSocketAddress_members  
qapi/qapi-visit-sockets.c:141
#6 0x56281dad17b6 in visit_type_SocketAddress_members  
qapi/qapi-visit-sockets.c:366
#7 0x56281dad186a in visit_type_SocketAddress 
qapi/qapi-visit-sockets.c:393
#8 0x56281da8062f in nbd_config   /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1716
#9 0x56281da8062f in nbd_process_options   
/mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1829
#10 0x56281da8062f in nbd_open  /mnt/sdb/qemu-4.2.0-rc0/block/nbd.c:1873

Reported-by: Euler Robot 
Signed-off-by: PanNengyuan 
---
 block/nbd.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/block/nbd.c b/block/nbd.c
index 1239761..bc40a25 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -1881,6 +1881,11 @@ static int nbd_open(BlockDriverState *bs, QDict 
*options, int flags,
 
 ret = nbd_client_connect(bs, errp);
 if (ret < 0) {
+object_unref(OBJECT(s->tlscreds));
+qapi_free_SocketAddress(s->saddr);
+g_free(s->export);
+g_free(s->tlscredsid);
+g_free(s->x_dirty_bitmap);
 return ret;
 }
 /* successfully connected */
-- 
2.7.2.windows.1

[PATCH V2] throttle-groups: fix memory leak in throttle_group_set_limit:

[pannengyuan]

From: PanNengyuan 

This avoid a memory leak when qom-set is called to set throttle_group
limits, here is an easy way to reproduce:

1. run qemu-iotests as follow and check the result with asan:
   ./check -qcow2 184

Following is the asan output backtrack:
Direct leak of 912 byte(s) in 3 object(s) allocated from:
#0 0x8d7ab3c3 in __interceptor_calloc   (/lib64/libasan.so.4+0xd33c3)
#1 0x8d4c31cb in g_malloc0   (/lib64/libglib-2.0.so.0+0x571cb)
#2 0x190c857 in qobject_input_start_struct  
/mnt/sdc/qemu-master/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:295
#3 0x19070df in visit_start_struct   
/mnt/sdc/qemu-master/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:49
#4 0x1948b87 in visit_type_ThrottleLimits   
qapi/qapi-visit-block-core.c:3759
#5 0x17e4aa3 in throttle_group_set_limits   
/mnt/sdc/qemu-master/qemu-4.2.0-rc0/block/throttle-groups.c:900
#6 0x1650eff in object_property_set 
/mnt/sdc/qemu-master/qemu-4.2.0-rc0/qom/object.c:1272
#7 0x1658517 in object_property_set_qobject   
/mnt/sdc/qemu-master/qemu-4.2.0-rc0/qom/qom-qobject.c:26
#8 0x15880bb in qmp_qom_set  
/mnt/sdc/qemu-master/qemu-4.2.0-rc0/qom/qom-qmp-cmds.c:74
#9 0x157e3e3 in qmp_marshal_qom_set  qapi/qapi-commands-qom.c:154

Reported-by: Euler Robot 
Signed-off-by: PanNengyuan 
---
Changes v2 to v1:
- remove unused var 'arg' (suggested by Alberto Garcia)
---
 block/throttle-groups.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 77014c7..37695b0 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -893,8 +893,7 @@ static void throttle_group_set_limits(Object *obj, Visitor 
*v,
 {
 ThrottleGroup *tg = THROTTLE_GROUP(obj);
 ThrottleConfig cfg;
-ThrottleLimits arg = { 0 };
-ThrottleLimits *argp = 
+ThrottleLimits *argp;
 Error *local_err = NULL;
 
 visit_type_ThrottleLimits(v, name, , _err);
@@ -912,6 +911,7 @@ static void throttle_group_set_limits(Object *obj, Visitor 
*v,
 unlock:
 qemu_mutex_unlock(>lock);
 ret:
+qapi_free_ThrottleLimits(argp);
 error_propagate(errp, local_err);
 return;
 }
-- 
2.7.2.windows.1

Re: [PATCH] throttle-groups: fix memory leak in throttle_group_set_limits

[pannengyuan]

Thanks, I think it can be removed, I will send a new version later.

On 2019/11/26 17:59, Alberto Garcia wrote:
> On Tue 26 Nov 2019 09:17:02 AM CET, pannengy...@huawei.com wrote:
>> --- a/block/throttle-groups.c
>> +++ b/block/throttle-groups.c
>> @@ -912,6 +912,7 @@ static void throttle_group_set_limits(Object *obj, 
>> Visitor *v,
>>  unlock:
>>  qemu_mutex_unlock(>lock);
>>  ret:
>> +qapi_free_ThrottleLimits(argp);
>>  error_propagate(errp, local_err);
>>  return;
> 
> Thanks, but I also think that 'arg' is not used so it can be removed?
> 
> diff --git a/block/throttle-groups.c b/block/throttle-groups.c
> index 77014c741b..37695b0cd7 100644
> --- a/block/throttle-groups.c
> +++ b/block/throttle-groups.c
> @@ -893,8 +893,7 @@ static void throttle_group_set_limits(Object *obj, 
> Visitor *v,
>  {
>  ThrottleGroup *tg = THROTTLE_GROUP(obj);
>  ThrottleConfig cfg;
> -ThrottleLimits arg = { 0 };
> -ThrottleLimits *argp = 
> +ThrottleLimits *argp;
>  Error *local_err = NULL;
>  
>  visit_type_ThrottleLimits(v, name, , _err);
> @@ -912,6 +911,7 @@ static void throttle_group_set_limits(Object *obj, 
> Visitor *v,
>  unlock:
>  qemu_mutex_unlock(>lock);
>  ret:
> +qapi_free_ThrottleLimits(argp);
>  error_propagate(errp, local_err);
>  return;
>  }
> 
> Berto
> 
> .
> 

[PATCH] throttle-groups: fix memory leak in throttle_group_set_limits

[pannengyuan]

From: PanNengyuan 

This avoid a memory leak when qom-set is called to set throttle_group
limits, here is an easy way to reproduce:

1. run qemu-iotests as follow and check the result with asan:
   ./check -qcow2 184

Following is the asan output backtrack:
Direct leak of 912 byte(s) in 3 object(s) allocated from:
#0 0x8d7ab3c3 in __interceptor_calloc(/lib64/libasan.so.4+0xd33c3)
#1 0x8d4c31cb in g_malloc0 (/lib64/libglib-2.0.so.0+0x571cb)
#2 0x190c857 in qobject_input_start_struct  
/mnt/sdc/qemu-master/qemu-4.2.0-rc0/qapi/qobject-input-visitor.c:295
#3 0x19070df in visit_start_struct 
/mnt/sdc/qemu-master/qemu-4.2.0-rc0/qapi/qapi-visit-core.c:49
#4 0x1948b87 in visit_type_ThrottleLimits  qapi/qapi-visit-block-core.c:3759
#5 0x17e4aa3 in throttle_group_set_limits  
/mnt/sdc/qemu-master/qemu-4.2.0-rc0/block/throttle-groups.c:900
#6 0x1650eff in object_property_set  
/mnt/sdc/qemu-master/qemu-4.2.0-rc0/qom/object.c:1272
#7 0x1658517 in object_property_set_qobject  
/mnt/sdc/qemu-master/qemu-4.2.0-rc0/qom/qom-qobject.c:26
#8 0x15880bb in qmp_qom_set 
/mnt/sdc/qemu-master/qemu-4.2.0-rc0/qom/qom-qmp-cmds.c:74
#9 0x157e3e3 in qmp_marshal_qom_set  qapi/qapi-commands-qom.c:154

Reported-by: Euler Robot 
Signed-off-by: PanNengyuan 
---
 block/throttle-groups.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 77014c7..88418e6 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -912,6 +912,7 @@ static void throttle_group_set_limits(Object *obj, Visitor 
*v,
 unlock:
 qemu_mutex_unlock(>lock);
 ret:
+qapi_free_ThrottleLimits(argp);
 error_propagate(errp, local_err);
 return;
 }
-- 
2.7.2.windows.1

[PATCH V2] ppc/spapr_events: fix potential NULL pointer dereference in rtas_event_log_dequeue

[pannengyuan]

From: PanNengyuan 

This fixes coverity issues 68911917:
360
CID 68911917: (NULL_RETURNS)
361. dereference: Dereferencing "source", which is known to be
 "NULL".
361if (source->mask & event_mask) {
362break;
363}

Reported-by: Euler Robot 
Signed-off-by: PanNengyuan 
---
 hw/ppc/spapr_events.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 0e4c195..e355e00 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -358,6 +358,7 @@ static SpaprEventLogEntry 
*rtas_event_log_dequeue(SpaprMachineState *spapr,
 rtas_event_log_to_source(spapr,
  spapr_event_log_entry_type(entry));
 
+g_assert(source);
 if (source->mask & event_mask) {
 break;
 }
-- 
2.7.2.windows.1

Re: [Qemu-devel][PATCH] ppc/spapr_events: fix potential NULL pointer dereference in rtas_event_log_dequeue

[pannengyuan]

Thanks for you reply.
I think you are right, I will send a new version later.

On 2019/11/19 10:50, David Gibson wrote:
> On Mon, Nov 18, 2019 at 09:50:13AM +0800, pannengy...@huawei.com wrote:
>> From: PanNengyuan 
>>
>> source is being dereferenced before it is null checked, hence there is a
>> potential null pointer dereference.
>>
>> This fixes:
>> 360
>> CID 68911917: (NULL_RETURNS)
>> 361. dereference: Dereferencing "source", which is known to be
>> "NULL".
>> 361if (source->mask & event_mask) {
>>     362    break;
>> 363}
>>
>> Reported-by: Euler Robot 
>> Signed-off-by: PanNengyuan 
> 
> I don't think this is the right solution.  The only events we ever
> generated are LOG_TYPE_EPOW and LOG_TYPE_HOTPLUG, so in fact source
> should never be NULL.
> 
> I think the correct way to satisfy Coverity here is to have
> rtas_event_log_to_source() do an assert(), rather than returning NULL
> for other event types.
> 
>> ---
>>  hw/ppc/spapr_events.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
>> index 0e4c195..febd2ef 100644
>> --- a/hw/ppc/spapr_events.c
>> +++ b/hw/ppc/spapr_events.c
>> @@ -358,7 +358,7 @@ static SpaprEventLogEntry 
>> *rtas_event_log_dequeue(SpaprMachineState *spapr,
>>  rtas_event_log_to_source(spapr,
>>   spapr_event_log_entry_type(entry));
>>  
>> -if (source->mask & event_mask) {
>> +if (source && (source->mask & event_mask)) {
>>  break;
>>  }
>>  }
> 

[QEMU-DEVEL][PATCH] gpio: fix memory leak in aspeed_gpio_init()

[pannengyuan]

From: PanNengyuan 

Address Sanitizer shows memory leak in hw/gpio/aspeed_gpio.c:875

Reported-by: Euler Robot 
Signed-off-by: PanNengyuan 
---
 hw/gpio/aspeed_gpio.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c
index 7acc5fa..41e11ea 100644
--- a/hw/gpio/aspeed_gpio.c
+++ b/hw/gpio/aspeed_gpio.c
@@ -876,6 +876,7 @@ static void aspeed_gpio_init(Object *obj)
pin_idx % GPIOS_PER_GROUP);
 object_property_add(obj, name, "bool", aspeed_gpio_get_pin,
 aspeed_gpio_set_pin, NULL, NULL, NULL);
+g_free(name);
 }
 }
 
-- 
2.7.2.windows.1

[Qemu-devel][PATCH] ppc/spapr_events: fix potential NULL pointer dereference in rtas_event_log_dequeue

[pannengyuan]

From: PanNengyuan 

source is being dereferenced before it is null checked, hence there is a
potential null pointer dereference.

This fixes:
360
CID 68911917: (NULL_RETURNS)
361. dereference: Dereferencing "source", which is known to be
"NULL".
361if (source->mask & event_mask) {
362break;
363}

Reported-by: Euler Robot 
Signed-off-by: PanNengyuan 
---
 hw/ppc/spapr_events.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_events.c b/hw/ppc/spapr_events.c
index 0e4c195..febd2ef 100644
--- a/hw/ppc/spapr_events.c
+++ b/hw/ppc/spapr_events.c
@@ -358,7 +358,7 @@ static SpaprEventLogEntry 
*rtas_event_log_dequeue(SpaprMachineState *spapr,
 rtas_event_log_to_source(spapr,
  spapr_event_log_entry_type(entry));
 
-if (source->mask & event_mask) {
+if (source && (source->mask & event_mask)) {
 break;
 }
 }
-- 
2.7.2.windows.1