Skip to content

Commit 6f0ff37

Browse files
author
Changpeng Liu
committed
nvmf/vfio-user: fix an heap-use-after-free issue
The controller data structure may be freed before subsystem resume done callback, we can take endpoint as the input parameter to avoid this issue. AddressSanitizer: heap-use-after-free on address 0x625000046100 at pc 0x00000082818f bp 0x7fff7b09bd10 sp 0x7fff7b09bd00 READ of size 8 at 0x625000046100 thread T0 (reactor_0) #0 0x82818e in vfio_user_dev_quiesce_resume_done /spdk/lib/nvmf/vfio_user.c:2147 #1 0x782cc0 in subsystem_state_change_done /spdk/lib/nvmf/subsystem.c:634 #2 0xad047b in _call_completion /spdk/lib/thread/thread.c:2344 #3 0xabc48d in msg_queue_run_batch /spdk/lib/thread/thread.c:710 spdk#4 0xac0670 in thread_poll /spdk/lib/thread/thread.c:926 spdk#5 0xac0ead in spdk_thread_poll /spdk/lib/thread/thread.c:986 spdk#6 0x9a5b4f in _reactor_run /spdk/lib/event/reactor.c:920 spdk#7 0x9a6442 in reactor_run /spdk/lib/event/reactor.c:958 spdk#8 0x9a717c in spdk_reactors_start /spdk/lib/event/reactor.c:1060 spdk#9 0x99884a in spdk_app_start /spdk/lib/event/app.c:643 spdk#10 0x407e82 in main /spdk/app/nvmf_tgt/nvmf_main.c:75 spdk#11 0x7f822095ff42 in __libc_start_main (/lib64/libc.so.6+0x23f42) spdk#12 0x407abd in _start (/spdk/build/bin/nvmf_tgt+0x407abd) 0x625000046100 is located 0 bytes inside of 8320-byte region [0x625000046100,0x625000048180) freed by thread T0 (reactor_0) here: #0 0x7f82219ff91f in __interceptor_free (/lib64/libasan.so.5+0x10d91f) #1 0x837059 in _free_ctrlr /spdk/lib/nvmf/vfio_user.c:2976 #2 0x837327 in free_ctrlr /spdk/lib/nvmf/vfio_user.c:2996 #3 0x843541 in nvmf_vfio_user_close_qpair /spdk/lib/nvmf/vfio_user.c:3742 spdk#4 0x7d1d91 in nvmf_transport_qpair_fini /spdk/lib/nvmf/transport.c:604 spdk#5 0x7ad922 in _nvmf_qpair_destroy /spdk/lib/nvmf/nvmf.c:1055 spdk#6 0x761362 in nvmf_qpair_request_cleanup /spdk/lib/nvmf/ctrlr.c:4026 spdk#7 0x761906 in spdk_nvmf_request_free /spdk/lib/nvmf/ctrlr.c:4041 spdk#8 0x75a931 in nvmf_qpair_free_aer /spdk/lib/nvmf/ctrlr.c:3576 spdk#9 0x7ae626 in spdk_nvmf_qpair_disconnect /spdk/lib/nvmf/nvmf.c:1127 spdk#10 0x83db36 in _vfio_user_qpair_disconnect /spdk/lib/nvmf/vfio_user.c:3433 spdk#11 0xabc48d in msg_queue_run_batch /spdk/lib/thread/thread.c:710 spdk#12 0xac0670 in thread_poll /spdk/lib/thread/thread.c:926 spdk#13 0xac0ead in spdk_thread_poll /spdk/lib/thread/thread.c:986 spdk#14 0x9a5b4f in _reactor_run /spdk/lib/event/reactor.c:920 spdk#15 0x9a6442 in reactor_run /spdk/lib/event/reactor.c:958 spdk#16 0x9a717c in spdk_reactors_start /spdk/lib/event/reactor.c:1060 spdk#17 0x99884a in spdk_app_start /spdk/lib/event/app.c:643 spdk#18 0x407e82 in main /spdk/app/nvmf_tgt/nvmf_main.c:75 spdk#19 0x7f822095ff42 in __libc_start_main (/lib64/libc.so.6+0x23f42) previously allocated by thread T0 (reactor_0) here: #0 0x7f82219fff16 in __interceptor_calloc (/lib64/libasan.so.5+0x10df16) #1 0x837413 in nvmf_vfio_user_create_ctrlr /spdk/lib/nvmf/vfio_user.c:3010 #2 0x83bc68 in nvmf_vfio_user_accept /spdk/lib/nvmf/vfio_user.c:3313 #3 0xabfbd8 in thread_execute_timed_poller /spdk/lib/thread/thread.c:872 spdk#4 0xac0c75 in thread_poll /spdk/lib/thread/thread.c:960 spdk#5 0xac0ead in spdk_thread_poll /spdk/lib/thread/thread.c:986 spdk#6 0x9a5b4f in _reactor_run /spdk/lib/event/reactor.c:920 spdk#7 0x9a6442 in reactor_run /spdk/lib/event/reactor.c:958 spdk#8 0x9a717c in spdk_reactors_start /spdk/lib/event/reactor.c:1060 spdk#9 0x99884a in spdk_app_start /spdk/lib/event/app.c:643 spdk#10 0x407e82 in main /spdk/app/nvmf_tgt/nvmf_main.c:75 spdk#11 0x7f822095ff42 in __libc_start_main (/lib64/libc.so.6+0x23f42) SUMMARY: AddressSanitizer: heap-use-after-free /spdk/lib/nvmf/vfio_user.c:2147 in vfio_user_dev_quiesce_resume_done Change-Id: Icf5e5b360b9107a3c5eb960ae59b7fe10ace1c66 Signed-off-by: Changpeng Liu <changpeng.liu@intel.com> Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/11420 Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com> Tested-by: SPDK CI Jenkins <sys_sgci@intel.com> Reviewed-by: Dong Yi <dongx.yi@intel.com> Reviewed-by: John Levon <levon@movementarian.org> Reviewed-by: Ben Walker <benjamin.walker@intel.com> Reviewed-by: Jim Harris <james.r.harris@intel.com>
1 parent 3e937f0 commit 6f0ff37

File tree

1 file changed

+9
-6
lines changed

1 file changed

+9
-6
lines changed

‎lib/nvmf/vfio_user.c

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2205,15 +2205,18 @@ vfio_user_dev_quiesce_done(struct spdk_nvmf_subsystem *subsystem,
22052205
void *cb_arg, int status);
22062206

22072207
static void
2208-
vfio_user_dev_quiesce_resume_done(struct spdk_nvmf_subsystem *subsystem,
2209-
void *cb_arg, int status)
2208+
vfio_user_endpoint_resume_done(struct spdk_nvmf_subsystem *subsystem,
2209+
void *cb_arg, int status)
22102210
{
2211-
struct nvmf_vfio_user_ctrlr *vu_ctrlr = cb_arg;
2212-
struct nvmf_vfio_user_endpoint *endpoint = vu_ctrlr->endpoint;
2211+
struct nvmf_vfio_user_endpoint *endpoint = cb_arg;
2212+
struct nvmf_vfio_user_ctrlr *vu_ctrlr = endpoint->ctrlr;
22132213
int ret;
22142214

2215-
SPDK_DEBUGLOG(nvmf_vfio, "%s resumed done with status %d\n", ctrlr_id(vu_ctrlr), status);
2215+
SPDK_DEBUGLOG(nvmf_vfio, "%s resumed done with status %d\n", endpoint_id(endpoint), status);
22162216

2217+
if (!vu_ctrlr) {
2218+
return;
2219+
}
22172220
vu_ctrlr->state = VFIO_USER_CTRLR_RUNNING;
22182221

22192222
/* Basically, once we call `vfu_device_quiesced` the device is unquiesced from
@@ -2261,7 +2264,7 @@ vfio_user_dev_quiesce_done(struct spdk_nvmf_subsystem *subsystem,
22612264
SPDK_DEBUGLOG(nvmf_vfio, "%s start to resume\n", ctrlr_id(vu_ctrlr));
22622265
vu_ctrlr->state = VFIO_USER_CTRLR_RESUMING;
22632266
ret = spdk_nvmf_subsystem_resume((struct spdk_nvmf_subsystem *)endpoint->subsystem,
2264-
vfio_user_dev_quiesce_resume_done, vu_ctrlr);
2267+
vfio_user_endpoint_resume_done, endpoint);
22652268
if (ret < 0) {
22662269
vu_ctrlr->state = VFIO_USER_CTRLR_PAUSED;
22672270
SPDK_ERRLOG("%s: failed to resume, ret=%d\n", endpoint_id(endpoint), ret);

0 commit comments

Comments
 (0)