Re: [PATCH bpf-next 15/36] bpf: varying offset access support for PTR_TO_BTF_ID pointers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: varying offset accesses to nested flexible arrays are incorrectly rejected
--

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

bpf: varying offset access support for PTR_TO_BTF_ID pointers

This commit relaxes the verifier requirements for reads like p->arr[i].field 
where p is a PTR_TO_BTF_ID, allowing i to be a non-constant value. It 
introduces bounds checking based on the access register's min and max bounds, 
along with alignment checks.

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
[ ... ]
> @@ -7846,6 +7905,54 @@ int btf_struct_access(struct bpf_verifier_log *log,
>  	} while (t);
>  
>  	return -EINVAL;
> +
> +check_variable_offset:
> +	if (min_off == max_off)
> +		return ret;
> +
> +	/* Find an offset at which access would go to a flexible array tail (if any). */
> +	t = btf_type_skip_modifiers(btf, reg->btf_id, NULL);
> +	member = find_flex_member(btf, t);
> +	flex_off = member ? __btf_member_bit_offset(t, member) / 8 : S64_MAX;

[Severity: Medium]
Does this code handle flexible arrays embedded within nested structs?

When a BPF program attempts a variable-offset access (for example,
ptr->nested_struct.flex_array[i]) on a PTR_TO_BTF_ID, find_flex_member()
is called strictly on the top-level struct type ID.

Because find_flex_member() does not recursively evaluate nested structs,
it returns NULL in this case. This causes flex_off to evaluate to S64_MAX,
which later makes the array bounds check fail since the flexible array
has a size of 0, resulting in -EINVAL being returned.

Can this be updated so the verifier doesn't incorrectly reject valid programs
performing varying-offset accesses to nested flexible arrays?

> +
> +	/*
> +	 * If this is a varying offset access, the step recorded within a register
> +	 * should correspond to one of the arrays visited while walking.
> +	 */
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260926-scev-minimal-rebase-v1-0-c8e5ab5ba79f@gmail.com?part=15