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

[sashiko-bot@kernel.org]

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?

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?

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