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>
This commit is contained in:
Arnav Sharma 2026-09-16 23:11:59 +05:30 • committed by Alan C. Assis
parent 432717c34c
commit 493031e7b1

View file

@ -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;
}