Re: [PATCH bpf-next v5 13/14] bpf: Reject tracing/freplace progs for struct_ops with arena args

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] The patch restricts attaching extension or tracing programs to the main program (`subprog == 0`) of a target with arena context arguments, but explicitly fails to restrict attachments to global subprograms (`subprog > 0`). This leaves a type confusion vulnerability open where a 32-bit user-controlled arena offset is treated as a trusted 64-bit kernel pointer.
- [High] A `WARN_ON_ONCE` check in the trampoline linking code incorrectly validates the attached tracing/extension program instead of the target program, rendering the safety check ineffective.
--

--- Patch [13]: [PATCH bpf-next v5 13/14] bpf: Reject tracing/freplace progs for struct_ops with arena args ---
commit b54b0858382f938c9a42e0941274db3241ba575f
Author: Kumar Kartikeya Dwivedi <[email protected]>

    bpf: Reject tracing/freplace progs for struct_ops with arena args

    This commit restricts attaching tracing and extension programs to a target
    program with arena context arguments. The restriction is added because the
    struct_ops indirect trampoline converts these arguments, and a generic
    tracing trampoline would expose the converted offset as a regular BTF
    pointer.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 74592272b124d..4bb3102ae4333 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -19258,6 +19258,16 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
>  			bpf_log(log, "Subprog %s doesn't exist\n", tname);
>  			return -EINVAL;
>  		}
> +		/*
> +		 * A struct_ops indirect trampoline converts arena arguments
> +		 * before invoking its program. A tracing or extension program
> +		 * attached to the main program would see the converted offset as a
> +		 * regular BTF pointer.
> +		 */
> +		if (subprog == 0 && bpf_prog_has_arena_ctx_arg(tgt_prog)) {

[Severity: Critical]
Does this code leave a type confusion vulnerability open for global
subprograms?

This check explicitly requires subprog == 0 and relies on ctx_arg_info from
the main program, bypassing protection for attachments to global subprograms.

Can an attacker load a struct_ops program that passes its converted 32-bit
arena offset to a global subprogram taking a struct bpf_arena * argument with
the __arg_arena tag, and then attach an fentry tracing program to the global
subprogram?

In btf_ctx_access(), this loop is skipped for fentry programs because their
ctx_arg_info_size is 0:

kernel/bpf/btf.c:btf_ctx_access() {
    ...
	for (i = 0; i < prog->aux->ctx_arg_info_size; i++) {
    ...
}

Would this cause the verifier to fall through, ignore the __arg_arena tag,
and incorrectly type the intercepted 32-bit offset as a 64-bit
PTR_TO_BTF_ID | PTR_TRUSTED kernel pointer, leading to arbitrary
out-of-bounds kernel memory read/write?

> +			bpf_log(log, "Cannot attach to a target with arena context arguments\n");
> +			return -EOPNOTSUPP;
> +		}
>  		if (aux->func && aux->func[subprog]->aux->exception_cb) {
>  			bpf_log(log,
>  				"%s programs cannot attach to exception callback\n",

[Severity: High]
Does the WARN_ON_ONCE check in the trampoline linking code incorrectly
validate the attached tracing/extension program instead of the target
program?

In __bpf_trampoline_link_prog():

kernel/bpf/trampoline.c:__bpf_trampoline_link_prog() {
    ...
	if (WARN_ON_ONCE(bpf_prog_has_arena_ctx_arg(node->link->prog)))
		return -ENOTSUPP;
    ...
}

When an fentry or freplace program is linked via bpf_trampoline_link_prog(),
__bpf_trampoline_link_prog() calls bpf_prog_has_arena_ctx_arg() with
node->link->prog.

Since node->link->prog is the fentry/freplace program (which never has
ctx_arg_info populated), doesn't this check always evaluate to false?

Should it have checked tgt_prog instead, so it doesn't fail to prevent
generic trampolines from attaching to programs with arena arguments?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13