Re: [PATCH bpf-next 2/4] bpf: Merge pointer types also when both are PTR_TO_MEM

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

Pre-existing issues:
- [High] The patch fixes a missing BPF_PROBE_MEM rewrite when an untrusted PTR_TO_MEM shares an instruction with a trusted PTR_TO_MEM, but leaves the exact same vulnerability open when mixed with PTR_TO_STACK, PTR_TO_MAP_VALUE, or other generic pointer types.
--

--- Patch [2]: [PATCH bpf-next 2/4] bpf: Merge pointer types also when both are PTR_TO_MEM ---
commit 7f1a4eb01f4f65011b9dba6050e4321da3dbce61
Author: Daniel Borkmann <[email protected]>

bpf: Merge pointer types also when both are PTR_TO_MEM

This commit fixes a verifier issue where merging a trusted PTR_TO_MEM and an
untrusted PTR_TO_MEM failed to trigger the BPF_PROBE_MEM rewrite. It updates
save_aux_ptr_type() to correctly merge these types when they intersect.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -17096,6 +17096,17 @@ static int save_aux_ptr_type(struct bpf_verifier_env *env, enum bpf_reg_type typ
>  		 * save type to validate intersecting paths
>  		 */
>  		*prev_type = type;
> +	} else if (*prev_type != type && allow_trust_mismatch &&
> +		   is_ptr_to_mem_or_btf_id(type) &&
> +		   is_ptr_to_mem_or_btf_id(*prev_type)) {
> +		/*
> +		 * Have to support a use case when one path through the
> +		 * program yields a TRUSTED pointer while another is
> +		 * UNTRUSTED. Merge them into a type which keeps the
> +		 * BPF_PROBE_MEM/BPF_PROBE_MEMSX rewrite when either
> +		 * side needs it.
> +		 */
> +		*prev_type = merge_ptr_types(type, *prev_type);
>  	} else if (reg_type_mismatch(type, *prev_type)) {

[Severity: High]
This is a pre-existing issue, but does this update still leave a vulnerability
open if an untrusted PTR_TO_MEM intersects with PTR_TO_STACK or
PTR_TO_MAP_VALUE?

If a BPF program has a conditional branch where a memory load is reached by
either a PTR_TO_STACK or an untrusted PTR_TO_MEM | PTR_UNTRUSTED (e.g. from
bpf_rdonly_cast()), the verifier might explore the PTR_TO_STACK path first.

When it evaluates the untrusted memory path in save_aux_ptr_type(), it will
skip the new type-merging logic because PTR_TO_STACK fails the
is_ptr_to_mem_or_btf_id() check.

It then falls through to reg_type_mismatch(), which returns false because both
types satisfy reg_type_mismatch_ok(). The verifier permits the shared
instruction but leaves ptr_type as PTR_TO_STACK.

During JIT compilation, fixup_bpf_calls() would then observe PTR_TO_STACK and
skip the BPF_PROBE_MEM rewrite. At runtime, when the untrusted branch executes,
could this result in an unhandled page fault and kernel panic if it performs
a plain memory load on an unverified address?

>  		/* Abuser program is trying to use the same insn
>  		 * dst_reg = *(u32*) (src_reg + off)

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