Re: [PATCH 2/4] vhost-user-blk: fix invalid memory access

[Raphael Norwitz]

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 0x555555ce948a bp 0x7fffffffb170 sp 0x7fffffffb160
> READ of size 8 at 0x604001020d18 thread T0
>     #0 0x555555ce9489 in vhost_dev_free_inflight 
> /root/smartx/qemu-el7/qemu-test/hw/virtio/vhost.c:1473
>     #1 0x555555cd86eb in virtio_reset 
> /root/smartx/qemu-el7/qemu-test/hw/virtio/virtio.c:1214
>     #2 0x5555560d3eff in virtio_pci_reset hw/virtio/virtio-pci.c:1859
>     #3 0x555555f2ac53 in device_set_realized hw/core/qdev.c:893
>     #4 0x5555561d572c in property_set_bool qom/object.c:1925
>     #5 0x5555561de8de in object_property_set_qobject qom/qom-qobject.c:27
>     #6 0x5555561d99f4 in object_property_set_bool qom/object.c:1188
>     #7 0x555555e50ae7 in qdev_device_add 
> /root/smartx/qemu-el7/qemu-test/qdev-monitor.c:626
>     #8 0x555555e51213 in qmp_device_add 
> /root/smartx/qemu-el7/qemu-test/qdev-monitor.c:806
>     #9 0x555555e8ff40 in hmp_device_add 
> /root/smartx/qemu-el7/qemu-test/hmp.c:1951
>     #10 0x555555be889a in handle_hmp_command 
> /root/smartx/qemu-el7/qemu-test/monitor.c:3404
>     #11 0x555555beac8b in monitor_command_cb 
> /root/smartx/qemu-el7/qemu-test/monitor.c:4296
>     #12 0x555556433eb7 in readline_handle_byte util/readline.c:393
>     #13 0x555555be89ec in monitor_read 
> /root/smartx/qemu-el7/qemu-test/monitor.c:4279
>     #14 0x5555563285cc in tcp_chr_read chardev/char-socket.c:470
>     #15 0x7ffff670b968 in g_main_context_dispatch 
> (/lib64/libglib-2.0.so.0+0x4a968)
>     #16 0x55555640727c in glib_pollfds_poll util/main-loop.c:215
>     #17 0x55555640727c in os_host_main_loop_wait util/main-loop.c:238
>     #18 0x55555640727c in main_loop_wait util/main-loop.c:497
>     #19 0x555555b2d0bf in main_loop /root/smartx/qemu-el7/qemu-test/vl.c:2013
>     #20 0x555555b2d0bf in main /root/smartx/qemu-el7/qemu-test/vl.c:4776
>     #21 0x7fffdd2eb444 in __libc_start_main (/lib64/libc.so.6+0x22444)
>     #22 0x555555b3767a  
> (/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 0x7ffff6f00508 in __interceptor_free (/lib64/libasan.so.4+0xde508)
>     #1 0x7ffff671107d in g_free (/lib64/libglib-2.0.so.0+0x5007d)
> 
> previously allocated by thread T0 here:
>     #0 0x7ffff6f00a88 in __interceptor_calloc (/lib64/libasan.so.4+0xdea88)
>     #1 0x7ffff6710fc5 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 <address@hidden>

Reviewed-by: Raphael Norwitz <address@hidden>

> ---
>  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(&s->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++) {
>          virtio_delete_queue(s->virtqs[i]);
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 01ebe12f28..aff98a0ede 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -1514,7 +1514,7 @@ void vhost_dev_set_config_notifier(struct vhost_dev 
> *hdev,
>  
>  void vhost_dev_free_inflight(struct vhost_inflight *inflight)
>  {
> -    if (inflight->addr) {
> +    if (inflight && inflight->addr) {
>          qemu_memfd_free(inflight->addr, inflight->size, inflight->fd);
>          inflight->addr = NULL;
>          inflight->fd = -1;
> -- 
> 2.11.0
> 
> 
> -- 
> The SmartX email address is only for business purpose. Any sent message 
> that is not related to the business is not authorized or permitted by 
> SmartX.
> 本邮箱为北京志凌海纳科技有限公司（SmartX）工作邮箱. 如本邮箱发出的邮件与工作无关,该邮件未得到本公司任何的明示或默示的授权.
> 
> 
>