Re: [PATCH 2/4] vhost-user-blk: fix invalid memory access
On Wed, Apr 15, 2020 at 11:28:24AM +0800, Li Feng wrote: > > when s->inflight is freed, vhost_dev_free_inflight may try to access > s->inflight->addr, it will retrigger the following issue. > > ==7309==ERROR: AddressSanitizer: heap-use-after-free on address > 0x604001020d18 at pc 0x55ce948a bp 0x7fffb170 sp 0x7fffb160 > READ of size 8 at 0x604001020d18 thread T0 > #0 0x55ce9489 in vhost_dev_free_inflight > /root/smartx/qemu-el7/qemu-test/hw/virtio/vhost.c:1473 > #1 0x55cd86eb in virtio_reset > /root/smartx/qemu-el7/qemu-test/hw/virtio/virtio.c:1214 > #2 0x560d3eff in virtio_pci_reset hw/virtio/virtio-pci.c:1859 > #3 0x55f2ac53 in device_set_realized hw/core/qdev.c:893 > #4 0x561d572c in property_set_bool qom/object.c:1925 > #5 0x561de8de in object_property_set_qobject qom/qom-qobject.c:27 > #6 0x561d99f4 in object_property_set_bool qom/object.c:1188 > #7 0x55e50ae7 in qdev_device_add > /root/smartx/qemu-el7/qemu-test/qdev-monitor.c:626 > #8 0x55e51213 in qmp_device_add > /root/smartx/qemu-el7/qemu-test/qdev-monitor.c:806 > #9 0x55e8ff40 in hmp_device_add > /root/smartx/qemu-el7/qemu-test/hmp.c:1951 > #10 0x55be889a in handle_hmp_command > /root/smartx/qemu-el7/qemu-test/monitor.c:3404 > #11 0x55beac8b in monitor_command_cb > /root/smartx/qemu-el7/qemu-test/monitor.c:4296 > #12 0x56433eb7 in readline_handle_byte util/readline.c:393 > #13 0x55be89ec in monitor_read > /root/smartx/qemu-el7/qemu-test/monitor.c:4279 > #14 0x563285cc in tcp_chr_read chardev/char-socket.c:470 > #15 0x7670b968 in g_main_context_dispatch > (/lib64/libglib-2.0.so.0+0x4a968) > #16 0x5640727c in glib_pollfds_poll util/main-loop.c:215 > #17 0x5640727c in os_host_main_loop_wait util/main-loop.c:238 > #18 0x5640727c in main_loop_wait util/main-loop.c:497 > #19 0x55b2d0bf in main_loop /root/smartx/qemu-el7/qemu-test/vl.c:2013 > #20 0x55b2d0bf in main /root/smartx/qemu-el7/qemu-test/vl.c:4776 > #21 0x7fffdd2eb444 in __libc_start_main (/lib64/libc.so.6+0x22444) > #22 0x55b3767a > (/root/smartx/qemu-el7/qemu-test/x86_64-softmmu/qemu-system-x86_64+0x5e367a) > > 0x604001020d18 is located 8 bytes inside of 40-byte region > [0x604001020d10,0x604001020d38) > freed by thread T0 here: > #0 0x76f00508 in __interceptor_free (/lib64/libasan.so.4+0xde508) > #1 0x7671107d in g_free (/lib64/libglib-2.0.so.0+0x5007d) > > previously allocated by thread T0 here: > #0 0x76f00a88 in __interceptor_calloc (/lib64/libasan.so.4+0xdea88) > #1 0x76710fc5 in g_malloc0 (/lib64/libglib-2.0.so.0+0x4ffc5) > > SUMMARY: AddressSanitizer: heap-use-after-free > /root/smartx/qemu-el7/qemu-test/hw/virtio/vhost.c:1473 in > vhost_dev_free_inflight > Shadow bytes around the buggy address: > 0x0c08801fc150: fa fa 00 00 00 00 04 fa fa fa fd fd fd fd fd fa > 0x0c08801fc160: fa fa fd fd fd fd fd fd fa fa 00 00 00 00 04 fa > 0x0c08801fc170: fa fa 00 00 00 00 00 01 fa fa 00 00 00 00 04 fa > 0x0c08801fc180: fa fa 00 00 00 00 00 01 fa fa 00 00 00 00 00 01 > 0x0c08801fc190: fa fa 00 00 00 00 00 fa fa fa 00 00 00 00 04 fa > =>0x0c08801fc1a0: fa fa fd[fd]fd fd fd fa fa fa fd fd fd fd fd fa > 0x0c08801fc1b0: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fa > 0x0c08801fc1c0: fa fa 00 00 00 00 00 fa fa fa fd fd fd fd fd fd > 0x0c08801fc1d0: fa fa 00 00 00 00 00 01 fa fa fd fd fd fd fd fa > 0x0c08801fc1e0: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fd > 0x0c08801fc1f0: fa fa 00 00 00 00 00 01 fa fa fd fd fd fd fd fa > Shadow byte legend (one shadow byte represents 8 application bytes): > Addressable: 00 > Partially addressable: 01 02 03 04 05 06 07 > Heap left redzone: fa > Freed heap region: fd > Stack left redzone: f1 > Stack mid redzone: f2 > Stack right redzone: f3 > Stack after return: f5 > Stack use after scope: f8 > Global redzone: f9 > Global init order: f6 > Poisoned by user:f7 > Container overflow: fc > Array cookie:ac > Intra object redzone:bb > ASan internal: fe > Left alloca redzone: ca > Right alloca redzone:cb > ==7309==ABORTING > > Signed-off-by: Li Feng Reviewed-by: Raphael Norwitz > --- > hw/block/vhost-user-blk.c | 4 > hw/virtio/vhost.c | 2 +- > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c > index 776b9af3eb..19e79b96e4 100644 > --- a/hw/block/vhost-user-blk.c > +++ b/hw/block/vhost-user-blk.c > @@ -463,7 +463,9 @@ reconnect: > > virtio_err: > g_free(s->vhost_vqs); > +s->vhost_vqs = NULL; > g_free(s->inflight); > +s->inflight = NULL; > for (i = 0; i < s->num_queues; i++) { > virtio_delete_queue(s->virtqs[i]); > } > @@ -484,7 +486,9 @@ static
[PATCH 2/4] vhost-user-blk: fix invalid memory access
when s->inflight is freed, vhost_dev_free_inflight may try to access s->inflight->addr, it will retrigger the following issue. ==7309==ERROR: AddressSanitizer: heap-use-after-free on address 0x604001020d18 at pc 0x55ce948a bp 0x7fffb170 sp 0x7fffb160 READ of size 8 at 0x604001020d18 thread T0 #0 0x55ce9489 in vhost_dev_free_inflight /root/smartx/qemu-el7/qemu-test/hw/virtio/vhost.c:1473 #1 0x55cd86eb in virtio_reset /root/smartx/qemu-el7/qemu-test/hw/virtio/virtio.c:1214 #2 0x560d3eff in virtio_pci_reset hw/virtio/virtio-pci.c:1859 #3 0x55f2ac53 in device_set_realized hw/core/qdev.c:893 #4 0x561d572c in property_set_bool qom/object.c:1925 #5 0x561de8de in object_property_set_qobject qom/qom-qobject.c:27 #6 0x561d99f4 in object_property_set_bool qom/object.c:1188 #7 0x55e50ae7 in qdev_device_add /root/smartx/qemu-el7/qemu-test/qdev-monitor.c:626 #8 0x55e51213 in qmp_device_add /root/smartx/qemu-el7/qemu-test/qdev-monitor.c:806 #9 0x55e8ff40 in hmp_device_add /root/smartx/qemu-el7/qemu-test/hmp.c:1951 #10 0x55be889a in handle_hmp_command /root/smartx/qemu-el7/qemu-test/monitor.c:3404 #11 0x55beac8b in monitor_command_cb /root/smartx/qemu-el7/qemu-test/monitor.c:4296 #12 0x56433eb7 in readline_handle_byte util/readline.c:393 #13 0x55be89ec in monitor_read /root/smartx/qemu-el7/qemu-test/monitor.c:4279 #14 0x563285cc in tcp_chr_read chardev/char-socket.c:470 #15 0x7670b968 in g_main_context_dispatch (/lib64/libglib-2.0.so.0+0x4a968) #16 0x5640727c in glib_pollfds_poll util/main-loop.c:215 #17 0x5640727c in os_host_main_loop_wait util/main-loop.c:238 #18 0x5640727c in main_loop_wait util/main-loop.c:497 #19 0x55b2d0bf in main_loop /root/smartx/qemu-el7/qemu-test/vl.c:2013 #20 0x55b2d0bf in main /root/smartx/qemu-el7/qemu-test/vl.c:4776 #21 0x7fffdd2eb444 in __libc_start_main (/lib64/libc.so.6+0x22444) #22 0x55b3767a (/root/smartx/qemu-el7/qemu-test/x86_64-softmmu/qemu-system-x86_64+0x5e367a) 0x604001020d18 is located 8 bytes inside of 40-byte region [0x604001020d10,0x604001020d38) freed by thread T0 here: #0 0x76f00508 in __interceptor_free (/lib64/libasan.so.4+0xde508) #1 0x7671107d in g_free (/lib64/libglib-2.0.so.0+0x5007d) previously allocated by thread T0 here: #0 0x76f00a88 in __interceptor_calloc (/lib64/libasan.so.4+0xdea88) #1 0x76710fc5 in g_malloc0 (/lib64/libglib-2.0.so.0+0x4ffc5) SUMMARY: AddressSanitizer: heap-use-after-free /root/smartx/qemu-el7/qemu-test/hw/virtio/vhost.c:1473 in vhost_dev_free_inflight Shadow bytes around the buggy address: 0x0c08801fc150: fa fa 00 00 00 00 04 fa fa fa fd fd fd fd fd fa 0x0c08801fc160: fa fa fd fd fd fd fd fd fa fa 00 00 00 00 04 fa 0x0c08801fc170: fa fa 00 00 00 00 00 01 fa fa 00 00 00 00 04 fa 0x0c08801fc180: fa fa 00 00 00 00 00 01 fa fa 00 00 00 00 00 01 0x0c08801fc190: fa fa 00 00 00 00 00 fa fa fa 00 00 00 00 04 fa =>0x0c08801fc1a0: fa fa fd[fd]fd fd fd fa fa fa fd fd fd fd fd fa 0x0c08801fc1b0: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fa 0x0c08801fc1c0: fa fa 00 00 00 00 00 fa fa fa fd fd fd fd fd fd 0x0c08801fc1d0: fa fa 00 00 00 00 00 01 fa fa fd fd fd fd fd fa 0x0c08801fc1e0: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fd 0x0c08801fc1f0: fa fa 00 00 00 00 00 01 fa fa fd fd fd fd fd fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user:f7 Container overflow: fc Array cookie:ac Intra object redzone:bb ASan internal: fe Left alloca redzone: ca Right alloca redzone:cb ==7309==ABORTING Signed-off-by: Li Feng --- hw/block/vhost-user-blk.c | 4 hw/virtio/vhost.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c index 776b9af3eb..19e79b96e4 100644 --- a/hw/block/vhost-user-blk.c +++ b/hw/block/vhost-user-blk.c @@ -463,7 +463,9 @@ reconnect: virtio_err: g_free(s->vhost_vqs); +s->vhost_vqs = NULL; g_free(s->inflight); +s->inflight = NULL; for (i = 0; i < s->num_queues; i++) { virtio_delete_queue(s->virtqs[i]); } @@ -484,7 +486,9 @@ static void vhost_user_blk_device_unrealize(DeviceState *dev, Error **errp) vhost_dev_cleanup(>dev); vhost_dev_free_inflight(s->inflight); g_free(s->vhost_vqs); +s->vhost_vqs = NULL; g_free(s->inflight); +s->inflight = NULL; for (i = 0; i < s->num_queues; i++) {