Re: [PATCHv4 04/13] uprobes/x86: Unmap trampoline vma object in case it's unused

[Jiri Olsa <olsajiri@gmail.com>]

On Tue, May 26, 2026 at 09:46:49PM +0000, bot+bpf-ci@kernel.org wrote:
> > commit 68883203f0583fb1a8d79536f5c8945542974abd
> > Author: Jiri Olsa <jolsa@kernel.org>
> > Date:   Tue May 26 22:58:31 2026 +0200
> >
> >     uprobes/x86: Unmap trampoline vma object in case it's unused
> >
> >     In case the optimization fails, we leak new-ly created trampoline
> >     vma mapping (in case we just created it), let's unmap it.
> >
> >     Fixes: ba2bfc97b462 ("uprobes/x86: Add support to optimize uprobes")
> >     Signed-off-by: Jiri Olsa <jolsa@kernel.org>
> 
> Should this Fixes: tag point to a9fe6f54968c instead? The vma leak bug
> was actually introduced by commit a9fe6f54968c ("uprobes/x86: Remove
> struct uprobe_trampoline object"), not ba2bfc97b462. Commit a9fe6f54968c
> removed the destroy_uprobe_trampoline() cleanup call, and its commit
> message explicitly states: "The new vma leak is fixed in following
> change."

there's no a9fe6f54968c hash yet

> 
> Suggested: Fixes: a9fe6f54968c ("uprobes/x86: Remove struct
> uprobe_trampoline object")
> 
> > diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
> > index c8af41ed681a..af5af7d67999 100644
> > --- a/arch/x86/kernel/uprobes.c
> > +++ b/arch/x86/kernel/uprobes.c
> > @@ -677,11 +677,14 @@ static unsigned long find_nearest_trampoline(unsigned long vaddr)
> >  	return high_tramp;
> >  }
> >
> > -static struct vm_area_struct *get_uprobe_trampoline(struct mm_struct *mm, unsigned long vaddr)
> > +static struct vm_area_struct *get_uprobe_trampoline(struct mm_struct *mm, unsigned long vaddr,
> > +						    bool *new_mapping)
> >  {
> >  	VMA_ITERATOR(vmi, mm, 0);
> >  	struct vm_area_struct *vma;
> >
> > +	*new_mapping = false;
> > +
> >  	if (vaddr > TASK_SIZE || vaddr < PAGE_SIZE)
> >  		return ERR_PTR(-EINVAL);
> >
> 
> Does this fix address the VMA leak scenario across fork chains? In v2,
> sashiko-bot raised a concern about inherited-but-not-tracked VMAs:
> 
> https://lore.kernel.org/bpf/20260518114208.CE128C2BCF6@smtp.kernel.org/
> 
> When VM_DONTCOPY is removed, child processes inherit trampoline VMAs
> but their tracking list (mm->uprobes_state.head_tramps) is empty. When
> the child executes a uprobe, get_uprobe_trampoline() fails to find the
> inherited VMA and creates a duplicate trampoline.
> 
> Child fork:
>   dup_mmap()
>     copies trampoline VMA to child
>   mm_init_uprobes_state()
>     initializes head_tramps as empty (parent's state not copied)
> 
> Child executes uprobe:
>   arch_uprobe_optimize()
>     get_uprobe_trampoline()
>       head_tramps is empty, fails to find inherited VMA
>       create_uprobe_trampoline()
>         find_nearest_trampoline() finds new unmapped area
>         installs 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 linear
> accumulation of redundant VMAs, eventually exhausting vm.max_map_count?
> 
> While this patch addresses unmapping failed optimization attempts, it
> does not prevent the inherited-but-not-tracked VMA leak scenario in fork
> chains.

there's not head_tramps list anymore, was removed by earlier:
      uprobes/x86: Remove struct uprobe_trampoline object

jirka