Re: [PATCH v1] io_uring/register.c: fix NULL pointer dereference in io_register_resize_rings

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

On 3/9/26 18:34, Jens Axboe wrote:
> On 3/9/26 10:29 AM, Jens Axboe wrote:
>> On Mar 9, 2026, at 10:05?AM, Linus Torvalds <torvalds@linuxfoundation.org> wrote:
>>>
>>> ?On Mon, 9 Mar 2026 at 06:11, Jens Axboe <axboe@kernel.dk> wrote:
>>>>
>>>> You probably want something ala:
>>>>
>>>> mutex_lock(&ctx->mmap_lock);
>>>> spin_lock(&ctx->completion_lock();
>>>> + local_irq_disable();
>>>
>>> How could that ever work?
>>>
>>> Irqs will happily continue on other CPUs, so disabling interrupts is
>>> complete nonsense as far as I can tell - whether done with
>>> spin_lock_irq() *or* with local_irq_disable()/.
>>>
>>> So basically, touching ctx->rings from irq context in this section is
>>> simply not an option - or the rings pointer just needs to be updated
>>> atomically so that it doesn't matter.
>>>
>>> I assume this was tested in qemu on a single-core setup, so that
>>> fundamental mistake was hidden by an irrelevant configuration.
>>>
>>> Where is the actual oops - for some inexplicable reason that had been
>>> edited out, and it only had the call trace leading up toit? Based on
>>> the incomplete information and the faulting address of 0x24, I'm
>>> *guessing* that it is
>>>
>>>         if (ctx->flags & IORING_SETUP_TASKRUN_FLAG)
>>>                 atomic_or(IORING_SQ_TASKRUN, &ctx->rings->sq_flags);
>>>
>>> in io_req_normal_work_add(), but that may be complete garbage.
>>>
>>> So the actual fix may be to just make damn sure that
>>> IORING_SETUP_TASKRUN_FLAG is *not* set when the rings are resized.
>>>
>>> But for all I know, (a) I may be looking at entirely the wrong place
>>> and (b) there might be millions of other places that want to access
>>> ctx->rings, so the above may be the rantings of a crazy old man.
>>
>> Nah you?re totally right. I?m operating in few hours of sleep and on a
>> plane. I?ll take a closer look later. The flag mask protecting it is a
>> good idea, another one could be just a specific irq safe resize lock
>> would be better here.
> 
> How about something like this? I don't particularly like using ->flags
> for this, as these are otherwise static after the ring has been set up.
> Hence it'd be better to to just use a separate value for this,
> ->in_resize, and use smp_load_acquire/release. The write side can be as
> expensive as we want it to be, as it's not a hot path at all. And the
> acquire read should light weight enough here.
> 
> 
> diff --git a/include/linux/io_uring_types.h b/include/linux/io_uring_types.h
> index 3e4a82a6f817..428eb5b2c624 100644
> --- a/include/linux/io_uring_types.h
> +++ b/include/linux/io_uring_types.h
> @@ -394,6 +394,7 @@ struct io_ring_ctx {
>   		atomic_t		cq_wait_nr;
>   		atomic_t		cq_timeouts;
>   		struct wait_queue_head	cq_wait;
> +		int			in_resize;
>   	} ____cacheline_aligned_in_smp;
>   
>   	/* timeouts */
> diff --git a/io_uring/register.c b/io_uring/register.c
> index 3378014e51fb..048a1dcd9df1 100644
> --- a/io_uring/register.c
> +++ b/io_uring/register.c
> @@ -575,6 +575,7 @@ static int io_register_resize_rings(struct io_ring_ctx *ctx, void __user *arg)
>   	 * ctx->mmap_lock as well. Likewise, hold the completion lock over the
>   	 * duration of the actual swap.
>   	 */
> +	smp_store_release(&ctx->in_resize, 1);
>   	mutex_lock(&ctx->mmap_lock);
>   	spin_lock(&ctx->completion_lock);
>   	o.rings = ctx->rings;
> @@ -647,6 +648,7 @@ static int io_register_resize_rings(struct io_ring_ctx *ctx, void __user *arg)
>   	if (ctx->sq_data)
>   		io_sq_thread_unpark(ctx->sq_data);
>   
> +	smp_store_release(&ctx->in_resize, 0);
>   	return ret;
>   }
>   
> diff --git a/io_uring/tw.c b/io_uring/tw.c
> index 1ee2b8ab07c8..c66ffa787ec7 100644
> --- a/io_uring/tw.c
> +++ b/io_uring/tw.c
> @@ -152,6 +152,13 @@ void tctx_task_work(struct callback_head *cb)
>   	WARN_ON_ONCE(ret);
>   }
>   
> +static void io_mark_taskrun(struct io_ring_ctx *ctx)
> +{
> +	if (ctx->flags & IORING_SETUP_TASKRUN_FLAG &&
> +	    !smp_load_acquire(&ctx->in_resize))
> +		atomic_or(IORING_SQ_TASKRUN, &ctx->rings->sq_flags);
> +}

That's not going to work, same raciness, but you can protect the
pointer with rcu + rcu sync on resize. Tips:

1) sq_flags might get out of sync at the end. Either say that
users should never try to resize with inflight reqs, or just
hand set all flags, e.g. SQ_WAKE can be set unconditionally

2) For a fix, it'll likely be cleaner to keep ->rings as is
and introduce a second pointer (rcu protected).

-- 
Pavel Begunkov