Re: [PATCH bpf 1/2] uprobes/x86: Fix red zone clobbering in nop5 optimization

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] `resolve_uprobe_addr()` unconditionally uses `current->mm`, causing `is_optimized()` to fail when called from tracer context, which leads to target instruction stream corruption and potential kernel panics.
- [High] Exhausting a process's trampoline page slots permanently disables nop5 optimization for that uprobe across the entire system.
--

commit 2c63c19710be8b050bc0e126d84e52c24209e062
Author: Andrii Nakryiko <andrii@kernel.org>
Date:   Sat May 9 00:30:56 2026 +0000

uprobes/x86: Fix red zone clobbering in nop5 optimization

The x86 uprobe nop5 optimization currently replaces a 5-byte NOP at the
probe site with a CALL into a uprobe trampoline. This commit changes the
optimization to use JMP instead, introducing a trampoline page with slots
to handle register state saving and jumping to the syscall.

> diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
> index ebb1baf1eb1d0..7e1f14200bbbf 100644
> --- a/arch/x86/kernel/uprobes.c
> +++ b/arch/x86/kernel/uprobes.c

[ ... ]

> @@ -761,147 +785,153 @@ void arch_uprobe_clear_state(struct mm_struct *mm)
[ ... ]
> +static bool resolve_uprobe_addr(unsigned long ip, unsigned long *probe_addr)
>  {
> -	struct vm_area_struct *vma = vma_lookup(current->mm, ip);
> +	struct uprobes_state *state = &current->mm->uprobes_state;

Can this cause a NULL pointer dereference or instruction corruption when
executed outside the target task's context?

resolve_uprobe_addr() unconditionally uses current->mm. However, it is
called by __is_optimized() via is_optimized() during uprobe registration
and unregistration.

If a tracer closes a BPF link file descriptor, triggering uprobe_unregister()
in the tracer's context, current->mm will point to the tracer rather than the
target. This causes resolve_uprobe_addr() to fail, making is_optimized()
return false. As a result, set_orig_insn() might write only 1 byte over
the 5-byte JMP instruction, leaving 4 garbage bytes in the target's
instruction stream.

Furthermore, if unregistration runs from a kernel workqueue, current->mm
is NULL, and dereferencing current->mm->uprobes_state will cause a kernel
panic.

Could we pass the correct mm struct down from is_optimized() to
resolve_uprobe_addr() instead of relying on current->mm?

[ ... ]

> @@ -1113,8 +1149,9 @@ static int __arch_uprobe_optimize(struct arch_uprobe *auprobe, struct mm_struct
>  {
>  	struct uprobe_trampoline *tramp;
>  	struct vm_area_struct *vma;
> +	unsigned long slot_vaddr;
>  	bool new = false;
> -	int err = 0;
> +	int slot, err;
>  
>  	vma = find_vma(mm, vaddr);
>  	if (!vma)
> @@ -1122,8 +1159,17 @@ static int __arch_uprobe_optimize(struct arch_uprobe *auprobe, struct mm_struct
>  	tramp = get_uprobe_trampoline(vaddr, &new);
>  	if (!tramp)
>  		return -EINVAL;
> -	err = swbp_optimize(auprobe, vma, vaddr, tramp->vaddr);
> -	if (WARN_ON_ONCE(err) && new)
> +
> +	slot = tramp_alloc_slot(tramp, vaddr);
> +	if (slot < 0) {
> +		if (new)
> +			destroy_uprobe_trampoline(tramp);
> +		return slot;
> +	}

If a process exhausts its trampoline page slots, will this permanently
disable the nop5 optimization for this uprobe across the entire system?

If get_uprobe_trampoline() returns a reachable trampoline that is completely
full (all 256 slots used), tramp_alloc_slot() returns -ENOSPC. This error
propagates up to arch_uprobe_optimize(), which handles failures by setting
the ARCH_UPROBE_FLAG_OPTIMIZE_FAIL bit on auprobe->flags.

Since struct arch_uprobe is shared across all processes tracing the same
inode and offset, hitting the 256-slot limit in any single process seems
to permanently blacklist the probe from being optimized system-wide for
all other processes, even if those processes have completely empty
trampolines.

Should get_uprobe_trampoline() also check if the trampoline has available
slots before returning it, or can the failure be handled without setting
the global failure flag?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260509003146.976844-1-andrii@kernel.org?part=1