[PATCH v2] watch_queue: Fix note pool publication race in watch_queue_set_size()

[PATCH v2] watch_queue: Fix note pool publication race 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 post_one_notification() reads them with
plain loads under a different lock, so nothing orders the four stores
against the poster's loads: 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 publishing nr_notes last with smp_store_release(), and by
reading it once with smp_load_acquire() in post_one_notification() before
the bitmap and the page array are touched.  A poster that observes a
non-zero nr_notes then also observes the pool behind it; one that observes
zero drops the notification as before.

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
Suggested-by: Dae R. Jeong <threeearcat@gmail.com>
Assisted-by: LLM
Signed-off-by: Cen Zhang (Microsoft) <cenzhang@linux.microsoft.com>
---
v2:
 - Use smp_store_release()/smp_load_acquire() instead of lock (Dae R. Jeong).
v1:
 - https://lore.kernel.org/all/20261008224233.9675-1-cenzhang@linux.microsoft.com/

 kernel/watch_queue.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/kernel/watch_queue.c b/kernel/watch_queue.c
index 538520861e8b..932a88ac1f39 100644
--- a/kernel/watch_queue.c
+++ b/kernel/watch_queue.c
@@ -101,7 +101,7 @@ static bool post_one_notification(struct watch_queue *wqueue,
 	struct pipe_inode_info *pipe = wqueue->pipe;
 	struct pipe_buffer *buf;
 	struct page *page;
-	unsigned int head, tail, note, offset, len;
+	unsigned int head, tail, note, offset, len, nr_notes;
 	bool done = false;
 
 	spin_lock_irq(&pipe->rd_wait.lock);
@@ -111,8 +111,10 @@ static bool post_one_notification(struct watch_queue *wqueue,
 	if (pipe_full(head, tail, pipe->ring_size))
 		goto lost;
 
-	note = find_first_bit(wqueue->notes_bitmap, wqueue->nr_notes);
-	if (note >= wqueue->nr_notes)
+	/* Pairs with the smp_store_release() in watch_queue_set_size(). */
+	nr_notes = smp_load_acquire(&wqueue->nr_notes);
+	note = find_first_bit(wqueue->notes_bitmap, nr_notes);
+	if (note >= nr_notes)
 		goto lost;
 
 	page = wqueue->notes[note / WATCH_QUEUE_NOTES_PER_PAGE];
@@ -297,7 +299,8 @@ long watch_queue_set_size(struct pipe_inode_info *pipe, unsigned int nr_notes)
 	wqueue->notes = pages;
 	wqueue->notes_bitmap = bitmap;
 	wqueue->nr_pages = nr_pages;
-	wqueue->nr_notes = nr_notes;
+	/* Pairs with the smp_load_acquire() in post_one_notification(). */
+	smp_store_release(&wqueue->nr_notes, nr_notes);
 	return 0;
 
 error_p:
-- 
2.55.0

Re: [PATCH v2] watch_queue: Fix note pool publication race in watch_queue_set_size()

[Bradley Morgan]

On 9 October 2026 23:47:15 BST, "Cen Zhang (Microsoft)"
<cenzhang@linux.microsoft.com> 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 post_one_notification() reads them with
>plain loads under a different lock, so nothing orders the four stores
>against the poster's loads: 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 publishing nr_notes last with smp_store_release(), and by
>reading it once with smp_load_acquire() in post_one_notification() before
>the bitmap and the page array are touched.  A poster that observes a
>non-zero nr_notes then also observes the pool behind it; one that observes
>zero drops the notification as before.

Gnarly looking bug.

>
>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
>Suggested-by: Dae R. Jeong <threeearcat@gmail.com>
>Assisted-by: LLM

Nit: Cc: All Applicable 
<stable@vger.kernel.org>

>Signed-off-by: Cen Zhang (Microsoft) <cenzhang@linux.microsoft.com>
>---
>v2:
> - Use smp_store_release()/smp_load_acquire() instead of lock (Dae R. Jeong).
>v1:
> - https://lore.kernel.org/all/20261008224233.9675-1-cenzhang@linux.microsoft.com/
>
> kernel/watch_queue.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
>diff --git a/kernel/watch_queue.c b/kernel/watch_queue.c
>index 538520861e8b..932a88ac1f39 100644
>--- a/kernel/watch_queue.c
>+++ b/kernel/watch_queue.c
>@@ -101,7 +101,7 @@ static bool post_one_notification(struct watch_queue *wqueue,
> 	struct pipe_inode_info *pipe = wqueue->pipe;
> 	struct pipe_buffer *buf;
> 	struct page *page;
>-	unsigned int head, tail, note, offset, len;
>+	unsigned int head, tail, note, offset, len, nr_notes;
> 	bool done = false;
> 
> 	spin_lock_irq(&pipe->rd_wait.lock);
>@@ -111,8 +111,10 @@ static bool post_one_notification(struct watch_queue *wqueue,
> 	if (pipe_full(head, tail, pipe->ring_size))
> 		goto lost;
> 
>-	note = find_first_bit(wqueue->notes_bitmap, wqueue->nr_notes);
>-	if (note >= wqueue->nr_notes)
>+	/* Pairs with the smp_store_release() in watch_queue_set_size(). */
>+	nr_notes = smp_load_acquire(&wqueue->nr_notes);
>+	note = find_first_bit(wqueue->notes_bitmap, nr_notes);
>+	if (note >= nr_notes)
> 		goto lost;
> 
> 	page = wqueue->notes[note / WATCH_QUEUE_NOTES_PER_PAGE];
>@@ -297,7 +299,8 @@ long watch_queue_set_size(struct pipe_inode_info *pipe, unsigned int nr_notes)
> 	wqueue->notes = pages;
> 	wqueue->notes_bitmap = bitmap;
> 	wqueue->nr_pages = nr_pages;
>-	wqueue->nr_notes = nr_notes;
>+	/* Pairs with the smp_load_acquire() in post_one_notification(). */
>+	smp_store_release(&wqueue->nr_notes, nr_notes);

Looks okay to me cen, just the nit:

Reviewed-by: Bradley Morgan <brads@mainlining.org>


> 	return 0;
> 
> error_p:
>

--- Thanks!
"I'm not a very positive person" - Linus torvalds

Re: [PATCH v2] watch_queue: Fix note pool publication race in watch_queue_set_size()

[Christian Brauner]

On Fri, 09 Oct 2026 18:47:15 -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 post_one_notification() reads them with
> plain loads under a different lock, so nothing orders the four stores
> against the poster's loads: 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].
> 
> [...]

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/1] watch_queue: Fix note pool publication race in watch_queue_set_size()
      https://git.kernel.org/vfs/vfs/c/e184fdfd7ea0