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

[Hao-Yu Yang <naup96721@gmail.com>]

On Mon, Mar 09, 2026 at 01:22:10PM -0600, Jens Axboe wrote:
> On 3/9/26 1:03 PM, Linus Torvalds wrote:
> > On Mon, 9 Mar 2026 at 11:35, Jens Axboe <axboe@kernel.dk> wrote:
> >>
> >> --- 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);
> > 
> > The store-release doesn't actually make sense here. It just says "this
> > store is visible after all previous stores".
> > 
> > It can still be delayed arbitraritly, and migrate down into the locked
> > regions, and be visible to other cpus much later.
> > 
> > On x86, getting a lock will be a full memory barrier, but that's not
> > true everywhere else: locks keep things *inside* the locked region
> > inside the lock, but don't stop things *outside* the locked region
> > from moving into it.
> > 
> > End result: the smp_store_release does nothing. You should use a write
> > barrier (or a smp_store_mb(), but that's expensive).
> > 
> > But even *that* won't work - because the irq can already be running on
> > another CPU, and maybe it already tested 'in_resize', and saw a zero,
> > and then did that
> > 
> >      atomic_or(IORING_SQ_TASKRUN, &ctx->rings->sq_flags);
> > 
> > afterwards.
> > 
> >> @@ -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);
> > 
> > On the release side, the store_release would make sense - the store is
> > visible to others after all the other stores are done (including,
> > obviously, the new 'rings' calue)
> > 
> > But see above. This just doesn't *work*, because the irq - running on
> > another cpu - will do the flag test and the cts->rings access as two
> > separate operations.
> > 
> > All these semantics means that 'in_resize' needs to basically be a lock.
> > 
> > You can then use 'trylock()' in irq context *around* the whole
> > sequence of using ctx->rings, to avoid disabling interrupts.
> 
> Agree - I think Pavel's suggestion to use an rcu protected pointer and
> have the resize sync rcu is probably better though. As mentioned, resize
> can be expensive, it's not a hot path operation. the local_work_add()
> path is extremely hot, however.
> 
> I'll take a look with fresh eyes tomorrow.
> 
> -- 
> Jens Axboe

Hello
Yes, crash point is at 

	if (!head) {
		if (ctx->flags & IORING_SETUP_TASKRUN_FLAG)
			atomic_or(IORING_SQ_TASKRUN, &ctx->rings->sq_flags);

When access &ctx->rings->sq_flags. I removed it accidentally, yesterday.