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

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

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>