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 74d2c2d119d1cb260a9a86341b4400552b9ee8e9
Author: Xiang Xiao <[email protected]>
AuthorDate: Fri Sep 11 02:23:29 2026 +0800

    fs/aio: use list_clear_node() to mark non-batch requests
    
    aio_fsync()/aio_read()/aio_write()/lio_listio() initialized
    aiocbp->lio_link with list_initialize(), which makes the node
    self-referential (prev = next = &node).  aio_signal() tests
    list_in_list(&lio_link) to detect lio_listio batches, so it wrongly
    entered the lio_listio completion path for every standalone AIO
    operation and notified through the uninitialized
    lio_sigevent/lio_sigwork.
    
    With CONFIG_SIG_EVTHREAD=y, garbage lio_sigevent.sigev_notify ==
    SIGEV_THREAD caused nxsig_notification() to queue &lio_sigwork.work
    onto the low-priority work queue with garbage func/value.  After the
    aiocb was freed, the dangling work_s was dispatched with worker=NULL,
    crashing in work_dispatch().
    
    Fix: initialize lio_link with list_clear_node() (prev = next = NULL)
    so list_in_list() returns false for non-lio_listio operations and
    aio_signal() skips the lio_listio path.
    
    While there, reject a NULL aiocbp in aio_fsync(): POSIX Issue 6 no
    longer defines a NULL special case, and the old DEBUGASSERT() panicked
    debug builds.
    
    Co-developed-by: dengwenqi <[email protected]>
    Co-developed-by: fangxinyong <[email protected]>
    Signed-off-by: fangxinyong <[email protected]>
    Signed-off-by: dengwenqi <[email protected]>
    Signed-off-by: Xiang Xiao <[email protected]>
---
 fs/aio/aio_fsync.c  |  17 +++++--
 fs/aio/aio_read.c   |   6 ++-
 fs/aio/aio_write.c  |   6 ++-
 fs/aio/lio_listio.c | 144 +++++++++++++++++++++++++++-------------------------
 4 files changed, 97 insertions(+), 76 deletions(-)

diff --git a/fs/aio/aio_fsync.c b/fs/aio/aio_fsync.c
index e2cfb692781..6fbc200786d 100644
--- a/fs/aio/aio_fsync.c
+++ b/fs/aio/aio_fsync.c
@@ -194,22 +194,29 @@ int aio_fsync(int op, FAR struct aiocb *aiocbp)
   FAR struct aio_container_s *aioc;
   int ret;
 
-  if (op != O_SYNC)
+  /* SUSv2 / POSIX Issue 5 specified that a NULL aiocbp produces no
+   * status through aiocbp and no completion signal. POSIX Issue 6 removed
+   * that special case, so reject NULL defensively.
+   */
+
+  if (op != O_SYNC || aiocbp == NULL)
     {
       set_errno(EINVAL);
       return ERROR;
     }
 
-  DEBUGASSERT(aiocbp);
-
   /* The result -EINPROGRESS means that the transfer has not yet completed */
 
   sigwork_init(&aiocbp->aio_sigwork);
   aiocbp->aio_result = -EINPROGRESS;
 
-  /* Initialize list_node using aiocbp for the first time */
+  /* Clear lio_link so list_in_list() returns false and aio_signal() skips
+   * the lio_listio path; list_initialize() would leave prev non-NULL, so
+   * list_in_list() wrongly returns true and aio_signal() notifies through
+   * the uninitialized lio_sigevent/lio_sigwork.
+   */
 
-  list_initialize(&aiocbp->lio_link);
+  list_clear_node(&aiocbp->lio_link);
 
   /* Create a container for the AIO control block.  This may cause us to
    * block if there are insufficient resources to satisfy the request.
diff --git a/fs/aio/aio_read.c b/fs/aio/aio_read.c
index dc092d42727..28b6f12f1cd 100644
--- a/fs/aio/aio_read.c
+++ b/fs/aio/aio_read.c
@@ -275,7 +275,11 @@ int aio_read(FAR struct aiocb *aiocbp)
       return ERROR;
     }
 
-  list_initialize(&aiocbp->lio_link);
+  /* Clear lio_link so aio_signal() skips the lio_listio path (see
+   * aio_fsync.c); list_initialize() would wrongly leave prev non-NULL.
+   */
+
+  list_clear_node(&aiocbp->lio_link);
   return aio_read_internal(aiocbp);
 }
 
diff --git a/fs/aio/aio_write.c b/fs/aio/aio_write.c
index f0fea80373d..81fcd37f8b2 100644
--- a/fs/aio/aio_write.c
+++ b/fs/aio/aio_write.c
@@ -311,7 +311,11 @@ int aio_write(FAR struct aiocb *aiocbp)
       return ERROR;
     }
 
-  list_initialize(&aiocbp->lio_link);
+  /* Clear lio_link so aio_signal() skips the lio_listio path (see
+   * aio_fsync.c); list_initialize() would wrongly leave prev non-NULL.
+   */
+
+  list_clear_node(&aiocbp->lio_link);
   return aio_write_internal(aiocbp);
 }
 
diff --git a/fs/aio/lio_listio.c b/fs/aio/lio_listio.c
index 59847ba9319..f64a8aa30c6 100644
--- a/fs/aio/lio_listio.c
+++ b/fs/aio/lio_listio.c
@@ -363,7 +363,11 @@ int lio_listio(int mode, FAR struct aiocb *restrict const 
list[restrict],
             }
           else
             {
-              list_initialize(&aiocbp->lio_link);
+              /* Not part of a lio_listio batch: clear lio_link so that
+               * aio_signal() skips the lio_listio completion path.
+               */
+
+              list_clear_node(&aiocbp->lio_link);
             }
         }
     }
@@ -377,78 +381,80 @@ int lio_listio(int mode, FAR struct aiocb *restrict const 
list[restrict],
       /* Skip over NULL entries */
 
       aiocbp = list[i];
-      if (aiocbp)
+      if (!aiocbp)
+        {
+          continue;
+        }
+
+      /* Submit the operation according to its opcode */
+
+      status = OK;
+      switch (aiocbp->aio_lio_opcode)
         {
-          /* Submit the operation according to its opcode */
+          case LIO_NOP:
+            {
+              /* Mark the do-nothing operation complete */
+
+              aiocbp->aio_result = OK;
+            }
+            break;
+
+          case LIO_READ:
+          case LIO_WRITE:
+            {
+              if (aiocbp->aio_lio_opcode == LIO_READ)
+                {
+                  /* Submit the asynchronous read operation */
+
+                  status = aio_read_internal(aiocbp);
+                }
+              else
+                {
+                  /* Submit the asynchronous write operation */
 
-          status = OK;
-          switch (aiocbp->aio_lio_opcode)
+                  status = aio_write_internal(aiocbp);
+                }
+
+              if (status < 0)
+                {
+                  /* Failed to queue the I/O.  Set up the error return. */
+
+                  errcode = get_errno();
+                  ferr("ERROR: aio_read/write failed: %d\n", errcode);
+                  DEBUGASSERT(errcode > 0);
+                  aiocbp->aio_result = -errcode;
+                  ret = ERROR;
+                }
+
+              if (status < 0 || aiocbp->aio_result == -EBADF ||
+                  aiocbp->aio_result == -EINVAL)
+                {
+                  if (mode == LIO_NOWAIT && sig)
+                    {
+                      aio_lock();
+                      list_delete(&aiocbp->lio_link);
+                      aio_unlock();
+                    }
+                }
+              else
+                {
+                  /* Increment the count of successfully queue operations */
+
+                  nqueued++;
+                }
+            }
+            break;
+
+          default:
             {
-            case LIO_NOP:
-              {
-                /* Mark the do-nothing operation complete */
-
-                aiocbp->aio_result = OK;
-              }
-              break;
-
-            case LIO_READ:
-            case LIO_WRITE:
-              {
-                if (aiocbp->aio_lio_opcode == LIO_READ)
-                  {
-                    /* Submit the asynchronous read operation */
-
-                    status = aio_read_internal(aiocbp);
-                  }
-                else
-                  {
-                    /* Submit the asynchronous write operation */
-
-                    status = aio_write_internal(aiocbp);
-                  }
-
-                if (status < 0)
-                  {
-                    /* Failed to queue the I/O.  Set up the error return. */
-
-                    errcode = get_errno();
-                    ferr("ERROR: aio_read/write failed: %d\n", errcode);
-                    DEBUGASSERT(errcode > 0);
-                    aiocbp->aio_result = -errcode;
-                    ret = ERROR;
-                  }
-
-                if (status < 0 || aiocbp->aio_result == -EBADF ||
-                    aiocbp->aio_result == -EINVAL)
-                  {
-                    if (mode == LIO_NOWAIT && sig)
-                      {
-                        aio_lock();
-                        list_delete(&aiocbp->lio_link);
-                        aio_unlock();
-                      }
-                  }
-                else
-                  {
-                    /* Increment the count of successfully queue operations */
-
-                    nqueued++;
-                  }
-              }
-              break;
-
-            default:
-              {
-                /* Make the invalid operation complete with an error */
-
-                ferr("ERROR: Unrecognized opcode: %d\n",
-                     aiocbp->aio_lio_opcode);
-                aiocbp->aio_result = -EINVAL;
-                ret = ERROR;
-              }
-              break;
+              /* Make the invalid operation complete with an error */
+
+              ferr("ERROR: Unrecognized opcode: %d\n",
+                   aiocbp->aio_lio_opcode);
+              aiocbp->aio_result = -EINVAL;
+              ret = ERROR;
             }
+            break;
         }
     }
 

Reply via email to