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

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

[Hao-Yu Yang]

During io_register_resize_rings execution, ctx->rings is temporarily
set to NULL before new ring memory is allocated. If a timer interrupt
fires during this window, the interrupt handler (via timerfd_tmrproc
-> io_poll_wake -> __io_req_task_work_add -> io_req_local_work_add)
attempts to access ctx->rings->sq_flags, causing race condition and
a NULL pointer dereference.

BUG: kernel NULL pointer dereference, address: 0000000000000024
PF: supervisor read access in kernel mode
PF: error_code(0x0000) - not-present page
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
Call Trace:
 <IRQ>
 __io_poll_execute (io_uring/poll.c:223)
 io_poll_wake (io_uring/poll.c:426)
 __wake_up_common (kernel/sched/wait.c:109)
 __wake_up_locked_key (kernel/sched/wait.c:167)
 timerfd_tmrproc (./include/linux/spinlock.h:407 fs/timerfd.c:71 fs/timerfd.c:78)
 ? __pfx_timerfd_tmrproc (fs/timerfd.c:75)
 __hrtimer_run_queues (kernel/time/hrtimer.c:1785 kernel/time/hrtimer.c:1849)
 hrtimer_interrupt (kernel/time/hrtimer.c:1914)
 __sysvec_apic_timer_interrupt (./arch/x86/include/asm/jump_label.h:37 ./arch/x86/include/asm/trace/irq_vectors.h:40 arch/x86/kernel/apic/apic.c:1063)
 sysvec_apic_timer_interrupt (arch/x86/kernel/apic/apic.c:1056 arch/x86/kernel/apic/apic.c:1056)
 </IRQ>
 <TASK>
 asm_sysvec_apic_timer_interrupt (./arch/x86/include/asm/idtentry.h:697)
 RIP: 0010:io_register_resize_rings (io_uring/register.c:593)
 ? io_register_resize_rings (io_uring/register.c:580)
 __io_uring_register (io_uring/register.c:898)
 ? fget (fs/file.c:1114)
 __x64_sys_io_uring_register (io_uring/register.c:1026 io_uring/register.c:1001 io_uring/register.c:1001)
 x64_sys_call (arch/x86/entry/syscall_64.c:41)
 do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall)
 entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
 </TASK>

Fix by using spin_lock_irq/spin_unlock_irq instead of spin_lock/spin_unlock
in io_register_resize_rings. This disables IRQs while ctx->rings is set to
NULL, preventing interrupt handlers from executing during the window when
ctx->rings is NULL.

Fixes: 79cfe9e59c2a ("io_uring/register: add IORING_REGISTER_RESIZE_RINGS")
Reported-by: Hao-Yu Yang <naup96721@gmail.com>
Signed-off-by: Hao-Yu Yang <naup96721@gmail.com>
---
 io_uring/register.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/io_uring/register.c b/io_uring/register.c
index 6015a3e9ce69..0526301f7a25 100644
--- a/io_uring/register.c
+++ b/io_uring/register.c
@@ -576,7 +576,7 @@ static int io_register_resize_rings(struct io_ring_ctx *ctx, void __user *arg)
 	 * duration of the actual swap.
 	 */
 	mutex_lock(&ctx->mmap_lock);
-	spin_lock(&ctx->completion_lock);
+	spin_lock_irq(&ctx->completion_lock);
 	o.rings = ctx->rings;
 	ctx->rings = NULL;
 	o.sq_sqes = ctx->sq_sqes;
@@ -640,7 +640,7 @@ static int io_register_resize_rings(struct io_ring_ctx *ctx, void __user *arg)
 	to_free = &o;
 	ret = 0;
 out:
-	spin_unlock(&ctx->completion_lock);
+	spin_unlock_irq(&ctx->completion_lock);
 	mutex_unlock(&ctx->mmap_lock);
 	io_register_free_rings(ctx, to_free);
 
-- 
2.34.1

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

[Jens Axboe]

On 3/9/26 12:27 AM, Hao-Yu Yang wrote:
> During io_register_resize_rings execution, ctx->rings is temporarily
> set to NULL before new ring memory is allocated. If a timer interrupt
> fires during this window, the interrupt handler (via timerfd_tmrproc
> -> io_poll_wake -> __io_req_task_work_add -> io_req_local_work_add)
> attempts to access ctx->rings->sq_flags, causing race condition and
> a NULL pointer dereference.
> 
> BUG: kernel NULL pointer dereference, address: 0000000000000024
> PF: supervisor read access in kernel mode
> PF: error_code(0x0000) - not-present page
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.15.0-1 04/01/2014
> Call Trace:
>  <IRQ>
>  __io_poll_execute (io_uring/poll.c:223)
>  io_poll_wake (io_uring/poll.c:426)
>  __wake_up_common (kernel/sched/wait.c:109)
>  __wake_up_locked_key (kernel/sched/wait.c:167)
>  timerfd_tmrproc (./include/linux/spinlock.h:407 fs/timerfd.c:71 fs/timerfd.c:78)
>  ? __pfx_timerfd_tmrproc (fs/timerfd.c:75)
>  __hrtimer_run_queues (kernel/time/hrtimer.c:1785 kernel/time/hrtimer.c:1849)
>  hrtimer_interrupt (kernel/time/hrtimer.c:1914)
>  __sysvec_apic_timer_interrupt (./arch/x86/include/asm/jump_label.h:37 ./arch/x86/include/asm/trace/irq_vectors.h:40 arch/x86/kernel/apic/apic.c:1063)
>  sysvec_apic_timer_interrupt (arch/x86/kernel/apic/apic.c:1056 arch/x86/kernel/apic/apic.c:1056)
>  </IRQ>
>  <TASK>
>  asm_sysvec_apic_timer_interrupt (./arch/x86/include/asm/idtentry.h:697)
>  RIP: 0010:io_register_resize_rings (io_uring/register.c:593)
>  ? io_register_resize_rings (io_uring/register.c:580)
>  __io_uring_register (io_uring/register.c:898)
>  ? fget (fs/file.c:1114)
>  __x64_sys_io_uring_register (io_uring/register.c:1026 io_uring/register.c:1001 io_uring/register.c:1001)
>  x64_sys_call (arch/x86/entry/syscall_64.c:41)
>  do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall)
>  entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
>  </TASK>
> 
> Fix by using spin_lock_irq/spin_unlock_irq instead of spin_lock/spin_unlock
> in io_register_resize_rings. This disables IRQs while ctx->rings is set to
> NULL, preventing interrupt handlers from executing during the window when
> ctx->rings is NULL.

I see the your crash, but this isn't the right fix - you can't just have
that one spot turn the completion lock into an IRQ disabling one,
that'll mess up the rest of the use cases in terms of lockdep. This lock
is never grabbed from IRQ context, hence it'd be misleading too.

You probably want something ala:

mutex_lock(&ctx->mmap_lock);
spin_lock(&ctx->completion_lock();
+ local_irq_disable();

...

+ local_irq_enable();
spin_unlock(&ctx->completion_lock);
mutex_unlock(&ctx->mmap_lock);

which I think should make lockdep happy with it too.

And then I think it wants a comment on top of that local_irq_disable(),
explaining how this is meant to prevent any irq/bh from triggering
task_work additions that touch ctx->rings to set the IORING_SQ_TASKRUN
flag.

Does that makes sense? If so, please do send a v2!

-- 
Jens Axboe