[PATCH] watch_queue: Fix missing locking in watch_queue_set_size()

[PATCH] watch_queue: Fix missing locking in watch_queue_set_size()

[Cen Zhang (Microsoft)]

There is a race between watch_queue_set_size() and post_one_notification().
watch_queue_set_size() publishes wqueue->notes, notes_bitmap, nr_pages and
nr_notes with plain stores and does not take wqueue->lock, so the four
stores are not atomic as a group and, depending on the build, notes can be
stored last (gcc 14 on x86-64 does this).  Since keyctl_watch_key() lets a
watch attach to a queue before it is sized, post_one_notification() can
already be running on another CPU at that moment: it reads nr_notes > 0,
passes its bounds check, and then reads notes while it is still NULL,
dereferencing notes[0].

An unprivileged user triggers this reliably by watching one of its own
keys through a notification pipe and racing ioctl(IOC_WATCH_QUEUE_SET_SIZE)
on that pipe against keyctl(KEYCTL_SETPERM) on the key from another CPU,
triggering:

  BUG: kernel NULL pointer dereference, address: 0000000000000000
  RIP: 0010:post_one_notification.isra.0+0xa2/0x1e0
  Call Trace:
   __post_watch_notification+0x148/0x180
   keyctl_setperm_key+0x119/0x130
   do_syscall_64+0x108/0x4c0
   entry_SYSCALL_64_after_hwframe+0x77/0x7f
  Kernel panic - not syncing: Fatal exception in interrupt

Fix this by putting the four publication stores under wqueue->lock.
post_one_notification() already runs with that lock held, so a poster now
sees either the whole pool or none of it.

Fixes: c73be61cede5 ("pipe: Add general notification queue support")
Reported-by: Dae R. Jeong <threeearcat@gmail.com>
Closes: https://lore.kernel.org/all/ZT-S8Q7tyutcvu_q@dragonet/
Reported-by: AutonomousCodeSecurity@microsoft.com
Assisted-by: LLM
Signed-off-by: Cen Zhang (Microsoft) <cenzhang@linux.microsoft.com>
---
 kernel/watch_queue.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/watch_queue.c b/kernel/watch_queue.c
index 538520861e8b..9fb3035b3ebd 100644
--- a/kernel/watch_queue.c
+++ b/kernel/watch_queue.c
@@ -294,10 +294,13 @@ long watch_queue_set_size(struct pipe_inode_info *pipe, unsigned int nr_notes)
 		goto error_p;
 
 	bitmap_fill(bitmap, nr_notes);
+	/* Use lock since watches may already be attached. */
+	spin_lock_bh(&wqueue->lock);
 	wqueue->notes = pages;
 	wqueue->notes_bitmap = bitmap;
 	wqueue->nr_pages = nr_pages;
 	wqueue->nr_notes = nr_notes;
+	spin_unlock_bh(&wqueue->lock);
 	return 0;
 
 error_p:
-- 
2.55.0

Re: [PATCH] watch_queue: Fix missing locking in watch_queue_set_size()

[Cen Zhang (Microsoft)]

> +	/* Use lock since watches may already be attached. */
> +	spin_lock_bh(&wqueue->lock);
>  	wqueue->notes = pages;
>  	wqueue->notes_bitmap = bitmap;
>  	wqueue->nr_pages = nr_pages;
>  	wqueue->nr_notes = nr_notes;
> +	spin_unlock_bh(&wqueue->lock);

I'll send a v2 tomorrow, replacing the lock with smp_store_release()/
smp_load_acquire() for wqueue->nr_notes.

Re: [PATCH] watch_queue: Fix missing locking in watch_queue_set_size()

[Greg KH]

On Thu, Oct 08, 2026 at 06:42:33PM -0400, Cen Zhang (Microsoft) wrote:
> There is a race between watch_queue_set_size() and post_one_notification().
> watch_queue_set_size() publishes wqueue->notes, notes_bitmap, nr_pages and
> nr_notes with plain stores and does not take wqueue->lock, so the four
> stores are not atomic as a group and, depending on the build, notes can be
> stored last (gcc 14 on x86-64 does this).  Since keyctl_watch_key() lets a
> watch attach to a queue before it is sized, post_one_notification() can
> already be running on another CPU at that moment: it reads nr_notes > 0,
> passes its bounds check, and then reads notes while it is still NULL,
> dereferencing notes[0].
> 
> An unprivileged user triggers this reliably by watching one of its own
> keys through a notification pipe and racing ioctl(IOC_WATCH_QUEUE_SET_SIZE)
> on that pipe against keyctl(KEYCTL_SETPERM) on the key from another CPU,
> triggering:
> 
>   BUG: kernel NULL pointer dereference, address: 0000000000000000
>   RIP: 0010:post_one_notification.isra.0+0xa2/0x1e0
>   Call Trace:
>    __post_watch_notification+0x148/0x180
>    keyctl_setperm_key+0x119/0x130
>    do_syscall_64+0x108/0x4c0
>    entry_SYSCALL_64_after_hwframe+0x77/0x7f
>   Kernel panic - not syncing: Fatal exception in interrupt
> 
> Fix this by putting the four publication stores under wqueue->lock.
> post_one_notification() already runs with that lock held, so a poster now
> sees either the whole pool or none of it.
> 
> Fixes: c73be61cede5 ("pipe: Add general notification queue support")
> Reported-by: Dae R. Jeong <threeearcat@gmail.com>
> Closes: https://lore.kernel.org/all/ZT-S8Q7tyutcvu_q@dragonet/
> Reported-by: AutonomousCodeSecurity@microsoft.com
> Assisted-by: LLM
> Signed-off-by: Cen Zhang (Microsoft) <cenzhang@linux.microsoft.com>
> ---
>  kernel/watch_queue.c | 3 +++
>  1 file changed, 3 insertions(+)

No need to cc: security@k.o for public patches like this, thanks.

greg k-h