From 245394ca0eac005313007ba1374cbdb50eb29d45 Mon Sep 17 00:00:00 2001 From: Changpeng Liu Date: Mon, 8 Nov 2021 17:44:00 +0800 Subject: [PATCH] vhost_scsi: fix a heap-use-after-free case `struct spdk_vhost_dev vdev` in `struct spdk_vhost_scsi_dev` can be unregistered in `vhost_scsi_dev_remove`, so we can't use it anymore in other places after `vhost_dev_unregister`. Ideally `state->remove_cb` should not take the `vdev` as the input parameter either, but I don't find it's used anywhere, so leave it unchanged. ==29555==ERROR: AddressSanitizer: heap-use-after-free on address 0x602000006df0 READ of size 2 at 0x602000006df0 thread T0 (reactor_0) #0 0x7f3c246c0f0a (/lib64/libasan.so.5+0x9cf0a) #1 0x7f3c246c3c15 in vsnprintf (/lib64/libasan.so.5+0x9fc15) #2 0xa55cfa in spdk_vlog /spdk/lib/log/log.c:158 #3 0xa5596f in spdk_log /spdk/lib/log/log.c:110 #4 0x842e43 in remove_scsi_tgt /spdk/lib/vhost/vhost_scsi.c:208 #5 0x851508 in vhost_scsi_dev_remove_tgt_cpl_cb /spdk/lib/vhost/vhost_scsi.c:1149 #6 0x8383f1 in foreach_session_finish_cb /spdk/lib/vhost/vhost.c:1144 #7 0x9d3223 in msg_queue_run_batch /spdk/lib/thread/thread.c:703 #8 0x9d73fe in thread_poll /spdk/lib/thread/thread.c:919 #9 0x9d7c3b in spdk_thread_poll /spdk/lib/thread/thread.c:979 #10 0x8812fe in _reactor_run /spdk/lib/event/reactor.c:920 #11 0x881bf1 in reactor_run /spdk/lib/event/reactor.c:958 #12 0x88292b in spdk_reactors_start /spdk/lib/event/reactor.c:1060 #13 0x873ff9 in spdk_app_start /spdk/lib/event/app.c:585 #14 0x408044 in main /spdk/app/vhost/vhost.c:105 #15 0x7f3c23691f42 in __libc_start_main (/lib64/libc.so.6+0x23f42) #16 0x407add in _start (/spdk/build/bin/vhost+0x407add) 0x602000006df0 is located 0 bytes inside of 8-byte region [0x602000006df0,0x602000006df8) freed by thread T0 (reactor_0) here: #0 0x7f3c2473191f in __interceptor_free (/lib64/libasan.so.5+0x10d91f) #1 0x8369f2 in vhost_dev_unregister /spdk/lib/vhost/vhost.c:1024 #2 0x84f32d in vhost_scsi_dev_remove /spdk/lib/vhost/vhost_scsi.c:913 #3 0x83cdb7 in spdk_vhost_dev_remove /spdk/lib/vhost/vhost.c:1494 #4 0x83ed66 in vhost_fini /spdk/lib/vhost/vhost.c:1644 #5 0x9d3223 in msg_queue_run_batch /spdk/lib/thread/thread.c:703 #6 0x9d73fe in thread_poll /spdk/lib/thread/thread.c:919 #7 0x9d7c3b in spdk_thread_poll /spdk/lib/thread/thread.c:979 #8 0x8812fe in _reactor_run /spdk/lib/event/reactor.c:920 #9 0x881bf1 in reactor_run /spdk/lib/event/reactor.c:958 #10 0x88292b in spdk_reactors_start /spdk/lib/event/reactor.c:1060 #11 0x873ff9 in spdk_app_start /spdk/lib/event/app.c:585 #12 0x408044 in main /spdk/app/vhost/vhost.c:105 #13 0x7f3c23691f42 in __libc_start_main (/lib64/libc.so.6+0x23f42) Change-Id: I511c4316a838cd92961d57c9193d384acd49d760 Signed-off-by: Changpeng Liu Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/10141 Community-CI: Broadcom CI Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins Reviewed-by: Shuhei Matsumoto Reviewed-by: Dong Yi Reviewed-by: Jim Harris Reviewed-by: Ben Walker --- lib/vhost/vhost_scsi.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/vhost/vhost_scsi.c b/lib/vhost/vhost_scsi.c index 3172265be96..40cb9ea4154 100644 --- a/lib/vhost/vhost_scsi.c +++ b/lib/vhost/vhost_scsi.c @@ -205,8 +205,7 @@ remove_scsi_tgt(struct spdk_vhost_scsi_dev *svdev, state->remove_cb(&svdev->vdev, state->remove_ctx); state->remove_cb = NULL; } - SPDK_INFOLOG(vhost, "%s: removed target 'Target %u'\n", - svdev->vdev.name, scsi_tgt_num); + SPDK_INFOLOG(vhost, "removed target 'Target %u'\n", scsi_tgt_num); if (--svdev->ref == 0 && svdev->registered == false) { free(svdev);