Re: [PATCH bpf-next v2 20/23] selftests/bpf: Test kfunc packet memory direct writes

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Passing packet data to a kfunc expecting a struct causes unaligned memory accesses, which panics the kernel on strict-alignment architectures.
- [Medium] The verifier unconditionally treats all `ARG_PTR_TO_MEM` kfunc arguments as writable, causing unnecessary packet unsharing (cloning) for read-only kfuncs.
--

commit 4fcd6afbde5f30a90a43ed9c7692746d4f5d5555
Author: Amery Hung <ameryhung@gmail.com>

selftests/bpf: Test kfunc packet memory direct writes

A kfunc memory argument may be backed by packet data. Exercise this
with bpf_skb_ct_lookup(), which writes errors through its opts memory
argument. Verify that the verifier marks the program as directly
writing packet data, causing TC to emit its writable-skb prologue.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_kfunc_packet_access.c b/tools/testing/selftests/bpf/progs/verifier_kfunc_packet_access.c
> new file mode 100644
> index 0000000000000..88009566d92f9
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_kfunc_packet_access.c
[ ... ]
> +int kfunc_packet_write(struct __sk_buff *skb)
> +{
> +	void *data_end = (void *)(long)skb->data_end;
> +	void *data = (void *)(long)skb->data;
> +	struct bpf_sock_tuple tuple = {};
> +	struct nf_conn *nfct;
> +
> +	if (data + sizeof(struct bpf_ct_opts) > data_end)
> +		return 0;
> +
> +	/* An invalid tuple size makes bpf_skb_ct_lookup() write opts->error. */
> +	nfct = bpf_skb_ct_lookup(skb, &tuple, 1, data, sizeof(struct bpf_ct_opts));

[Severity: High]
This is a pre-existing issue, but will this test cause a kernel panic on
strict-alignment architectures?

The verifier's get_kfunc_arg_type() assigns ARG_PTR_TO_MEM | MEM_FIXED_SIZE
for kfunc struct arguments but fails to set MEM_ALIGNED. This allows BPF
programs to pass unaligned pointers like skb->data to kfuncs expecting structs.

When this selftest intentionally passes data as the opts parameter, the kfunc's
underlying C code in __bpf_nf_ct_lookup() dereferences the struct fields
directly (e.g., READ_ONCE(opts->netns_id)). On strict-alignment architectures
like SPARC or older ARM, this unaligned 32-bit read causes a hardware exception
and panics the kernel. Executing this selftest acts as a concrete panic vector.

[Severity: Medium]
This is a pre-existing issue, but does the verifier unconditionally treat all
ARG_PTR_TO_MEM kfunc arguments as writable?

While this selftest verifies the writable-skb prologue, the verifier's
get_kfunc_arg_type() never sets MEM_RDONLY based on the BTF const modifier.
Additionally, check_func_arg() forcefully applies BPF_READ | BPF_WRITE via
the if (meta->btf) condition.

Consequently, any memory argument passed to a kfunc is treated as writable.
When packet data (PTR_TO_PACKET) is passed to a purely read-only kfunc, the
verifier sets env->seen_direct_write = true, unnecessarily triggering the
bpf_unclone_prologue(). This forces a packet reallocation and copy for every
packet at runtime, degrading performance.

> +	if (nfct)
> +		bpf_ct_release(nfct);
> +	return 0;
> +}

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