[PATCH] uprobes: Remove redundant guard(rcu) in find_active_uprobe_speculative()

[PATCH] uprobes: Remove redundant guard(rcu) in find_active_uprobe_speculative()

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

find_active_uprobe_speculative() is called from find_active_uprobe_rcu(),
which is invoked by handle_swbp() and handle_syscall_uprobe().
Both callers already hold RCU Tasks Trace (via rcu_read_lock_trace() or
guard(rcu_tasks_trace)()).

Calling guard(rcu)() inside find_active_uprobe_speculative() is redundant
because the caller already guarantees RCU Tasks Trace protection.
Furthermore, standard RCU (rcu_read_lock()) does not protect uprobes_tree
against asynchronous uprobe destruction (call_rcu_tasks_trace()).

Remove the redundant guard(rcu)() from find_active_uprobe_speculative() and
replace it with lockdep_assert(rcu_read_lock_trace_held()) and a comment
documenting that callers must hold RCU Tasks Trace protection, matching the
convention used by find_active_uprobe_rcu() and find_uprobe_rcu().

Fixes: e0925f2dc4de ("uprobes: add speculative lockless VMA-to-inode-to-uprobe resolution")
Assisted-by: Antigravity:gemini-3.6-flash
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 kernel/events/uprobes.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 4084e926e284..b4e253d43b30 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -2424,6 +2424,7 @@ static int is_trap_at_addr(struct mm_struct *mm, unsigned long vaddr)
 	return is_trap_insn(&opcode);
 }
 
+/* assumes being inside RCU Tasks Trace protected region */
 static struct uprobe *find_active_uprobe_speculative(unsigned long bp_vaddr)
 {
 	struct mm_struct *mm = current->mm;
@@ -2433,7 +2434,7 @@ static struct uprobe *find_active_uprobe_speculative(unsigned long bp_vaddr)
 	loff_t offset;
 	unsigned int seq;
 
-	guard(rcu)();
+	lockdep_assert(rcu_read_lock_trace_held());
 
 	if (!mmap_lock_speculate_try_begin(mm, &seq))
 		return NULL;

Re: [PATCH] uprobes: Remove redundant guard(rcu) in find_active_uprobe_speculative()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing `guard(rcu)()` from `find_active_uprobe_speculative()` eliminates standard RCU protection, exposing lockless VMA and file accesses to Use-After-Free and page faults.
--

--- Patch [1]: [PATCH] uprobes: Remove redundant guard(rcu) in find_active_uprobe_speculative() ---
commit 008639a39e6344f6f057b7d120c62792b7cd6ddf
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>

uprobes: Remove redundant guard(rcu) in find_active_uprobe_speculative()

This commit removes the guard(rcu)() from find_active_uprobe_speculative()
and adds a lockdep assertion for RCU Tasks Trace, assuming Tasks Trace RCU
protection provided by the caller is sufficient.

> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 07f69dd3093d5..52833db53b25c 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -2416,6 +2416,7 @@ static int is_trap_at_addr(struct mm_struct *mm, unsigned long vaddr)
>  	return is_trap_insn(&opcode);
>  }
>  
> +/* assumes being inside RCU Tasks Trace protected region */
>  static struct uprobe *find_active_uprobe_speculative(unsigned long bp_vaddr)
>  {
>  	struct mm_struct *mm = current->mm;
> @@ -2425,7 +2426,7 @@ static struct uprobe *find_active_uprobe_speculative(unsigned long bp_vaddr)
>  	loff_t offset;
>  	unsigned int seq;
>  
> -	guard(rcu)();
> +	lockdep_assert(rcu_read_lock_trace_held());
>  
>  	if (!mmap_lock_speculate_try_begin(mm, &seq))
>  		return NULL;

[Severity: High]
Does removing the standard RCU protection here expose the subsequent VMA and
file accesses to use-after-free and page faults?

The VMA and file objects are allocated from SLAB_TYPESAFE_BY_RCU caches,
meaning their underlying slab pages can be returned to the page allocator
after a standard RCU grace period. RCU Tasks Trace does not block standard
RCU grace periods.

The subsequent call to vma_lookup() briefly takes standard RCU to safely walk
the Maple Tree, but drops it before returning the VMA pointer. If the thread
is preempted after this point, and another thread unmaps the VMA and closes
the file, a standard RCU grace period could complete, freeing the VMA and file
slab pages.

When the thread wakes up and dereferences vma->vm_file or vm_file->f_inode on
a freed page, could it cause a fatal page fault? The code relies on the VMA
and file not being freed from under it, which appears to require standard RCU
protection.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/178537593930.153254.1296986092000402239.stgit@devnote2?part=1

Re: [PATCH] uprobes: Remove redundant guard(rcu) in find_active_uprobe_speculative()

[Masami Hiramatsu]

On Thu, 30 Jul 2026 01:59:47 +0000
sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Removing `guard(rcu)()` from `find_active_uprobe_speculative()` eliminates standard RCU protection, exposing lockless VMA and file accesses to Use-After-Free and page faults.
> --
> 
> --- Patch [1]: [PATCH] uprobes: Remove redundant guard(rcu) in find_active_uprobe_speculative() ---
> commit 008639a39e6344f6f057b7d120c62792b7cd6ddf
> Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> uprobes: Remove redundant guard(rcu) in find_active_uprobe_speculative()
> 
> This commit removes the guard(rcu)() from find_active_uprobe_speculative()
> and adds a lockdep assertion for RCU Tasks Trace, assuming Tasks Trace RCU
> protection provided by the caller is sufficient.
> 
> > diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> > index 07f69dd3093d5..52833db53b25c 100644
> > --- a/kernel/events/uprobes.c
> > +++ b/kernel/events/uprobes.c
> > @@ -2416,6 +2416,7 @@ static int is_trap_at_addr(struct mm_struct *mm, unsigned long vaddr)
> >  	return is_trap_insn(&opcode);
> >  }
> >  
> > +/* assumes being inside RCU Tasks Trace protected region */
> >  static struct uprobe *find_active_uprobe_speculative(unsigned long bp_vaddr)
> >  {
> >  	struct mm_struct *mm = current->mm;
> > @@ -2425,7 +2426,7 @@ static struct uprobe *find_active_uprobe_speculative(unsigned long bp_vaddr)
> >  	loff_t offset;
> >  	unsigned int seq;
> >  
> > -	guard(rcu)();
> > +	lockdep_assert(rcu_read_lock_trace_held());
> >  
> >  	if (!mmap_lock_speculate_try_begin(mm, &seq))
> >  		return NULL;
> 
> [Severity: High]
> Does removing the standard RCU protection here expose the subsequent VMA and
> file accesses to use-after-free and page faults?
> 

Oops, this guard() is for protecting VMA. OK, please ignore this.

Thanks,

> The VMA and file objects are allocated from SLAB_TYPESAFE_BY_RCU caches,
> meaning their underlying slab pages can be returned to the page allocator
> after a standard RCU grace period. RCU Tasks Trace does not block standard
> RCU grace periods.
> 
> The subsequent call to vma_lookup() briefly takes standard RCU to safely walk
> the Maple Tree, but drops it before returning the VMA pointer. If the thread
> is preempted after this point, and another thread unmaps the VMA and closes
> the file, a standard RCU grace period could complete, freeing the VMA and file
> slab pages.
> 
> When the thread wakes up and dereferences vma->vm_file or vm_file->f_inode on
> a freed page, could it cause a fatal page fault? The code relies on the VMA
> and file not being freed from under it, which appears to require standard RCU
> protection.
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/178537593930.153254.1296986092000402239.stgit@devnote2?part=1


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH] uprobes: Remove redundant guard(rcu) in find_active_uprobe_speculative()

[Masami Hiramatsu]

On Thu, 30 Jul 2026 10:45:39 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> find_active_uprobe_speculative() is called from find_active_uprobe_rcu(),
> which is invoked by handle_swbp() and handle_syscall_uprobe().
> Both callers already hold RCU Tasks Trace (via rcu_read_lock_trace() or
> guard(rcu_tasks_trace)()).
> 
> Calling guard(rcu)() inside find_active_uprobe_speculative() is redundant
> because the caupller already guarantees RCU Tasks Trace protection.
> Furthermore, standard RCU (rcu_read_lock()) does not protect uprobes_tree
> against asynchronous uprobe destruction (call_rcu_tasks_trace()).
> 
> Remove the redundant guard(rcu)() from find_active_uprobe_speculative() and
> replace it with lockdep_assert(rcu_read_lock_trace_held()) and a comment
> documenting that callers must hold RCU Tasks Trace protection, matching the
> convention used by find_active_uprobe_rcu() and find_uprobe_rcu().
> 
> Fixes: e0925f2dc4de ("uprobes: add speculative lockless VMA-to-inode-to-uprobe resolution")
> Assisted-by: Antigravity:gemini-3.6-flash
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Sashiko mentioned that this guard is not for uprobes_tree, but VMA lookup.
Please ignore this.

Thanks,

> ---
>  kernel/events/uprobes.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 4084e926e284..b4e253d43b30 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -2424,6 +2424,7 @@ static int is_trap_at_addr(struct mm_struct *mm, unsigned long vaddr)
>  	return is_trap_insn(&opcode);
>  }
>  
> +/* assumes being inside RCU Tasks Trace protected region */
>  static struct uprobe *find_active_uprobe_speculative(unsigned long bp_vaddr)
>  {
>  	struct mm_struct *mm = current->mm;
> @@ -2433,7 +2434,7 @@ static struct uprobe *find_active_uprobe_speculative(unsigned long bp_vaddr)
>  	loff_t offset;
>  	unsigned int seq;
>  
> -	guard(rcu)();
> +	lockdep_assert(rcu_read_lock_trace_held());
>  
>  	if (!mmap_lock_speculate_try_begin(mm, &seq))
>  		return NULL;
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>