Re: [PATCH bpf-next 2/4] bpf: Add BPF_TRACE_UPROBE attach type

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] bpf: private stack remains enabled for uprobes without BPF_TRACE_UPROBE
- [Medium] bpf: missing BPF_TRACE_UPROBE validation at prog load and attach
--

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

bpf: Add BPF_TRACE_UPROBE attach type

This commit adds the BPF_TRACE_UPROBE expected attach type for classic uprobe
programs, disables the private stack for programs using it in the verifier,
and rejects attaching them to non-uprobe perf events.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index a9c6a4b8eecce..7c941e009a337 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5707,7 +5707,8 @@ static enum priv_stack_mode bpf_enable_priv_stack(struct bpf_prog *prog)
>  	switch (prog->type) {
>  	case BPF_PROG_TYPE_KPROBE:
>  		/* Disable private stack for uprobe programs. */
> -		if (prog->expected_attach_type == BPF_TRACE_UPROBE_MULTI ||
> +		if (prog->expected_attach_type == BPF_TRACE_UPROBE ||
> +		    prog->expected_attach_type == BPF_TRACE_UPROBE_MULTI ||
>  		    prog->expected_attach_type == BPF_TRACE_UPROBE_SESSION)
>  			return NO_PRIV_STACK;
>  		fallthrough;

[Severity: High]
Can classic uprobe programs still end up with a private stack enabled and
corrupt per-CPU stack state when expected_attach_type is 0?

Classic uprobe programs run via __uprobe_perf_func() ->
bpf_prog_run_array_uprobe() in preemptible (and sleepable when
prog->sleepable is set) context with migrate_disable() and without any
per-CPU recursion guard:

include/linux/bpf.h:bpf_prog_run_array_uprobe() {
	...
	migrate_disable();
	while ((prog = READ_ONCE(item->prog))) {
		if (!prog->sleepable)
			rcu_read_lock();

		run_ctx.bpf_cookie = item->bpf_cookie;
		ret &= run_prog(prog, ctx);
		item++;

		if (!prog->sleepable)
			rcu_read_unlock();
	}
	migrate_enable();
	...
}

If a BPF_PROG_TYPE_KPROBE program with a stack depth of at least
BPF_PRIV_STACK_MIN_SIZE is loaded with expected_attach_type == 0 (including
programs with uprobe-only properties such as prog->sleepable or
prog->aux->kprobe_write_ctx), bpf_enable_priv_stack() still returns
PRIV_STACK_ADAPTIVE.

Even with libbpf updated in the follow-up commit to pass BPF_TRACE_UPROBE,
__perf_event_set_bpf_prog() still permits attaching a BPF_PROG_TYPE_KPROBE
program with expected_attach_type == 0 and prog->aux->jits_use_priv_stack
set to a uprobe perf event. If one task executing the uprobe on a CPU is
preempted or sleeps on a page fault and another task on the same CPU hits the
same uprobe, both tasks will share prog->aux->priv_stack_ptr and overwrite
each other's stack frames.

Should bpf_enable_priv_stack() also check prog->sleepable and
prog->aux->kprobe_write_ctx, and should __perf_event_set_bpf_prog() reject
attaching a program with prog->aux->jits_use_priv_stack to a uprobe perf
event?

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 457d75ea71420..016d479467edc 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -11835,6 +11835,9 @@ static int __perf_event_set_bpf_prog(struct perf_event *event,
>  	    (is_syscall_tp && prog->type != BPF_PROG_TYPE_TRACEPOINT))
>  		return -EINVAL;
>  
> +	if (prog->expected_attach_type == BPF_TRACE_UPROBE && !is_uprobe)
> +		return -EINVAL;
> +
>  	if (prog->type == BPF_PROG_TYPE_KPROBE && prog->sleepable && !is_uprobe)
>  		/* only uprobe programs are allowed to be sleepable */
>  		return -EINVAL;

[Severity: Medium]
What happens if a non-kprobe program is loaded with BPF_TRACE_UPROBE, or if a
kprobe program with BPF_TRACE_UPROBE is attached via BPF_LINK_CREATE with a
mismatched attach_type?

In kernel/bpf/syscall.c, bpf_prog_load_check_attach() does not restrict
BPF_TRACE_UPROBE to BPF_PROG_TYPE_KPROBE. If a BPF_PROG_TYPE_PERF_EVENT
program is loaded with expected_attach_type == BPF_TRACE_UPROBE and attached
to a non-tracing perf event, __perf_event_set_bpf_prog() returns early before
reaching the new check:

kernel/events/core.c:__perf_event_set_bpf_prog() {
	if (event->state <= PERF_EVENT_STATE_REVOKED)
		return -ENODEV;

	if (!perf_event_is_tracing(event))
		return perf_event_set_bpf_handler(event, prog, bpf_cookie);
	...
	if (prog->expected_attach_type == BPF_TRACE_UPROBE && !is_uprobe)
		return -EINVAL;
}

Additionally, in kernel/bpf/syscall.c, bpf_prog_attach_check_attach_type()
validates BPF_TRACE_KPROBE_MULTI, BPF_TRACE_KPROBE_SESSION,
BPF_TRACE_UPROBE_MULTI, and BPF_TRACE_UPROBE_SESSION under
BPF_PROG_TYPE_KPROBE, but does not check that prog->expected_attach_type ==
BPF_TRACE_UPROBE requires attach_type == BPF_PERF_EVENT:

kernel/bpf/syscall.c:bpf_prog_attach_check_attach_type() {
	...
	case BPF_PROG_TYPE_KPROBE:
		if (prog->expected_attach_type == BPF_TRACE_KPROBE_MULTI &&
		    attach_type != BPF_TRACE_KPROBE_MULTI)
			return -EINVAL;
		if (prog->expected_attach_type == BPF_TRACE_KPROBE_SESSION &&
		    attach_type != BPF_TRACE_KPROBE_SESSION)
			return -EINVAL;
		if (prog->expected_attach_type == BPF_TRACE_UPROBE_MULTI &&
		    attach_type != BPF_TRACE_UPROBE_MULTI)
			return -EINVAL;
		if (prog->expected_attach_type == BPF_TRACE_UPROBE_SESSION &&
		    attach_type != BPF_TRACE_UPROBE_SESSION)
			return -EINVAL;
}

If a BPF_PROG_TYPE_KPROBE program with expected_attach_type ==
BPF_TRACE_UPROBE is passed to link_create() with attach_type set to
BPF_TRACE_KPROBE_MULTI or BPF_TRACE_UPROBE_MULTI, it bypasses
bpf_prog_attach_check_attach_type() and returns -EOPNOTSUPP instead of
-EINVAL when CONFIG_FPROBE or CONFIG_UPROBES is disabled or on 32-bit
architectures.

Should bpf_prog_load_check_attach() and bpf_prog_attach_check_attach_type()
also validate BPF_TRACE_UPROBE?

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