Re: [PATCH bpf-next] bpf: Fix security_bpf_map_create error handling

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Medium] Incomplete fix: the double LSM hook invocation bug fixed for maps also exists in the BPF program load path.
--

--- Patch [1]: [PATCH bpf-next] bpf: Fix security_bpf_map_create error handling ---
commit cf9b2144a32439abed11faad9a9b189d6b3f71f7
Author: Daniel Borkmann <daniel@iogearbox.net>

bpf: Fix security_bpf_map_create error handling

This commit prevents the security_bpf_map_free() hook from being called
twice when map creation is denied. It routes the map_create() denial
path to free_map instead of free_map_sec.

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -1649,7 +1649,7 @@ static int map_create(union bpf_attr *attr, bpfptr_t uattr, struct bpf_common_at
>  
>  	err = security_bpf_map_create(map, attr, token, uattr.is_kernel);
>  	if (err)
> -		goto free_map_sec;
> +		goto free_map;
>  
>  	err = bpf_map_alloc_id(map);
>  	if (err)

[Severity: Medium]
This isn't a bug introduced by this patch, but since you are fixing the
double invocation for maps, does the exact same issue also exist in the
BPF program load path?

Looking at the program load path, security_bpf_prog_load() cleans up the
LSM state internally upon denial:

security/security.c:security_bpf_prog_load() {
    ...
	if (unlikely(rc))
		security_bpf_prog_free(prog);
    ...
}

However, the error path in bpf_prog_load() eventually initiates an
unconditional program teardown:

kernel/bpf/syscall.c:bpf_prog_load() {
    ...
free_used_maps:
	/* In case we have subprogs, we need to wait for a grace
	 * period before we can tear down JIT memory since symbols
	 * are already exposed under kallsyms.
	 */
	__bpf_prog_put_noref(prog, prog->aux->real_func_cnt);
    ...
}

Which then unconditionally calls the free hook again:

kernel/bpf/syscall.c:__bpf_prog_put_rcu() {
    ...
	security_bpf_prog_free(aux->prog);
    ...
}

Could this lead to double invocations for denied programs (or unbalanced
invocations if verification fails before reaching the LSM load hook),
creating unexpected states for BPF LSM programs attached to bpf_prog_free?

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