* [PATCH] vhost-user-blk: fix invalid memory access
@ 2020-04-17 10:17 Li Feng
2020-04-17 10:54 ` Philippe Mathieu-Daudé
2020-04-17 11:11 ` Michael S. Tsirkin
0 siblings, 2 replies; 4+ messages in thread
From: Li Feng @ 2020-04-17 10:17 UTC (permalink / raw)
To: Raphael Norwitz, Michael S. Tsirkin, Kevin Wolf, Max Reitz,
open list:Block layer core, open list:All patches CC here
Cc: lifeng1519, Li Feng, fanyang, kyle
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 <fengli@smartx.com>
---
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）工作邮箱. 如本邮箱发出的邮件与工作无关,该邮件未得到本公司任何的明示或默示的授权.
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] vhost-user-blk: fix invalid memory access
2020-04-17 10:17 [PATCH] vhost-user-blk: fix invalid memory access Li Feng
@ 2020-04-17 10:54 ` Philippe Mathieu-Daudé
2020-04-17 11:37 ` Li Feng
2020-04-17 11:11 ` Michael S. Tsirkin
1 sibling, 1 reply; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-17 10:54 UTC (permalink / raw)
To: Li Feng, Raphael Norwitz, Michael S. Tsirkin, Kevin Wolf,
Max Reitz, open list:Block layer core,
open list:All patches CC here
Cc: lifeng1519, fanyang, kyle
Hi Li,
On 4/17/20 12:17 PM, 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
Maybe cut <--
> #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
cut <--
> 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 <fengli@smartx.com>
What did you change since your previous version Raphael Norwitz already
reviewed? The patch looks the same...
https://www.mail-archive.com/qemu-devel@nongnu.org/msg696385.html
> ---
> 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;
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] vhost-user-blk: fix invalid memory access
2020-04-17 10:17 [PATCH] vhost-user-blk: fix invalid memory access Li Feng
2020-04-17 10:54 ` Philippe Mathieu-Daudé
@ 2020-04-17 11:11 ` Michael S. Tsirkin
1 sibling, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2020-04-17 11:11 UTC (permalink / raw)
To: Li Feng
Cc: Kevin Wolf, lifeng1519, fanyang, open list:Block layer core,
peter.maydell, open list:All patches CC here, Max Reitz, kyle,
Raphael Norwitz
On Fri, Apr 17, 2020 at 06:17:07PM +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 <fengli@smartx.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Pater do you want to pick this up now?
If not I'll pick it up beginning next week ...
> ---
> 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）工作邮箱. 如本邮箱发出的邮件与工作无关,该邮件未得到本公司任何的明示或默示的授权.
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] vhost-user-blk: fix invalid memory access
2020-04-17 10:54 ` Philippe Mathieu-Daudé
@ 2020-04-17 11:37 ` Li Feng
0 siblings, 0 replies; 4+ messages in thread
From: Li Feng @ 2020-04-17 11:37 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Kevin Wolf, lifeng1519, Yang Fan, open list:Block layer core,
Michael S. Tsirkin, open list:All patches CC here, Max Reitz,
Kyle Zhang, Raphael Norwitz
Nothing changed.
Just separate this patch from those patchset series.
Thanks,
Feng Li
Philippe Mathieu-Daudé <philmd@redhat.com> 于2020年4月17日周五 下午6:55写道：
>
> Hi Li,
>
> On 4/17/20 12:17 PM, 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
>
> Maybe cut <--
>
> > #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
>
> cut <--
>
> > 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 <fengli@smartx.com>
>
> What did you change since your previous version Raphael Norwitz already
> reviewed? The patch looks the same...
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg696385.html
>
> > ---
> > 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;
> >
>
--
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）工作邮箱. 如本邮箱发出的邮件与工作无关,该邮件未得到本公司任何的明示或默示的授权.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-04-17 11:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-04-17 10:17 [PATCH] vhost-user-blk: fix invalid memory access Li Feng
2020-04-17 10:54 ` Philippe Mathieu-Daudé
2020-04-17 11:37 ` Li Feng
2020-04-17 11:11 ` Michael S. Tsirkin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).