[PATCH] target/i386/hax-posix: fix two 'format-truncation' compile warnings
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
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
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(&s->vhost_user); } @@ -478,12 +480,13 @@ static void vhost_user_blk_device_unrealize(DeviceState *dev, Error **errp) NULL, NULL, NULL, false); vhost_dev_cleanup(&s->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(&s->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
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(&s->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(&s->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(&s->vhost_user); } -- 2.18.2
[PATCH] migration/savevm: release gslist after dump_vmstate_json
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
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, &s->rtc_irq); + +memory_region_init_io(&s->iomem, obj, &pxa2xx_rtc_ops, s, + "pxa2xx-rtc", 0x1); +sysbus_init_mmio(dev, &s->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, &s->rtc_irq); - -memory_region_init_io(&s->iomem, obj, &pxa2xx_rtc_ops, s, - "pxa2xx-rtc", 0x1); -sysbus_init_mmio(dev, &s->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 = &vmstate_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 = &vmstate_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(&tm); 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, &s->rtc_irq); sysbus_init_irq(dev, &s->rtc_hz_irq); @@ -410,6 +407,13 @@ static void strongarm_rtc_init(Object *obj) sysbus_init_mmio(dev, &s->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 = &vmstate_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, &s->iomem); sysbus_init_irq(dev, &s->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 =
[PATCH v2 1/2] s390x: fix memleaks in cpu_finalize
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, &scc->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.
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
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, &s->rtc_irq); + +memory_region_init_io(&s->iomem, obj, &pxa2xx_rtc_ops, s, + "pxa2xx-rtc", 0x1); +sysbus_init_mmio(dev, &s->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, &s->rtc_irq); - -memory_region_init_io(&s->iomem, obj, &pxa2xx_rtc_ops, s, - "pxa2xx-rtc", 0x1); -sysbus_init_mmio(dev, &s->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 = &vmstate_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 = &vmstate_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(&tm); 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, &s->rtc_irq); sysbus_init_irq(dev, &s->rtc_hz_irq); @@ -410,6 +407,13 @@ static void strongarm_rtc_init(Object *obj) sysbus_init_mmio(dev, &s->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 = &vmstate_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, &s->iomem); sysbus_init_irq(dev, &s->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,
[PATCH 1/2] s390x: fix memleaks in cpu_finalize
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
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
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 2/2] vhost-use-blk: convert to new virtio_delete_queue
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(&s->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(&s->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 1/2] vhost-user-blk: delete virtioqueues in unrealize to fix memleaks
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(&s->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(&s->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(&s->vhost_user); } -- 2.21.0.windows.1
[PATCH] migration-test: fix some memleaks in migration-test
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
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, &s->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, &s->opaque[0]); s->timer[1] = timer_new_ns(QEMU_CLOCK_VIRTUAL, gptm_tick, &s->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 = &vmstate_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
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(&s->iomem, obj, &systick_ops, s, "systick", 0xe0); sysbus_init_mmio(sbd, &s->iomem); sysbus_init_irq(sbd, &s->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 = &vmstate_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
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(&s->iomem, obj, &stm32f2xx_timer_ops, s, "stm32f2xx_timer", 0x400); sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->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 = &vmstate_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
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
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(&tm, 0); s->tick_offset = mktimegm(&tm) - 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 = &vmstate_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
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
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
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()
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
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
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
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
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 = &v->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
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
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 = &v->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
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
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(&vsock->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(&vsock->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
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
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(&vsock->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
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 = &v->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
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()
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
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()
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()
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
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
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
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
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
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
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
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()
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
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
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
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
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 &vdev->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(&vdev->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()
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
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
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
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()
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
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
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
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 &vdev->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(&vdev->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
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()
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
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
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
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
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
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 &vdev->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(&vdev->vq[n]); > } > > static void virtio_set_isr(VirtIODevice *vdev, int value) > > > . >
[PATCH] virtio-balloon: fix memory leak while attach virtio-balloon device
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
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
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()
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
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()
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()
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()
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()
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()
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()
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:
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 = &arg; +ThrottleLimits *argp; Error *local_err = NULL; visit_type_ThrottleLimits(v, name, &argp, &local_err); @@ -912,6 +911,7 @@ static void throttle_group_set_limits(Object *obj, Visitor *v, unlock: qemu_mutex_unlock(&tg->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
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(&tg->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 = &arg; > +ThrottleLimits *argp; > Error *local_err = NULL; > > visit_type_ThrottleLimits(v, name, &argp, &local_err); > @@ -912,6 +911,7 @@ static void throttle_group_set_limits(Object *obj, > Visitor *v, > unlock: > qemu_mutex_unlock(&tg->lock); > ret: > +qapi_free_ThrottleLimits(argp); > error_propagate(errp, local_err); > return; > } > > Berto > > . >
[PATCH] throttle-groups: fix memory leak in throttle_group_set_limits
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(&tg->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
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
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()
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
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