On Mon, Oct 26, 2026 at 09:52:21PM +0000, James Houghton wrote:
> get_perf_callchain() only checks is_user_task(current) when the regs it
> is given aren't user mode regs. However, BPF's bpf_get_task_stack() (and
> bpf_get_task_stack_sleepable()) pass task_pt_regs(current) when called on
> the current task, which are user mode regs for a user task, even if that
> task has already gone through exit_mm(), e.g. when called from a program
> attached to exit_files() or to the dying task's final sched_switch.
> 
> perf_callchain_user() then unwinds the user stack without an mm. That
> access does not reliably fail: the user half of the address space
> belongs to whatever mm happens to be lazily loaded, if any, so the
> unwinder may read another task's memory. This affects e.g. arm64,
> riscv, loongarch and csky, whose perf_callchain_user() doesn't check
> current->mm itself. perf events themselves are not affected, as
> perf_callchain() already clears the user callchain request when
> current is not a user task with an mm.
> 
> Check is_user_task(current) regardless of the regs' mode.
> 
> Fixes: fa28dcb82a38 ("bpf: Introduce helper bpf_get_task_stack()")
> Signed-off-by: James Houghton <[email protected]>
> ---
>  kernel/events/callchain.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/events/callchain.c b/kernel/events/callchain.c
> index 9d24b6e0c91f..a451ca2a5573 100644
> --- a/kernel/events/callchain.c
> +++ b/kernel/events/callchain.c
> @@ -245,11 +245,15 @@ get_perf_callchain(struct pt_regs *regs, bool kernel, 
> bool user,
>       }
>  
>       if (user && !crosstask) {
> -             if (!user_mode(regs)) {
> -                     if (!is_user_task(current))
> -                             goto exit_put;
> +             /*
> +              * Even if @regs are user mode regs, e.g. task_pt_regs(current)
> +              * passed in by bpf_get_task_stack(), current may already have
> +              * gone through exit_mm().
> +              */
> +             if (!is_user_task(current))
> +                     goto exit_put;
> +             if (!user_mode(regs))
>                       regs = task_pt_regs(current);
> -             }
>  
>               if (defer_cookie) {
>                       /*

We have perf_event_exit_task() before exit_mm().

Still, it doesn't hurt to be cautious. So no real objection, but perhaps
adjust the Changelog?

Reply via email to