Skip to content

fs/aio: rework lio_listio() and fix AIO crashes and POSIX conformance - #20107

Open
xiaoxiang781216 wants to merge 16 commits into
apache:masterfrom
xiaoxiang781216:upstream-aio
Open

fs/aio: rework lio_listio() and fix AIO crashes and POSIX conformance#20107
xiaoxiang781216 wants to merge 16 commits into
apache:masterfrom
xiaoxiang781216:upstream-aio

Conversation

@xiaoxiang781216

Copy link
Copy Markdown
Contributor

Summary

  • Rework lio_listio() to link all requests of a batch into a list before submitting any I/O; aio_signal() then removes each completed node under aio_lock() and notifies the caller only when the list becomes empty. This fixes the thread-unsafe "submit first, set up notification state later" ordering of the old implementation.
  • Fix a family of crashes around the new lio_link machinery: uninitialized/overwritten list nodes (use list_clear_node() for non-batch operations, call the new aio_read_internal()/aio_write_internal() from lio_listio() to preserve list membership), a NULL-aiocbp dereference when no I/O could be queued, and an invalid list_delete() for failed submissions in LIO_WAIT mode.
  • Fix aioc use-after-free: the I/O workers decanted (freed) the container before signaling completion; aioc_decant() now runs after aio_signal().
  • Fix aio_cancel(): endless loop when cancelling already-running I/O, and missing EBADF validation of the file descriptor (file_get()/file_put()).
  • Align aio_read()/aio_write()/aio_error() return values with POSIX: -1 + errno = EINVAL for rejected requests (also retrievable via aio_error()), but 0 with the error reported through aio_error() for a bad file descriptor.
  • aio_suspend() now re-checks the completion list after every wakeup so a SIGPOLL from unrelated AIO no longer causes a spurious return, and the timeout is honored across wakeups.
  • Reject a NULL aiocbp in aio_fsync() (POSIX Issue 6 removed the NULL special case).
  • Make the lio_listio() prototype match POSIX (restrict qualifiers, unnamed parameters).
  • Add a configurable CONFIG_FS_AIO_LISTIO_MAX (default 10), validate nent against {AIO_LISTIO_MAX} in lio_listio(), and report it via sysconf(_SC_AIO_LISTIO_MAX).
  • Move lio_listio.c from libs/libc/aio to fs/aio so the whole AIO implementation lives in one directory.

Impact

  • All changes are confined to fs/aio/, libs/libc/aio/, include/aio.h, include/limits.h, libs/libc/libc.csv and libs/libc/unistd/lib_sysconf.c; no new dependencies.
  • struct aiocb layout changes (the unused aio_priv field is replaced by lio_link/lio_sigevent/lio_sigwork) — ABI-affecting for out-of-tree users of include/aio.h, which is why this is submitted as one series.
  • lio_listio() prototype gains restrict qualifiers per POSIX; existing callers compile unchanged.

Testing

  • sim:nsh with CONFIG_FS_AIO=y, CONFIG_TESTING_OSTEST=y, CONFIG_TESTING_OSTEST_AIO=y: build is warning-free; the full ostest run exits with status 0 and the AIO test reports all 7 cases (poll, LIO_WAIT, aio_suspend, individual signals, list completion signal, cancel by aiocb, cancel by fd) successful:
user_main: AIO test
AIO test case 1: Poll for transfer complete
...
AIO test case 7:Cancel I/O by file descriptor
  aio_cancel return 1
  ...
aio_test: Test completed successfully
ostest_main: Exiting with status 0
  • tools/checkpatch.sh -c -u -m -g passes for the whole series.

wushenhui and others added 11 commits September 11, 2026 01:15
…listio()

Previously, lio_listio() would first call aio_read()/aio_write() to
submit I/O, and then set the handler and private variables, which led to
thread-unsafe behavior.

After this change, lio_listio() now links all I/O requests into a list.
When a task is completed, its corresponding node is removed from the
list in aio_signal(). The signal is triggered only when the list is
empty.

Signed-off-by: wushenhui <wushenhui@xiaomi.com>
by a I/O not referenced by the 'list'

Signed-off-by: wushenhui <wushenhui@xiaomi.com>
When aio_cancel() holds the lock, the I/O in the work_thread is blocked.
If the I/O that aio_cancel() attempts to cancel is already running,
work_cancel() will return an error, but aio_cancel() does not skip
this I/O, resulting in a endless-loop.

Signed-off-by: wushenhui <wushenhui@xiaomi.com>
Moving aioc_decant() after I/O finished, it can avoid use-after-free
issue, and also avoid read/write errors caused by the caller prematurely
closing the file.

Signed-off-by: wushenhui <wushenhui@xiaomi.com>
In the POSIX standard, aio_cancel should return -1 and
set errno to EBADF when it receives an invalid fd.

Signed-off-by: tengshuangshuang <tengshuangshuang@xiaomi.com>
avoid wild pointer for the first time

Signed-off-by: tengshuangshuang <tengshuangshuang@xiaomi.com>
aiocbp may be NULL when passed to nxsig_notification

Signed-off-by: zhengyu16 <zhengyu16@xiaomi.com>
A call to aio_error()may return -1 and set errno to EINVAL if
aiocbp does not refer to an operation whose return status has not
yet been retrieved. A call to aio_read() and aio_write() may return -1 and set errno to EINVAL if the criteria is not met. And in aio_read() and aio_write(), aio_result should be set so that aio_error() function can query correctly.

Signed-off-by: tengshuangshuang <tengshuangshuang@xiaomi.com>
…ash.

so let's skip delete it

Signed-off-by: tengshuangshuang <tengshuangshuang@xiaomi.com>
In the case test of ltp, nent is set to 10, so the value of AIO_LISTIO_MAX in limit.h is modified to 11 in order to pass the ltp test.

Signed-off-by: tengshuangshuang <tengshuangshuang@xiaomi.com>
aio_read and aio_write function miss check for fd.
ltp requires a return value of 0 when fd is invalid.

Signed-off-by: tengshuangshuang <tengshuangshuang@xiaomi.com>
@github-actions github-actions Bot added Area: File System File System issues Size: L The size of the change in this PR is large labels Sep 10, 2026
extinguish and others added 4 commits September 11, 2026 02:48
…andard

by remove the parameter name from the prototype

Signed-off-by: guoshichao <guoshichao@xiaomi.com>
Extract aio_read_internal/aio_write_internal that skip lio_link
initialization. aio_read/aio_write initialize lio_link before
calling the internal function, while lio_listio calls the internal
function directly to preserve its own lio_link setup.

This fixes a crash when aio_signal accesses uninitialized lio_link
from aiocb filled with 0xff by the ostest.

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
aio_fsync() dereferences aiocbp after a DEBUGASSERT(). Passing NULL can
therefore panic debug builds and crash release builds.

POSIX Issue 6 and later no longer define a NULL aiocbp special case for
aio_fsync(). Treat NULL as an invalid argument and return ERROR with
errno set to EINVAL, matching the defensive argument checks used by
aio_read() and aio_write().

Signed-off-by: fangxinyong <fangxinyong@xiaomi.com>
Root cause: 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) (prev != NULL) 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() (prefetch abort at PC=0, lpwork task).

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.

Signed-off-by: dengwenqi <dengwenqi@xiaomi.com>
@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

arduino-mega2560

  • flash: .text +22 B (+0.0%, 65,112 B / 262,144 B, total: 25% used)

esp32-devkitc

  • ROM: .flash.text +4 B (+0.0%, 124,892 B / 4,194,272 B, total: 3% used)
  • irom0_0_seg: .flash.text +4 B (+0.0%, 89,096 B / 3,342,304 B, total: 3% used)

mirtoo

  • kseg0_progmem: .text +16 B (+0.0%, 67,752 B / 131,072 B, total: 52% used)

qemu-armv8a

  • Code: .text.nxsched_set_scheduler +12 B, .text.waitid -40 B (+0.0%, 336,908 B)

qemu-intel64

  • Code: .text +33 B (+0.0%, 8,659,798 B)

rx65n-rsk2mb

  • ROM: .text +16 B (+0.0%, 86,880 B / 2,097,152 B, total: 4% used)

s698pm-dkit

  • Code: .text +32 B (+0.0%, 367,488 B)

stm32-nucleo-f103rb

  • flash: .text +12 B (+0.0%, 34,548 B / 131,072 B, total: 26% used)
    No memory changes detected for:
  • hifive1-revb

Fix the misindented 'if (mode == LIO_NOWAIT && sig)' block (one space
short) and hoist the opcode switch out of the 'if (aiocbp)' block via
an early continue, so that the nesting and case label indentation pass
nxstyle/checkpatch.  No functional change.

Signed-off-by: Xiang Xiao <xiaoxiang@xiaomi.com>
Comment thread fs/aio/aio_cancel.c
@raiden00pl

Copy link
Copy Markdown
Member

intel64 LTP pass with this PR and #20112

@raiden00pl

raiden00pl commented Sep 11, 2026

Copy link
Copy Markdown
Member

@xiaoxiang781216 ltp_interfaces_lio_listio_2_1 test case for rv-virt/citest failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: File System File System issues Size: L The size of the change in this PR is large

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants