Re: [PATCHv2 bpf-next 2/2] bpf: Disable private stacks for untagged kprobe programs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] bpf: bpf_enable_priv_stack() allows private stack on uprobe
  via IOC_SET_BPF
- [Low] bpf: missing Fixes trailer for commit a76ab5731e32 in commit
  message
--

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

> bpf: Disable private stacks for untagged kprobe programs
>
> Uprobe programs can be preempted and re-entered on the same CPU. Since
> the private stack is shared per program and CPU, another invocation can
> overwrite the preempted invocation's stack.
>
> Programs without a multi or session attach type may be standard kprobes
> or uprobes. Disable private stacks for them, and retain private stacks
> only for kprobe multi and session programs, which have attach types that
> identify them as kprobes.
>
> Update verifier tests that rely on private stacks to use kprobe.multi,
> and add coverage that untagged kprobe and uprobe programs do not use
> private stacks.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>

[Severity: Low]
Should this commit include a Fixes tag for a76ab5731e32 ("bpf: Find
eligible subprogs for private stack support")?

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index b7329d164771c..1b58a1796a2e4 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5749,6 +5749,15 @@ static enum priv_stack_mode bpf_enable_priv_stack(struct bpf_prog *prog)
>  	 */
>  	switch (prog->type) {
>  	case BPF_PROG_TYPE_KPROBE:
> +		/*
> +		 * Uprobes are preemptible and lack recursion protection. Classic
> +		 * kprobe and uprobe progs have no expected attach type, so enable
> +		 * private stacks only for kprobe.multi and kprobe.session progs.
> +		 */
> +		if (prog->expected_attach_type != BPF_TRACE_KPROBE_MULTI &&
> +		    prog->expected_attach_type != BPF_TRACE_KPROBE_SESSION)
> +			return NO_PRIV_STACK;

[Severity: Medium]
Can a BPF_PROG_TYPE_KPROBE program with a private stack still be attached
to a classic uprobe through PERF_EVENT_IOC_SET_BPF?

At load time, bpf_prog_load_check_attach() accepts BPF_TRACE_KPROBE_MULTI
or BPF_TRACE_KPROBE_SESSION on any BPF_PROG_TYPE_KPROBE program (including
sleepable ones, since can_be_sleepable() only checks prog->type ==
BPF_PROG_TYPE_KPROBE).  While BPF_LINK_CREATE checks expected_attach_type
via bpf_prog_attach_check_attach_type(), attaching via
ioctl(perf_event_fd, PERF_EVENT_IOC_SET_BPF, prog_fd) ->
__perf_event_set_bpf_prog() only checks prog->type:

kernel/events/core.c:__perf_event_set_bpf_prog() {
    ...
	if (((is_kprobe || is_uprobe) && prog->type != BPF_PROG_TYPE_KPROBE) ||
	    (is_tracepoint && prog->type != BPF_PROG_TYPE_TRACEPOINT) ||
	    (is_syscall_tp && prog->type != BPF_PROG_TYPE_TRACEPOINT))
		return -EINVAL;
    ...
}

A BPF_PROG_TYPE_KPROBE program loaded with BPF_TRACE_KPROBE_MULTI or
BPF_TRACE_KPROBE_SESSION therefore still gets a per-CPU private stack and
can be attached to a uprobe perf_event, where __uprobe_perf_func() ->
bpf_prog_run_array_uprobe() runs it in preemptible (and optionally
sleepable) context without recursion protection, allowing preemption and
re-entry on the same CPU to overwrite the active invocation's private
stack.

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