nvme_cuse: Fix write-after-free when cuse thread early-exits
There is a bad memory corruption where the code for CUSE attempts to write one byte (with value 0x1) after the memory is freed. Context: When the CUSE device is unregistered, the poller thread is signaled with fuse_session_exit(), which writes the value 1 to fuse_session::exited. The poller thread then detects with fuse_session_exited() that it must exit the routing and finally destroys its own fuse session with cuse_lowlevel_teardown() before it exits. However, FUSE may also call fuse_session_exit() for its internal purposes. I'm not sure exactly under what conditions that happens, but I added some trace messages and I could clearly see that the CUSE thread exits before it was requested to exit in cuse_nvme_ns_stop(). If the poller thread early-exits, it would destroy its own FUSE session (and free the memory) before fuse_session_exit() gets executed, causing the memory to be corrupted with a single byte of value 0x1. Reproducer: The bug can be reproduced by resetting the FUSE session to NULL after it is destroyed. This will cuse_nvme_ns_stop() to crash with a segmentation fault in fuse_session_exit() because it tries to access a NULL pointer. static void * cuse_thread(void *arg) { [...] free(buf.mem); fuse_session_reset(cuse_device->session); + cuse_device->session = NULL; pthread_exit(NULL); } This fix: The fix I suggest is to destroy the FUSE session with cuse_lowlevel_teardown() after the thread is joined. Signed-off-by: Sylvain Didelot <sdidelot@ddn.com> Change-Id: I47202891a358f139506845110b012f840974b6fc Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/9931 Community-CI: Broadcom CI <spdk-ci.pdl@broadcom.com> Community-CI: Mellanox Build Bot Tested-by: SPDK CI Jenkins <sys_sgci@intel.com> Reviewed-by: Jim Harris <james.r.harris@intel.com> Reviewed-by: Tomasz Zawadzki <tomasz.zawadzki@intel.com>
This commit is contained in:
parent
f5f5ca1949
commit
a1014fccee
@ -790,7 +790,6 @@ cuse_thread(void *arg)
|
||||
}
|
||||
free(buf.mem);
|
||||
fuse_session_reset(cuse_device->session);
|
||||
cuse_lowlevel_teardown(cuse_device->session);
|
||||
pthread_exit(NULL);
|
||||
}
|
||||
|
||||
@ -851,6 +850,9 @@ cuse_nvme_ns_stop(struct cuse_device *ctrlr_device, struct cuse_device *ns_devic
|
||||
}
|
||||
pthread_join(ns_device->tid, NULL);
|
||||
TAILQ_REMOVE(&ctrlr_device->ns_devices, ns_device, tailq);
|
||||
if (ns_device->session != NULL) {
|
||||
cuse_lowlevel_teardown(ns_device->session);
|
||||
}
|
||||
free(ns_device);
|
||||
}
|
||||
|
||||
@ -935,6 +937,9 @@ cuse_nvme_ctrlr_stop(struct cuse_device *ctrlr_device)
|
||||
spdk_bit_array_free(&g_ctrlr_started);
|
||||
}
|
||||
nvme_cuse_unclaim(ctrlr_device);
|
||||
if (ctrlr_device->session != NULL) {
|
||||
cuse_lowlevel_teardown(ctrlr_device->session);
|
||||
}
|
||||
free(ctrlr_device);
|
||||
}
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user