Re: [PATCHv5 01/13] uprobes/x86: Use proper mm_struct in __in_uprobe_trampoline

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing vma_start_write() before modifying page tables under mmap_write_lock() in uprobe modification paths.
--

commit e98c98f0853de23f35b1d0253c941b8b823e5967
Author: Jiri Olsa <jolsa@kernel.org>

uprobes/x86: Use proper mm_struct in __in_uprobe_trampoline

This commit updates the unregister path to pass the correct mm_struct to
__in_uprobe_trampoline instead of relying on current->mm. It adds the
mm_struct argument and updates related callers to provide the proper
pointer in the tracer context.

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

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but there is a pre-existing
issue in the uprobes subsystem regarding the per-VMA locks.

Does the uprobe registration and optimization path race with concurrent
per-VMA lock readers?

In arch_uprobe_optimize(), the code takes the mmap_write_lock but doesn't
call vma_start_write() on the target VMA:

arch/x86/kernel/uprobes.c:arch_uprobe_optimize() {
        ...
        mmap_write_lock(mm);
        ...
        if (__arch_uprobe_optimize(auprobe, mm, vaddr))
        ...
}

A similar sequence happens in register_for_each_vma():

kernel/events/uprobes.c:register_for_each_vma() {
        ...
        mmap_write_lock(mm);
        if (check_stable_address_space(mm))
                goto unlock;

        vma = find_vma(mm, info->vaddr);
        ...
        err = install_breakpoint(uprobe, vma, info->vaddr);
        ...
}

Both of these paths eventually call down to __uprobe_write(), which
modifies the page tables directly:

kernel/events/uprobes.c:__uprobe_write() {
        ...
        fw->pte = ptep_clear_flush(vma, vaddr, fw->ptep);
        ...
}

Since mmap_write_lock() alone does not drain existing per-VMA read lock
holders (its sequence counter only prevents new acquisitions), could this
cause a race where concurrent paths executing under the per-VMA lock
(like page faults or MADV_DONTNEED) read stale page table state while
__uprobe_write() clears and flushes the PTEs? 

Should these paths call vma_start_write() to safely drain the per-VMA
readers before making page table modifications?

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