Re: [PATCH bpf-next v3 20/51] bpf: varying offset access support for PTR_TO_BTF_ID pointers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] bpf: truncation of 64-bit var_off to 32-bit bypasses custom btf_struct_access bounds
- [High] bpf: unbounded variable offset read on trusted PTR_TO_BTF_ID bypasses PROBE_MEM
--

commit ec6d1e8a8c55dc442e46ac3d70d36a00ec8a9ffd
Author: Eduard Zingerman <eddyz87@gmail.com>

bpf: varying offset access support for PTR_TO_BTF_ID pointers

This commit relaxes requirements for varying offset accesses to PTR_TO_BTF_ID
pointers, verifying they stay within array bounds and align with elements.
It modifies btf_struct_access to validate minimum offsets and array spans,
while skipping upper bounds checks for flexible arrays.

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 0630675377aaa..f4d119f3d5ef9 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
[ ... ]
> @@ -7899,6 +7961,55 @@ int btf_struct_access(struct bpf_verifier_log *log,
[ ... ]
> +		array_start = min_off - arrays[i].off;
> +		/* An array within the flexible tail has no upper bound to exceed */
> +		if (array_start >= flex_off && btf_type_is_array(t) &&
> +		    btf_type_array(t)->nelems == 0)
> +			return ret;

[Severity: High]
Does this allow unbounded variable offset reads on trusted pointers?

This logic explicitly skips the upper bound check for flexible arrays,
assuming PROBE_MEM will safely catch out-of-bounds page faults at runtime.

However, if the pointer being accessed is trusted (meaning it has the
PTR_TRUSTED flag and lacks PTR_UNTRUSTED), wouldn't bpf_may_fault_on_deref()
evaluate to false?

If bpf_may_fault_on_deref() is false, bpf_convert_ctx_accesses() won't
convert the load into a PROBE_MEM exception-handled load. Passing an
arbitrarily large offset here could result in a native load from unmapped
memory, potentially crashing the kernel.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index f5b2500335a7b..192efb7229975 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -6608,6 +6604,18 @@ static int check_ptr_to_btf_access(struct bpf_verifier_env *env,
>  	}
>  
>  	if (env->ops->btf_struct_access && !type_is_alloc(reg->type) && atype == BPF_WRITE) {
> +		if (!tnum_is_const(reg->var_off)) {
> +			char tn_buf[48];
> +
> +			tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
> +			verbose(env,
> +				"%s is ptr_%s invalid variable offset: off=%d, var_off=%s\n",
> +				reg_arg_name(env, argno), tname, off, tn_buf);
> +			return -EACCES;
> +		}
> +
> +		off += reg->var_off.value;
> +
>  		if (!btf_is_kernel(reg->btf)) {
>  			verifier_bug(env, "reg->btf must be kernel btf");
>  			return -EFAULT;

[Severity: High]
Could this assignment truncate a large 64-bit value into a negative 32-bit
offset?

Earlier in check_ptr_to_btf_access(), the negative check for off was removed
and replaced with min_off. Now that off += reg->var_off.value is executed
down here for BPF_WRITE with custom handlers, a very large 64-bit
var_off.value could wrap into a negative 32-bit integer.

Since there is no negativity check immediately following this addition, the
truncated negative offset gets passed directly into the custom
btf_struct_access handler.

If a handler like bpf_scx_btf_struct_access compares this negative offset
against size_t boundaries, couldn't this bypass the bounds check and
allow out-of-bounds writes to kernel objects?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-scev-minimal-rebase-v3-0-bcba9bc53ac9@gmail.com?part=20