Re: [PATCH] uprobes: Remove redundant guard(rcu) in find_active_uprobe_speculative()
Masami Hiramatsu (Google) <[email protected]> Thu, 30 Jul 2026 12:36:30 +0900
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-perf-users |
|---|---|
| Message-ID | <[email protected]> |
On Thu, 30 Jul 2026 01:59:47 +0000 [email protected] 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) <[email protected]> > > 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) <[email protected]>