[PATCH bpf-next v4 3/9] bpf: Verify signed loader metadata at load time

[bot+bpf-ci at kernel.org]

> A signed gen_loader program carries the programs, maps and relocations
> it installs in a metadata array map. The loader instructions are covered
> by the PKCS#7 signature, but the metadata map is not: Today the loader
> compares the map contents from within BPF against a hash baked into its
> (signed) instructions, using the kernel-cached map hash. The kernel
> itself never actually attests that the metadata the loader installs is
> the metadata that was signed.
>
> [ ... ]
>
> A signed loader binds its metadata map(s) through the existing fd_array,
> and an exclusive map is already bound to a program digest (excl_prog_hash).
> So when a signature is present, collect the exclusive maps from fd_array
> and append their frozen contents to the instructions before verification:
> The signature now covers insns || metadata_0 || metadata_1 || [...] in
> the fd_array order, and verification completes in bpf_check(), once the
> fd_array maps are resolved into used_maps, before the LSM admission hook
> and the rest of verification.
>
> [ ... ]
>
> Signed-off-by: Daniel Borkmann <daniel at iogearbox.net>

This changes how the existing BPF program signing scheme attests the
loader payload, and reworks the verification that was added when
signature support first landed.

Should it carry a Fixes: tag pointing at the commit that introduced the
signing scheme, for example:

  Fixes: 349271568303 ("bpf: Implement signature verification for BPF programs")

That commit added attr->signature/signature_size/keyring_id and the
original bpf_prog_verify_signature() that only covered prog->insnsi. The
later commit ea1535e28bb3 ("bpf: Limit bpf program signature size")
already uses this same target.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index e227c76a8dd9..65f53128fc33 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -19909,11 +20068,27 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr,
>  	ret = bpf_vlog_init(&env->log, attr_log->level, attr_log->ubuf, attr_log->size);
>  	if (ret)
>  		goto err_free_env;
> +	if (env->signature) {
> +		ret = bpf_prog_calc_tag(env->prog);
> +		if (ret < 0)
> +			goto err_prep;
> +	}
>
>  	ret = process_fd_array(env, attr, uattr);
>  	if (ret)
>  		goto err_prep;
>
> +	if (env->signature) {
> +		ret = bpf_prog_verify_signature(env, attr, uattr.is_kernel);
> +		if (ret)
> +			goto err_prep;
> +	}
> +
> +	ret = security_bpf_prog_load(env->prog, attr, env->prog->aux->token,
> +				     uattr.is_kernel);
> +	if (ret)
> +		goto err_prep;

This moves the security_bpf_prog_load() admission hook out of
bpf_prog_load() in syscall.c and into bpf_check(), now placed after
process_fd_array().

At this point the program instructions have already been allocated and
copied in, up to the maximum program size. Does relocating the hook here
mean that for a policy which denies a load, the denial now happens only
after that allocation, rather than before it as in the previous
placement?

This placement carries a Nacked-by from the LSM maintainer on the same
grounds, and the code keeps the hook in the new location. Is it worth
resolving that objection before this lands, or is the intent to leave it
for the merge-window decision?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/28797884155