Re: [PATCH 1/2] io_uring: ensure ctx->rings is stable for task work flags manipulation

[Pavel Begunkov <asml.silence@gmail.com>]

On 3/10/26 14:45, Jens Axboe wrote:
> If DEFER_TASKRUN | SETUP_TASKRUN is used and task work is added while
> the ring is being resized, it's possible for the OR'ing of
> IORING_SQ_TASKRUN to happen in the small window of swapping into the
> new rings and the old rings being freed.
> 
> Prevent this by adding a 2nd ->rings pointer, ->rings_rcu, which is
> protected by RCU. The task work flags manipulation is inside RCU
> already, and if the resize ring freeing is done post an RCU synchronize,
> then there's no need to add locking to the fast path of task work
> additions.
> 
> Note: this is only done for DEFER_TASKRUN, as that's the only setup mode
> that supports ring resizing. If this ever changes, then they too need to
> use the io_ctx_mark_taskrun() helper.
> 
> Link: https://lore.kernel.org/io-uring/20260309062759.482210-1-naup96721@gmail.com/
> Cc: stable@vger.kernel.org
> Fixes: 79cfe9e59c2a ("io_uring/register: add IORING_REGISTER_RESIZE_RINGS")
> Reported-by: Hao-Yu Yang <naup96721@gmail.com>
> Suggested-by: Pavel Begunkov <asml.silence@gmail.com>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> ---
>   include/linux/io_uring_types.h |  1 +
>   io_uring/io_uring.c            |  2 ++
>   io_uring/register.c            | 20 ++++++++++++++++++--
>   io_uring/tw.c                  | 24 ++++++++++++++++++++++--
>   4 files changed, 43 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
> index 3e4a82a6f817..dd1420bfcb73 100644
> --- a/include/linux/io_uring_types.h
> +++ b/include/linux/io_uring_types.h
> @@ -388,6 +388,7 @@ struct io_ring_ctx {
>   	 * regularly bounce b/w CPUs.
>   	 */
>   	struct {
> +		struct io_rings	__rcu	*rings_rcu;
>   		struct llist_head	work_llist;
>   		struct llist_head	retry_llist;
>   		unsigned long		check_cq;
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index ccab8562d273..20fdc442e014 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -2066,6 +2066,7 @@ static void io_rings_free(struct io_ring_ctx *ctx)
>   	io_free_region(ctx->user, &ctx->sq_region);
>   	io_free_region(ctx->user, &ctx->ring_region);
>   	ctx->rings = NULL;
> +	RCU_INIT_POINTER(ctx->rings_rcu, NULL);
>   	ctx->sq_sqes = NULL;
>   }
>   
> @@ -2703,6 +2704,7 @@ static __cold int io_allocate_scq_urings(struct io_ring_ctx *ctx,
>   	if (ret)
>   		return ret;
>   	ctx->rings = rings = io_region_get_ptr(&ctx->ring_region);
> +	rcu_assign_pointer(ctx->rings_rcu, rings);
>   	if (!(ctx->flags & IORING_SETUP_NO_SQARRAY))
>   		ctx->sq_array = (u32 *)((char *)rings + rl->sq_array_offset);
>   
> diff --git a/io_uring/register.c b/io_uring/register.c
> index a839b22fd392..5f2985ba0879 100644
> --- a/io_uring/register.c
> +++ b/io_uring/register.c
> @@ -487,6 +487,18 @@ static void io_register_free_rings(struct io_ring_ctx *ctx,
>   			 IORING_SETUP_CQE32 | IORING_SETUP_NO_MMAP | \
>   			 IORING_SETUP_CQE_MIXED | IORING_SETUP_SQE_MIXED)
>   
> +static void io_resize_assign_rings(struct io_ring_ctx *ctx, struct io_rings *rings)
> +{
> +	/*
> +	 * Just mark any flag we may have missed and that the application
> +	 * should act on unconditionally. Worst case it'll be an extra
> +	 * syscall.
> +	 */
> +	atomic_or(IORING_SQ_TASKRUN | IORING_SQ_NEED_WAKEUP, &rings->sq_flags);
> +	ctx->rings = rings;
> +	rcu_assign_pointer(ctx->rings_rcu, rings);
> +}
> +
>   static int io_register_resize_rings(struct io_ring_ctx *ctx, void __user *arg)
>   {
>   	struct io_ctx_config config;
> @@ -579,6 +591,7 @@ static int io_register_resize_rings(struct io_ring_ctx *ctx, void __user *arg)
>   	spin_lock(&ctx->completion_lock);
>   	o.rings = ctx->rings;
>   	ctx->rings = NULL;
> +	RCU_INIT_POINTER(ctx->rings_rcu, NULL);
>   	o.sq_sqes = ctx->sq_sqes;
>   	ctx->sq_sqes = NULL;

Should be better to not have a transient null, and then there
is no need to check for that in task_work. I.e. don't zero it
and only assign the new value if you successfully created a
new set of rings.

-- 
Pavel Begunkov