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

commit b7d6c8ff41f563bd751dcd07da3b9a553285ee72
Author: Xiang Xiao <[email protected]>
AuthorDate: Fri Sep 11 02:21:55 2026 +0800

    fs/aio: fix aioc use-after-free and aio_cancel() issues
    
    aioc_decant() frees the AIO container and detaches the aiocbp.  The
    I/O workers (aio_read_worker, aio_write_worker, aio_fsync_worker)
    called it before signaling completion, so aio_signal() and any code
    touching the container afterwards ran on freed memory.  Additionally,
    if the caller closed the file early the detached container could be
    reused with a stale file reference.  Move aioc_decant() to after
    aio_signal() and use aioc->aioc_aiocbp directly in the workers.
    
    aio_cancel() also had two problems: with no aiocbp it looped over
    g_aio_pending with a do/while that skipped the list re-entry check, so
    a failed work_cancel() on an already running I/O caused an endless
    loop; and an invalid fildes only checked 'fildes < 0' instead of
    validating the descriptor, so a closed fd was not reported as EBADF.
    Use a for-loop that always advances and validate the descriptor with
    file_get()/file_put().
    
    Co-developed-by: wushenhui <[email protected]>
    Signed-off-by: wushenhui <[email protected]>
    Signed-off-by: tengshuangshuang <[email protected]>
---
 fs/aio/aio_cancel.c | 35 ++++++++++++++++++++---------------
 fs/aio/aio_fsync.c  |  3 ++-
 fs/aio/aio_read.c   |  3 ++-
 fs/aio/aio_write.c  |  3 ++-
 4 files changed, 26 insertions(+), 18 deletions(-)

diff --git a/fs/aio/aio_cancel.c b/fs/aio/aio_cancel.c
index ceb31dd4a41..ca26f5a1d66 100644
--- a/fs/aio/aio_cancel.c
+++ b/fs/aio/aio_cancel.c
@@ -32,6 +32,7 @@
 #include <errno.h>
 
 #include <nuttx/wqueue.h>
+#include <nuttx/fs/fs.h>
 
 #include "aio/aio.h"
 
@@ -83,18 +84,23 @@
 
 int aio_cancel(int fildes, FAR struct aiocb *aiocbp)
 {
-  if (fildes < 0)
-    {
-      set_errno(EBADF);
-      return ERROR;
-    }
-
   FAR struct aio_container_s *aioc;
   FAR struct aio_container_s *next;
+  FAR struct file            *filep;
+
   pid_t pid;
   int status;
   int ret;
 
+  ret = file_get(fildes, &filep);
+  if (ret < 0)
+    {
+      set_errno(-ret);
+      return ERROR;
+    }
+
+  file_put(filep);
+
   /* Check if a non-NULL aiocbp was provided */
 
   /* Lock the scheduler so that no I/O events can complete on the worker
@@ -165,14 +171,16 @@ int aio_cancel(int fildes, FAR struct aiocb *aiocbp)
     {
       /* No aiocbp.. cancel all outstanding I/O for the fildes */
 
-      next = (FAR struct aio_container_s *)g_aio_pending.head;
-      do
+      for (aioc = (FAR struct aio_container_s *)g_aio_pending.head;
+           aioc;
+           aioc = next)
         {
-          /* Find the next container with this AIO control block */
+          next = (FAR struct aio_container_s *)aioc->aioc_link.flink;
 
-          for (aioc = next;
-               aioc && aioc->aioc_aiocbp->aio_fildes != fildes;
-               aioc = (FAR struct aio_container_s *)aioc->aioc_link.flink);
+          if (aioc->aioc_aiocbp->aio_fildes != fildes)
+            {
+              continue;
+            }
 
           /* Did we find the container?  We should; the aio_result says
            * that the transfer is pending.  If not we return AIO_ALLDONE.
@@ -195,8 +203,6 @@ int aio_cancel(int fildes, FAR struct aiocb *aiocbp)
                    * transfers
                    */
 
-                  next   =
-                    (FAR struct aio_container_s *)aioc->aioc_link.flink;
                   pid    = aioc->aioc_pid;
                   aiocbp = aioc_decant(aioc);
                   DEBUGASSERT(aiocbp);
@@ -217,7 +223,6 @@ int aio_cancel(int fildes, FAR struct aiocb *aiocbp)
                 }
             }
         }
-      while (aioc);
     }
 
   aio_unlock();
diff --git a/fs/aio/aio_fsync.c b/fs/aio/aio_fsync.c
index f81a8a60a4c..9a4978dcbb8 100644
--- a/fs/aio/aio_fsync.c
+++ b/fs/aio/aio_fsync.c
@@ -79,7 +79,7 @@ static void aio_fsync_worker(FAR void *arg)
 #ifdef CONFIG_PRIORITY_INHERITANCE
   prio   = aioc->aioc_prio;
 #endif
-  aiocbp = aioc_decant(aioc);
+  aiocbp = aioc->aioc_aiocbp;
 
   /* Perform the fsync using aioc_filep */
 
@@ -97,6 +97,7 @@ static void aio_fsync_worker(FAR void *arg)
   /* Signal the client */
 
   aio_signal(pid, aiocbp);
+  aioc_decant(aioc);
 
 #ifdef CONFIG_PRIORITY_INHERITANCE
   /* Restore the low priority worker thread default priority */
diff --git a/fs/aio/aio_read.c b/fs/aio/aio_read.c
index 9cdb670d004..88d541556c7 100644
--- a/fs/aio/aio_read.c
+++ b/fs/aio/aio_read.c
@@ -79,7 +79,7 @@ static void aio_read_worker(FAR void *arg)
 #ifdef CONFIG_PRIORITY_INHERITANCE
   prio   = aioc->aioc_prio;
 #endif
-  aiocbp = aioc_decant(aioc);
+  aiocbp = aioc->aioc_aiocbp;
 
   /* Perform the file read using:
    *
@@ -106,6 +106,7 @@ static void aio_read_worker(FAR void *arg)
   /* Signal the client */
 
   aio_signal(pid, aiocbp);
+  aioc_decant(aioc);
 
 #ifdef CONFIG_PRIORITY_INHERITANCE
   /* Restore the low priority worker thread default priority */
diff --git a/fs/aio/aio_write.c b/fs/aio/aio_write.c
index c21c0ef361b..6c5b3f37dcf 100644
--- a/fs/aio/aio_write.c
+++ b/fs/aio/aio_write.c
@@ -82,7 +82,7 @@ static void aio_write_worker(FAR void *arg)
 #ifdef CONFIG_PRIORITY_INHERITANCE
   prio   = aioc->aioc_prio;
 #endif
-  aiocbp = aioc_decant(aioc);
+  aiocbp = aioc->aioc_aiocbp;
 
   /* Call fcntl(F_GETFL) to get the file open mode. */
 
@@ -134,6 +134,7 @@ errout:
   /* Signal the client */
 
   aio_signal(pid, aiocbp);
+  aioc_decant(aioc);
 
 #ifdef CONFIG_PRIORITY_INHERITANCE
   /* Restore the low priority worker thread default priority */

Reply via email to