Re: [PATCHv2 02/11] uprobes/x86: Allow to copy uprobe trampolines on fork

[Jiri Olsa <olsajiri@gmail.com>]

On Mon, May 18, 2026 at 11:42:08AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] __in_uprobe_trampoline uses current->mm instead of the target mm_struct, causing kernel crashes and instruction stream corruption during uprobe_unregister.
> - [High] Removing VM_DONTCOPY without synchronizing the uprobes state causes a linear VMA leak across fork chains.
> --
> 
> commit be0e6254bd9df4f56adce23d7a5e24a78c286291
> Author: Jiri Olsa <jolsa@kernel.org>
> uprobes/x86: Allow to copy uprobe trampolines on fork
> 
> This commit allows vma uprobe trampoline objects to be copied on fork to
> the new process by removing the VM_DONTCOPY flag.
> 
> > diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
> > --- a/arch/x86/kernel/uprobes.c
> > +++ b/arch/x86/kernel/uprobes.c
> > @@ -702,7 +702,7 @@ static struct uprobe_trampoline *create_uprobe_trampoline(unsigned long vaddr)
> >  
> >  	tramp->vaddr = vaddr;
> >  	vma = _install_special_mapping(mm, tramp->vaddr, PAGE_SIZE,
> > -				VM_READ|VM_EXEC|VM_MAYEXEC|VM_MAYREAD|VM_DONTCOPY|VM_IO,
> > +				VM_READ|VM_EXEC|VM_MAYEXEC|VM_MAYREAD|VM_IO,
> 
> Does removing VM_DONTCOPY without synchronizing the uprobes state cause a
> linear VMA leak across fork chains?
> 
> When dup_mmap() copies the uprobe trampoline VMA to the child process during
> fork(), the child's tracking list (mm->uprobes_state.head_tramps) is
> initialized as completely empty via mm_init_uprobes_state(), meaning the
> parent's state is not duplicated.
> 
> When the child process executes a uprobe, arch_uprobe_optimize() calls
> get_uprobe_trampoline(). Because head_tramps is empty, it fails to find the
> VMA it already inherited. It then falls back to create_uprobe_trampoline(),
> which calls find_nearest_trampoline(). Since the inherited VMA is already
> mapped, find_nearest_trampoline() finds a new unmapped area and installs a
> second duplicate trampoline VMA.
> 
> In a deep fork chain, each generation inherits all previous VMAs, has an
> empty head_tramps list, and creates a new VMA. Can this cause a linear
> accumulation of redundant VMAs, eventually exhausting vm.max_map_count and
> virtual memory?

I don't think it's big problem, because it's unlikely you'll insert extra uprobes
to child process.. but perhaps we could go away with struct uprobe_trampoline
object and operate directly on top of vma objects, it could event make code
easier, will try

> 
> As a related question about the uprobe trampolines introduced in the preceding
> commit ba2bfc97b462, does __in_uprobe_trampoline() safely handle unregistered
> uprobes?
> 
> When a uprobe is unregistered (e.g., via bpf_uprobe_unregister in a workqueue
> or by a different process like perf), set_orig_insn() calls is_optimized() to
> check if the uprobe is optimized. If is_optimized() internally calls
> __in_uprobe_trampoline() which hardcodes current->mm for the VMA lookup, does
> it completely ignore the passed mm parameter?

__in_uprobe_trampoline has no longer hardcoded current->mm, check
previous patch:

      uprobes/x86: Use proper mm_struct in __in_uprobe_trampoline

jirka

> 
> If unregistered asynchronously from a kernel thread, current->mm is NULL. Can
> this cause a deterministic NULL pointer dereference in vma_lookup() and crash
> the kernel?
> 
> Additionally, if unregistered by a different process, would vma_lookup()
> search the wrong mm, return false, and cause set_orig_insn() to incorrectly
> assume the instruction is not optimized? Could this lead to replacing only
> the first byte of the 5-byte CALL instruction with the original opcode,
> leaving the 4-byte relative offset as garbage and crashing the traced process
> when it executes the corrupted instruction?
> 
> >  				&tramp_mapping);
> >  	if (IS_ERR(vma)) {
> >  		kfree(tramp);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260518105957.123445-3-jolsa@kernel.org?part=1