Re: [PATCH v7 04/10] lsm: framework for BPF integrity verification

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Double execution of `security_bpf_prog_free()` on error paths, leading to duplicate invocation of LSM cleanup hooks.
--

commit da3e568b0e83968c7e672f02d2dc4e5b4cb4a3e9
Author: Paul Moore <paul@paul-moore.com>

lsm: framework for BPF integrity verification

Add a new LSM hook and two new LSM hook callbacks to support LSMs that
perform integrity verification, e.g. digital signature verification,
of BPF programs.

> diff --git a/security/security.c b/security/security.c
> index a26c1474e2e49..bb78f7e45a98f 100644
> --- a/security/security.c
> +++ b/security/security.c
[ ... ]
> @@ -5255,9 +5315,18 @@ int security_bpf_prog_load(struct bpf_prog *prog, union bpf_attr *attr,
>  	if (unlikely(rc))
>  		return rc;
>  
> +	rc = call_int_hook(bpf_prog_load_integrity, prog, attr, token, kernel);
> +	if (unlikely(rc))
> +		goto err;
> +
>  	rc = call_int_hook(bpf_prog_load, prog, attr, token, kernel);
>  	if (unlikely(rc))
> -		security_bpf_prog_free(prog);
> +		goto err;
> +
> +	return rc;
> +
> +err:
> +	security_bpf_prog_free(prog);
>  	return rc;
>  }

Does this introduce a regression by causing a double execution of
security_bpf_prog_free() on error paths?

If either the new bpf_prog_load_integrity hook or the existing
bpf_prog_load hook fails, the code jumps to the err label, calls
security_bpf_prog_free(), and returns the error code.

However, the caller bpf_prog_load() in kernel/bpf/syscall.c receives the
error code and jumps to the free_prog_sec label, which unconditionally calls
security_bpf_prog_free() again:

kernel/bpf/syscall.c:bpf_prog_load() {
    ...
    err = security_bpf_prog_load(prog, attr, token, bpf_prog_is_kernel(prog));
    if (err)
        goto free_prog_sec;
    ...
free_prog_sec:
    ...
    security_bpf_prog_free(prog);
    ...
}

This causes the bpf_prog_free LSM hook to execute twice for the same BPF
program. During the first call, prog->aux->security is freed and set to
NULL. The second call invokes the hook again with the NULL security blob.

Could this duplicate invocation violate lifecycle symmetry and potentially
cause reference count underflows, state corruption, or NULL pointer
dereferences in attached BPF LSM programs?

While this double-free behavior already existed for the bpf_prog_load hook,
this patch expands the flawed error handling to the newly introduced
bpf_prog_load_integrity hook. Should the internal cleanup in
security_bpf_prog_load() be removed to let the caller handle it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260507191416.2984054-1-bboscaccy@linux.microsoft.com?part=4