Re: [PATCH bpf-next v6 3/8] libbpf: Drop in-loader metadata check for load-time verification

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] bpftool does not include the map contents when generating the PKCS#7 signature, which causes the kernel to always reject signed loaders.
- [High] The `run_gen_loader` function in the selftest does not set `attr.fd_array_cnt`, causing the verifier to reject the loader program with `-EACCES`.
--

--- Patch [3]: [PATCH bpf-next v6 3/8] libbpf: Drop in-loader metadata check for load-time verification ---
commit 7b60178a5e722682c8474b704e91ce40e0fc21ed
Author: Daniel Borkmann <daniel@iogearbox.net>

libbpf: Drop in-loader metadata check for load-time verification

This patch removes the in-BPF verification logic from the signed loader,
relying on the kernel to verify the metadata at BPF_PROG_LOAD time. It
also sets fd_array_cnt in skel_internal.h for signed loads so the kernel
can scan fd_array for the exclusive metadata map.

> libbpf: Drop in-loader metadata check for load-time verification
> 
> The signed gen_loader used to police its own metadata map from within
> BPF: emit_signature_match() read the kernel-cached map->sha[] back
> through hardcoded struct bpf_map offsets and compared it against a hash
> that compute_sha_update_offsets() baked into the signed instructions,
> after a BPF_OBJ_GET_INFO_BY_FD round-trip to populate map->sha[].
> 
> The kernel now verifies the metadata at BPF_PROG_LOAD time by folding
> the frozen contents of the loader's exclusive fd_array maps into the
> signature, so the loader no longer checks anything itself. Generated

[Severity: High]
Since the kernel now concatenates the map contents to the signed payload
during signature verification in bpf_prog_verify_signature(), does
bpftool need a corresponding update?

Currently, bpftool_prog_sign() in tools/bpf/bpftool/sign.c only signs
the BPF instructions (opts->insns) without including the map contents
(opts->data). Won't this cause the kernel to consistently reject loaders
signed by bpftool with -EKEYREJECTED?

> loaders thus carry no verification logic of their own anymore: Nothing
> in the signing chain depends on emitted loader bytecode doing the right
> thing.

[ ... ]

> diff --git a/tools/lib/bpf/skel_internal.h b/tools/lib/bpf/skel_internal.h
> index 74503d358bc8a..53fee53d36d51 100644
> --- a/tools/lib/bpf/skel_internal.h
> +++ b/tools/lib/bpf/skel_internal.h

[ ... ]

> @@ -400,6 +371,8 @@ static inline int bpf_load_and_run(struct bpf_load_and_run_opts *opts)
>  #ifndef __KERNEL__
>  	attr.signature = (long) opts->signature;
>  	attr.signature_size = opts->signature_sz;
> +	if (opts->signature)
> +		attr.fd_array_cnt = 1;

[Severity: High]
Does run_gen_loader() in
tools/testing/selftests/bpf/prog_tests/signed_loader.c need a similar
update to set attr.fd_array_cnt = 1?

Without this, when run_gen_loader() loads the signed program,
fd_array_cnt remains 0. The verifier's fd_array_get_map_idx() will then
see env->signature is set but env->fd_array is missing, which causes
the load to abort with -EACCES and breaks the selftest.

>  #else
>  	if (opts->signature || opts->signature_sz)
>  		pr_warn("signatures are not supported from bpf_preload\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708075343.358712-1-daniel@iogearbox.net?part=3