From 493031e7b145e629b20b3a485cdbb2d0b3e28ccf Mon Sep 17 00:00:00 2001 From: Arnav Sharma <2006arnavsharma@gmail.com> Date: Wed, 16 Sep 2026 23:11:59 +0530 Subject: [PATCH] drivers/mtd/mtd_config: fix UAF in mtdconfig_unregister_by_path mtdconfig_unregister_by_path() opened the device with file_open(), which runs mtdconfig_open() and therefore holds dev->lock for the whole lifetime of the temporary file reference. It then destroyed the mutex and freed the private device structure while that reference was still open, so the subsequent file_close() reached mtdconfig_close(), which performs nxmutex_unlock() on freed memory. Destroying a held mutex and unlocking it after free corrupt the heap; on sim this crashes deterministically in the next allocation (EXC_BAD_ACCESS in mm_malloc). Both file_close() and unregister_driver() return values were also discarded and the function unconditionally returned OK, masking legitimate errors. Reorder the teardown to close -> unregister -> destroy/free and propagate errors, so that: - file_close() (driver close callback and inode release) runs while the private device structure is still valid, releasing the exclusive access taken by mtdconfig_open(), - the private structure is destroyed and freed only after unregister_driver() succeeds. On failure the inode (and with it i_private) may still be referenced, so freeing would be wrong. Returning the error also honors the documented API contract (zero on success, negated errno on failure). This matches the established close -> unregister -> teardown ordering used by e.g. bchdev_unregister(). Verified with sim:configdata plus a register/unregister lifetime exercise in examples/configdata: 934706/934706 checks pass with the fix; with the fix stashed the same run dies with SIGSEGV right after mtdconfig_unregister_by_path() returns. Fixes: https://github.com/apache/nuttx/issues/20166 Signed-off-by: Arnav Sharma <2006arnavsharma@gmail.com> --- drivers/mtd/mtd_config.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/drivers/mtd/mtd_config.c b/drivers/mtd/mtd_config.c index 560d69eb69d..7590dac2039 100644 --- a/drivers/mtd/mtd_config.c +++ b/drivers/mtd/mtd_config.c @@ -1828,13 +1828,28 @@ int mtdconfig_unregister_by_path(FAR const char *path) inode = file.f_inode; dev = inode->i_private; + + /* Close the file before freeing the private device structure. */ + + ret = file_close(&file); + if (ret < 0) + { + ferr("ERROR: close %s failed: %d\n", path, ret); + return ret; + } + + /* Free the private device structure only after unregistering the driver. */ + + ret = unregister_driver(path); + if (ret < 0) + { + ferr("ERROR: unregister %s failed: %d\n", path, ret); + return ret; + } + nxmutex_destroy(&dev->lock); kmm_free(dev); - file_close(&file); - - unregister_driver(path); - return OK; }