Re: [PATCH bpf-next v1 06/18] selftests/bpf: Add tests for helper and kfunc mem+size arguments

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The selftest `mapofmaps_value_as_kfunc_mem_buf` uses an out-of-bounds buffer size (8 instead of 4), and both tests lack explicit `__msg(...)` assertions. This causes the test to pass (fail verification) for the wrong reason, rendering it ineffective as a regression guard.
--

--- Patch [6]: [PATCH bpf-next v1 06/18] selftests/bpf: Add tests for helper and kfunc mem+size arguments ---
commit 967fe45d86aee49807b5742ffece3f858b20a4fd
Author: Amery Hung <ameryhung@gmail.com>

selftests/bpf: Add tests for helper and kfunc mem+size arguments

Make sure the return of map-of-maps lookup cannot be passed to nullable
memory buffer argument for helper and kfunc.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_mem_size_reg.c b/tools/testing/selftests/bpf/progs/verifier_mem_size_reg.c
> new file mode 100644
> index 0000000000000..78a9ad22b10df
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_mem_size_reg.c
[ ... ]
> +/* An un-narrowed map-of-maps value must be rejected as a __nullable kfunc mem buffer */
> +SEC("?tc")
> +__failure
> +int mapofmaps_value_as_kfunc_mem_buf(struct __sk_buff *skb)
> +{
> +	struct bpf_dynptr dptr;
> +	__u32 key = 0;
> +	void *inner;
> +	char *p;
> +
> +	inner = bpf_map_lookup_elem(&outer_map, &key);
> +	/* intentionally NOT NULL-checked: reg stays PTR_TO_MAP_VALUE_OR_NULL */
> +
> +	bpf_dynptr_from_skb(skb, 0, &dptr);
> +	/* pass the un-narrowed map-of-maps value as the scratch buffer */
> +	p = bpf_dynptr_slice(&dptr, 0, inner, 8);

[Severity: Medium]
Does using a size of 8 here exceed the outer map's value size of 4?

If the intended type check is missing, the verifier will still reject the
program due to a bounds check failure (size 8 > 4). Since the test uses
__failure without __msg(...), this unrelated bounds-check failure satisfies
the test expectation and causes it to pass for the wrong reason.

> +	if (p)
> +		return p[0];
> +	return 0;
> +}
> +
> +/* An un-narrowed map-of-maps value must be rejected as a PTR_MAYBE_NULL helper mem buffer */
> +SEC("?tc")
> +__failure

[Severity: Medium]
Similar to the above, would it be better to include a __msg(...) assertion
here?

Without an explicit message check, this test could pass due to unrelated
verifier errors.

> +int mapofmaps_value_as_helper_mem_buf(struct __sk_buff *skb)
> +{
> +	__u32 key = 0;
> +	void *inner;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723050806.1158442-1-ameryhung@gmail.com?part=6