From dbf70f0a0388d0ba5bdf9c31ff3b81f77f4a90eb Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Mon, 3 Nov 2025 21:29:19 -0500 Subject: [PATCH] aio-posix: fix race between io_uring CQE and AioHandler deletion When an AioHandler is enqueued on ctx->submit_list for removal, the fill_sq_ring() function will submit an io_uring POLL_REMOVE operation to cancel the in-flight POLL_ADD operation. There is a race when another thread enqueues an AioHandler for deletion on ctx->submit_list when the POLL_ADD CQE has already appeared. In that case POLL_REMOVE is unnecessary. The code already handled this, but forgot that the AioHandler itself is still on ctx->submit_list when the POLL_ADD CQE is being processed. It's unsafe to delete the AioHandler at that point in time (use-after-free). Solve this problem by keeping the AioHandler alive but setting a flag so that it will be deleted by fill_sq_ring() when it runs. Signed-off-by: Stefan Hajnoczi Reviewed-by: Eric Blake Reviewed-by: Kevin Wolf Message-ID: <20251104022933.618123-2-stefanha@redhat.com> Signed-off-by: Kevin Wolf --- util/fdmon-io_uring.c | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/util/fdmon-io_uring.c b/util/fdmon-io_uring.c index b0d68bdc44..ad89160f31 100644 --- a/util/fdmon-io_uring.c +++ b/util/fdmon-io_uring.c @@ -52,9 +52,10 @@ enum { FDMON_IO_URING_ENTRIES = 128, /* sq/cq ring size */ /* AioHandler::flags */ - FDMON_IO_URING_PENDING = (1 << 0), - FDMON_IO_URING_ADD = (1 << 1), - FDMON_IO_URING_REMOVE = (1 << 2), + FDMON_IO_URING_PENDING = (1 << 0), + FDMON_IO_URING_ADD = (1 << 1), + FDMON_IO_URING_REMOVE = (1 << 2), + FDMON_IO_URING_DELETE_AIO_HANDLER = (1 << 3), }; static inline int poll_events_from_pfd(int pfd_events) @@ -218,6 +219,16 @@ static void fill_sq_ring(AioContext *ctx) if (flags & FDMON_IO_URING_REMOVE) { add_poll_remove_sqe(ctx, node); } + if (flags & FDMON_IO_URING_DELETE_AIO_HANDLER) { + /* + * process_cqe() sets this flag after ADD and REMOVE have been + * cleared. They cannot be set again, so they must be clear. + */ + assert(!(flags & FDMON_IO_URING_ADD)); + assert(!(flags & FDMON_IO_URING_REMOVE)); + + QLIST_INSERT_HEAD_RCU(&ctx->deleted_aio_handlers, node, node_deleted); + } } } @@ -241,7 +252,12 @@ static bool process_cqe(AioContext *ctx, */ flags = qatomic_fetch_and(&node->flags, ~FDMON_IO_URING_REMOVE); if (flags & FDMON_IO_URING_REMOVE) { - QLIST_INSERT_HEAD_RCU(&ctx->deleted_aio_handlers, node, node_deleted); + if (flags & FDMON_IO_URING_PENDING) { + /* Still on ctx->submit_list, defer deletion until fill_sq_ring() */ + qatomic_or(&node->flags, FDMON_IO_URING_DELETE_AIO_HANDLER); + } else { + QLIST_INSERT_HEAD_RCU(&ctx->deleted_aio_handlers, node, node_deleted); + } return false; } @@ -347,10 +363,13 @@ void fdmon_io_uring_destroy(AioContext *ctx) unsigned flags = qatomic_fetch_and(&node->flags, ~(FDMON_IO_URING_PENDING | FDMON_IO_URING_ADD | - FDMON_IO_URING_REMOVE)); + FDMON_IO_URING_REMOVE | + FDMON_IO_URING_DELETE_AIO_HANDLER)); - if (flags & FDMON_IO_URING_REMOVE) { - QLIST_INSERT_HEAD_RCU(&ctx->deleted_aio_handlers, node, node_deleted); + if ((flags & FDMON_IO_URING_REMOVE) || + (flags & FDMON_IO_URING_DELETE_AIO_HANDLER)) { + QLIST_INSERT_HEAD_RCU(&ctx->deleted_aio_handlers, + node, node_deleted); } QSLIST_REMOVE_HEAD_RCU(&ctx->submit_list, node_submitted); -- 2.47.3