From 81efe031412c98782d2c47e31b25a274257cf848 Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Mon, 7 Sep 2026 09:05:14 +0200 Subject: [PATCH 1/5] sched/wqueue: restore -ENOENT from work_cancel() for unqueued work work_cancel() used to return -ENOENT when the work structure was not in the queue, and callers depend on that: aio_cancel() tears down the AIO container (file_put() + aioc_free()) only when work_cancel() reports success, because a work item that is not queued may already be executing on a worker thread (see the comment in fs/aio/aio_cancel.c). Since commit 6f72f5481d ("sched/wqueue: Refactor delayed and periodical workqueue") work_cancel() returns OK unconditionally, and commit d2e01b90553 ("sched/wqueue: harden custom queue lifecycle") kept that behaviour and dropped -ENOENT from the function documentation. Under SMP the LTP aio_cancel tests then free the aio container and its file while the lpwork thread is still executing aio_write_worker() on it, which ends in a page fault in file_write() (f_inode == NULL) and a panic. Return -ENOENT again when the work is not queued, and document it. For the synchronous variant "not queued" alone does not tell whether the callback is running: the worker scan does, so report OK when a running callback was found and waited for, and -ENOENT only when the work was neither queued nor running. Assisted-by: Claude Code Signed-off-by: raiden00pl --- sched/wqueue/kwork_cancel.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/sched/wqueue/kwork_cancel.c b/sched/wqueue/kwork_cancel.c index 2329abaf70824..99c7774f48549 100644 --- a/sched/wqueue/kwork_cancel.c +++ b/sched/wqueue/kwork_cancel.c @@ -48,6 +48,7 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue, bool sync, { irqstate_t flags; pid_t self = sync ? nxsched_gettid() : INVALID_PROCESS_ID; + int ret = -ENOENT; if (wqueue == NULL || work == NULL) { @@ -83,8 +84,18 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue, bool sync, { work_timer_reset(wqueue); } + + ret = OK; } + /* Otherwise the work is not queued: either it was never queued or a + * worker has already dequeued it and may be executing its callback + * right now. Only the scan below can tell. Report -ENOENT if the + * work is neither queued nor running, so that callers (e.g. + * aio_cancel()) do not free resources that the callback is still + * using. + */ + if (sync) { for (wndx = 0; wndx < wqueue->nthreads; wndx++) @@ -93,6 +104,7 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue, bool sync, { worker[wndx].wait_count++; sync_wait = &worker[wndx].wait; + ret = OK; break; } } @@ -102,7 +114,7 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue, bool sync, if (sync_wait == NULL) { - return OK; + return ret; } nxsem_wait_uninterruptible(sync_wait); @@ -130,6 +142,8 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue, bool sync, * Zero on success, a negated errno on failure * * -EINVAL - An invalid work queue was specified + * -ENOENT - The work is not queued (and, for the sync variant, not + * running either) * ****************************************************************************/ From e919e4fa96d9679e4fb76ee87d6d32b614164c64 Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Tue, 8 Sep 2026 09:28:52 +0200 Subject: [PATCH 2/5] libs/libc/aio: fix nxstyle issues in lio_listio.c fix nxstyle issues in lio_listio.c Assisted-by: Claude Code Signed-off-by: raiden00pl --- libs/libc/aio/lio_listio.c | 113 +++++++++++++++++++------------------ 1 file changed, 58 insertions(+), 55 deletions(-) diff --git a/libs/libc/aio/lio_listio.c b/libs/libc/aio/lio_listio.c index 42b6f3cfa9c03..2bbc37f8c3d2a 100644 --- a/libs/libc/aio/lio_listio.c +++ b/libs/libc/aio/lio_listio.c @@ -233,6 +233,7 @@ static int lio_sigsetup(FAR struct aiocb * const *list, int nent, if (status != OK) { int errcode = get_errno(); + ferr("ERROR sigprocmask failed: %d\n", errcode); DEBUGASSERT(errcode > 0); return -errcode; @@ -547,59 +548,61 @@ int lio_listio(int mode, FAR struct aiocb * const list[], int nent, status = OK; switch (aiocbp->aio_lio_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(aiocbp); - } - else - { - /* Submit the asynchronous write operation */ - - status = aio_write(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; - } - 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; + 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(aiocbp); + } + else + { + /* Submit the asynchronous write operation */ + + status = aio_write(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; + } + else + { + /* Increment the count of successfully queued + * 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; } } } @@ -674,8 +677,8 @@ int lio_listio(int mode, FAR struct aiocb * const list[], int nent, * and this is the first error to be reported. */ - retcode = -status; - ret = ERROR; + retcode = -status; + ret = ERROR; } } } From f475936764fe557095b501c88a096c6cd0052622 Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Mon, 7 Sep 2026 14:31:16 +0200 Subject: [PATCH 3/5] libs/libc/aio: fix lio_listio() racing with early I/O completion lio_listio(LIO_NOWAIT) submits the requests first and only afterwards installs the SIGPOLL handler and attaches the per-request private data in lio_sigsetup(). Under SMP the requests complete on the low priority work queue while this is going on, and aio_signal() queues SIGPOLL for each of them as it finishes: - A signal delivered for a request that lio_sigsetup() found already finished (aio_priv left NULL) made lio_sighandler() dereference a NULL private pointer (LTP lio_listio 10-1, 15-1, 2-1 crash with a page fault in lio_sighandler on qemu-intel64 SMP). - A signal delivered between the aio_result check and the aio_priv store of the last outstanding request was consumed without private data, no later signal arrived, and the caller was never notified. - If every request had completed by the time the loop ran, no handler invocation could ever see the private data and the caller hung. - After the completing handler had notified the caller, signals still queued for other requests ran the handler again with their own copy of the private data. Block SIGPOLL while the handler is installed and the private data is attached, so that the completion signals are delivered only once every entry is set up. Ignore a signal for a request without private data, notify the caller directly from lio_sigsetup() when everything had already completed, and detach the private data of every entry once the list is done so that late signals find nothing to do. Assisted-by: Claude Code Signed-off-by: raiden00pl --- libs/libc/aio/lio_listio.c | 76 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 73 insertions(+), 3 deletions(-) diff --git a/libs/libc/aio/lio_listio.c b/libs/libc/aio/lio_listio.c index 2bbc37f8c3d2a..f4612aec819be 100644 --- a/libs/libc/aio/lio_listio.c +++ b/libs/libc/aio/lio_listio.c @@ -157,7 +157,18 @@ static void lio_sighandler(int signo, siginfo_t *info, void *ucontext) /* Recover our private data from the AIO control block */ sighand = (FAR struct lio_sighand_s *)aiocbp->aio_priv; - DEBUGASSERT(sighand && sighand->list); + if (sighand == NULL) + { + /* This I/O completed before lio_sigsetup() attached our private data + * to it (the completion raced with lio_listio() on another CPU). + * Nothing to do: an entry that still carries the private data will + * finish the job, or lio_sigsetup() already notified the caller. + */ + + return; + } + + DEBUGASSERT(sighand->list); aiocbp->aio_priv = NULL; /* Check if all of the pending I/O has completed */ @@ -165,6 +176,26 @@ static void lio_sighandler(int signo, siginfo_t *info, void *ucontext) ret = lio_checkio(sighand->list, sighand->nent); if (ret != -EINPROGRESS) { + FAR struct aiocb *other; + int i; + + /* Detach the private data of every other entry. A SIGPOLL that was + * dispatched for one of them before we got here still carries this + * handler and will be delivered later, possibly after the caller has + * been notified and its list is gone; with aio_priv cleared such a + * late invocation finds nothing to do. + */ + + for (i = 0; i < sighand->nent; i++) + { + other = sighand->list[i]; + if (other != NULL && other->aio_priv != NULL) + { + lib_free(other->aio_priv); + other->aio_priv = NULL; + } + } + /* All pending I/O has completed */ /* Restore the signal handler */ @@ -211,10 +242,12 @@ static int lio_sigsetup(FAR struct aiocb * const *list, int nent, FAR struct sigevent *sig) { FAR struct aiocb *aiocbp; + FAR struct aiocb *first = NULL; struct lio_sighand_s sighand; sigset_t set; struct sigaction act; int status; + int nprivs = 0; int i; /* Initialize the allocated structure */ @@ -225,11 +258,15 @@ static int lio_sigsetup(FAR struct aiocb * const *list, int nent, sighand.nent = nent; sighand.pid = _SCHED_GETPID(); - /* Make sure that SIGPOLL is not blocked */ + /* Block SIGPOLL until every entry carries its private data. The I/O is + * already in flight and may complete on another CPU at any time; a + * completion signal delivered before the loop below has attached the + * private data would be lost to lio_sighandler(). + */ sigemptyset(&set); sigaddset(&set, SIGPOLL); - status = sigprocmask(SIG_UNBLOCK, &set, &sighand.oprocmask); + status = sigprocmask(SIG_BLOCK, &set, &sighand.oprocmask); if (status != OK) { int errcode = get_errno(); @@ -271,6 +308,11 @@ static int lio_sigsetup(FAR struct aiocb * const *list, int nent, { FAR void *priv = NULL; + if (first == NULL) + { + first = aiocbp; + } + /* Check if I/O is pending for this entry */ if (aiocbp->aio_result == -EINPROGRESS) @@ -284,12 +326,40 @@ static int lio_sigsetup(FAR struct aiocb * const *list, int nent, memcpy(priv, (FAR void *)&sighand, sizeof(struct lio_sighand_s)); + nprivs++; } aiocbp->aio_priv = priv; } } + if (nprivs == 0 && first != NULL) + { + /* Every I/O completed while the signal handler was being set up, so + * no lio_sighandler() invocation will see our private data. Finish + * the job here: restore the signal state and notify the caller. + */ + + sigaction(SIGPOLL, &sighand.oact, NULL); + sigprocmask(SIG_SETMASK, &sighand.oprocmask, NULL); + return nxsig_notification(sighand.pid, &sighand.sig, SI_ASYNCIO, + &first->aio_sigwork); + } + + /* Let the completion signals through. Any that arrived meanwhile are + * delivered right here. + */ + + status = sigprocmask(SIG_UNBLOCK, &set, NULL); + if (status != OK) + { + int errcode = get_errno(); + + ferr("ERROR sigprocmask failed: %d\n", errcode); + DEBUGASSERT(errcode > 0); + return -errcode; + } + return OK; } From d3a6117be73369e9d3059b94db768241646f3b1f Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Mon, 7 Sep 2026 14:31:16 +0200 Subject: [PATCH 4/5] fs/aio: advance the pending list in aio_cancel() before work_cancel() The "cancel everything on this descriptor" loop in aio_cancel() only moved to the next container inside the branch taken when work_cancel() succeeded. When work_cancel() reports -ENOENT because a worker thread is already executing the request, the loop restarted its search from the same container and spun forever, hanging the task (and, since the scan runs with interrupts enabled but never yields, that CPU) without any output. This was hit by the LTP aio_cancel tests on SMP. Capture the next container before calling work_cancel() so the loop always makes progress. Assisted-by: Claude Code Signed-off-by: raiden00pl --- fs/aio/aio_cancel.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/fs/aio/aio_cancel.c b/fs/aio/aio_cancel.c index ceb31dd4a4106..d715033afae48 100644 --- a/fs/aio/aio_cancel.c +++ b/fs/aio/aio_cancel.c @@ -180,6 +180,13 @@ int aio_cancel(int fildes, FAR struct aiocb *aiocbp) if (aioc) { + /* Advance to the next container now: work_cancel() may report + * that this one is already executing (-ENOENT), and we must + * not examine the same container again in that case. + */ + + next = (FAR struct aio_container_s *)aioc->aioc_link.flink; + /* Yes... attempt to cancel the I/O. There are two * possibilities:* (1) the work has already been started and * is no longer queued, or (2) the work has not been started @@ -195,8 +202,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); From fd64cb80d682f8e9781114eacc92eaebf12c0432 Mon Sep 17 00:00:00 2001 From: raiden00pl Date: Tue, 8 Sep 2026 14:56:32 +0200 Subject: [PATCH 5/5] fs/aio: decant the container only after the I/O has been performed The aio worker functions started by decanting the container, which frees the container back to the pool and drops the file reference, and only then performed the I/O through the aiocb. Nothing was holding the file open while the read/write/fsync was in progress, and the request was no longer on the pending list, so a concurrent close() or aio_cancel() could pull the file from under the running operation. Keep the container until the operation has completed and decant it just before signalling completion. Assisted-by: Claude Code Signed-off-by: raiden00pl --- fs/aio/aio_fsync.c | 8 +++++++- fs/aio/aio_read.c | 8 +++++++- fs/aio/aio_write.c | 8 +++++++- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/fs/aio/aio_fsync.c b/fs/aio/aio_fsync.c index cc4ad40dbbbe7..5484dfa9ed5cf 100644 --- a/fs/aio/aio_fsync.c +++ b/fs/aio/aio_fsync.c @@ -79,7 +79,12 @@ 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 I/O while the container and its file reference are + * still valid. aioc_decant() drops the reference and frees the + * container, so it must be the last use of 'aioc'. + */ /* Perform the fsync using aioc_filep */ @@ -96,6 +101,7 @@ static void aio_fsync_worker(FAR void *arg) /* Signal the client */ + aioc_decant(aioc); aio_signal(pid, aiocbp); #ifdef CONFIG_PRIORITY_INHERITANCE diff --git a/fs/aio/aio_read.c b/fs/aio/aio_read.c index b3e366215e24b..8c2dca953af14 100644 --- a/fs/aio/aio_read.c +++ b/fs/aio/aio_read.c @@ -79,7 +79,12 @@ 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 I/O while the container and its file reference are + * still valid. aioc_decant() drops the reference and frees the + * container, so it must be the last use of 'aioc'. + */ /* Perform the file read using: * @@ -105,6 +110,7 @@ static void aio_read_worker(FAR void *arg) /* Signal the client */ + aioc_decant(aioc); aio_signal(pid, aiocbp); #ifdef CONFIG_PRIORITY_INHERITANCE diff --git a/fs/aio/aio_write.c b/fs/aio/aio_write.c index 0ba11dc357ea9..cefd3bc9abe2d 100644 --- a/fs/aio/aio_write.c +++ b/fs/aio/aio_write.c @@ -82,7 +82,12 @@ 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; + + /* Perform the I/O while the container and its file reference are + * still valid. aioc_decant() drops the reference and frees the + * container, so it must be the last use of 'aioc'. + */ /* Call fcntl(F_GETFL) to get the file open mode. */ @@ -133,6 +138,7 @@ static void aio_write_worker(FAR void *arg) /* Signal the client */ + aioc_decant(aioc); aio_signal(pid, aiocbp); #ifdef CONFIG_PRIORITY_INHERITANCE