(nuttx) branch master updated: drivers/mtd/mtd_config: fix UAF in mtdconfig_unregister_by_path

[acassis]

This is an automated email from the ASF dual-hosted git repository.

acassis pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git


The following commit(s) were added to refs/heads/master by this push:
     new 493031e7b14 drivers/mtd/mtd_config: fix UAF in 
mtdconfig_unregister_by_path
493031e7b14 is described below

commit 493031e7b145e629b20b3a485cdbb2d0b3e28ccf
Author: Arnav Sharma <2006arnavsha...@gmail.com>
AuthorDate: Wed Sep 16 23:11:59 2026 +0530

    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 <2006arnavsha...@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,12 +1828,27 @@ int mtdconfig_unregister_by_path(FAR const char *path)
 
   inode = file.f_inode;
   dev = inode->i_private;
-  nxmutex_destroy(&dev->lock);
-  kmm_free(dev);
 
-  file_close(&file);
+  /* 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. */
 
-  unregister_driver(path);
+  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);
 
   return OK;
 }