Re: [PATCH bpf-next v5 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 1 potential issue(s) to consider:
- [Critical] Removal of the 'excl' field from 'struct bpf_map' and the new requirement for 'fd_array_cnt > 0' on signed programs breaks backward compatibility with statically compiled userspace binaries containing BPF light skeletons generated by older versions of bpftool.
--

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

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

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
loaders thus carry no verification logic of their own anymore. The struct
bpf_map layout BUILD_BUG_ON()s on the kernel side are removed as well.

> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index adf53f7edf287..c1a98fa367381 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -299,7 +299,6 @@ struct bpf_map_owner {
>  
>  struct bpf_map {
>  	u8 sha[SHA256_DIGEST_SIZE];
> -	u32 excl;
>  	const struct bpf_map_ops *ops;
>  	struct bpf_map *inner_map_meta;

[Severity: Critical]
Does removing the excl field break backward compatibility with older
userspace binaries that contain statically compiled BPF light skeletons?

Older skeletons generated by bpftool (with gen_hash=true) include a BPF loader
that hardcodes a 4-byte read at offset 32 of struct bpf_map to verify the
signature by reading the excl field:

emit_signature_match()
...
emit(gen, BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, SHA256_DIGEST_LENGTH));

With the excl field removed, this offset now points to the 8-byte ops pointer
(on 64-bit platforms).

Will the BPF verifier's btf_struct_access() reject partial reads of pointers
and return -EACCES?

On 32-bit architectures, could this result in a pointer-to-scalar comparison
failing verification?

Will this cause existing, statically compiled binaries to unconditionally
fail to load on the new kernel?

[ ... ]
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index e898fad01aafe..358f2b0ce2bd3 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1598,13 +1598,6 @@ static int map_create_alloc(union bpf_attr *attr, bpfptr_t uattr, struct bpf_ver
>  			err = -EFAULT;
>  			goto free_map;
>  		}
> -
> -		/* See libbpf: emit_signature_match() */
> -		BUILD_BUG_ON(offsetof(struct bpf_map, excl) != SHA256_DIGEST_SIZE);

[Severity: Critical]
Is it safe to remove this ABI contract guarantee?

Older generated loaders depend on this exact layout to successfully load.

[ ... ]
> 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: Critical]
Can relying on fd_array_cnt break old skeleton loaders?

The verifier in kernel/bpf/verifier.c:fd_array_get_map_idx() explicitly rejects
signed BPF programs that do not set fd_array_cnt:

if (env->signature) {
	verbose(env, "signed program must bind maps via a continuous fd_array (fd_array_cnt)\n");
	return -EACCES;
}

Since older bpftool-generated user-space stubs did not set fd_array_cnt,
will this cause all existing signed skeletons to fail to load on the new kernel?

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