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

[Jens Axboe]

On 3/9/26 1:03 PM, Linus Torvalds wrote:
> On Mon, 9 Mar 2026 at 11:35, Jens Axboe <axboe@xxxxxxxxx> 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