Re: [PATCH bpf-next v2 1/6] bpf: Reject load-acquire from pointers requiring fault protection

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] A pre-existing flaw allows merged untrusted pointer types to bypass BPF_PROBE_MEM rewriting for plain BPF_LDX instructions, leading to kernel panics upon NULL pointer dereference.
--

--- Patch [1]: [PATCH bpf-next v2 1/6] bpf: Reject load-acquire from pointers requiring fault protection ---
commit e15f6fca389dca527ccadeba23f22ab6d2f2f36d
Author: Daniel Borkmann <daniel@iogearbox.net>

bpf: Reject load-acquire from pointers requiring fault protection

This commit prevents BPF_LOAD_ACQ instructions from using pointer types that
would normally require fault protection (like untrusted pointers or bare
PTR_TO_BTF_ID). Since BPF_LOAD_ACQ is not rewritten to use BPF_PROBE_MEM
by the verifier, a fault from these pointers would panic the kernel
instead of being safely handled. The fix explicitly rejects these unsafe
source pointer types for load-acquires.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index e6233c0081d10..648c5784178ec 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -4923,6 +4923,30 @@ static bool is_arena_reg(struct bpf_verifier_env *env, int regno)
>  	return reg->type == PTR_TO_ARENA;
>  }
>  
> +static bool is_load_acq_unsafe(struct bpf_verifier_env *env, int regno,
> +			       struct bpf_insn *insn)
> +{
> +	const struct bpf_reg_state *reg = reg_state(env, regno);
> +
> +	/*
> +	 * A BPF_LOAD_ACQ is not rewritten to a BPF_PROBE_MEM load by the
> +	 * verifier, unlike a regular BPF_LDX. The JIT would emit a plain load
> +	 * with no exception table entry, so a fault (e.g. NULL deref) crashes
> +	 * the kernel instead of being handled.
> +	 *
> +	 * Reject the source pointer types that a BPF_LDX would have had that
> +	 * fault protection applied to, i.e. the ones bpf_convert_ctx_accesses()
> +	 * turns into BPF_PROBE_MEM: a bare PTR_TO_BTF_ID and any PTR_UNTRUSTED
> +	 * pointer (untrusted btf ids, untrusted MEM_ALLOC, rdonly untrusted
> +	 * memory). A PTR_TRUSTED pointer is not among them, is not converted,
> +	 * and stays allowed. Same for the other flagged PTR_TO_BTF_ID variants
> +	 * (MEM_ALLOC, MEM_RCU, ...), hence the exact match on the base type.
> +	 */

[Severity: High]
This is a pre-existing issue, but does bpf_convert_ctx_accesses() actually
rewrite all PTR_UNTRUSTED pointers as the comment suggests?

It appears that if control flow paths merge at a BPF_LDX instruction,
save_aux_ptr_type() in kernel/bpf/verifier.c merges their pointer types:

    if (type & PTR_UNTRUSTED || *prev_type & PTR_UNTRUSTED)
        merged_type |= PTR_UNTRUSTED;
    if ((type & MEM_RDONLY) || (*prev_type & MEM_RDONLY))
        merged_type |= MEM_RDONLY;
    *prev_type = merged_type;

If one path uses a valid memory pointer like PTR_TO_MEM and the other uses
an untrusted pointer like PTR_TO_BTF_ID | PTR_UNTRUSTED, they merge into
PTR_TO_MEM | PTR_UNTRUSTED without the MEM_RDONLY flag.

However, looking at bpf_convert_ctx_accesses() in kernel/bpf/fixups.c:

    case PTR_TO_BTF_ID | MEM_ALLOC | PTR_UNTRUSTED:
    case PTR_TO_MEM | MEM_RDONLY | PTR_UNTRUSTED:
        if (type == BPF_READ) {
            if (BPF_MODE(insn->code) == BPF_MEM)

This switch statement doesn't have a case for PTR_TO_MEM | PTR_UNTRUSTED.
Could this allow a regular BPF_LDX instruction to bypass the BPF_PROBE_MEM
rewrite and cause a kernel panic if the untrusted pointer path is taken at
runtime and is NULL?

> +	return insn->imm == BPF_LOAD_ACQ &&
> +	       (reg->type == PTR_TO_BTF_ID ||
> +		(type_flag(reg->type) & PTR_UNTRUSTED));
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806201047.333389-1-daniel@iogearbox.net?part=1