Re: Uninitialized Stack Variable / NULL Pointer Dereference in __sys_socket_create via BPF LSM hooks

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [Critical] Incomplete fix for LSM cgroup return value bypass leaves other hooks vulnerable
--

commit 9f0f3f7cd06f4ada568fc6000ce559112bec1c80
Author: Quan Sun <2022090917019@std.uestc.edu.cn>

Uninitialized Stack Variable / NULL Pointer Dereference in __sys_socket_create via BPF LSM hooks

This commit introduces a new bpf_lsm_set_retval helper to prevent
bpf_lsm_socket_create from returning positive values and bypassing error
checks. It restricts the return value for this specific hook to avoid an
uninitialized stack variable dereference in __sys_socket_create.

> diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
> index c5c925f002021..c4b9e9d4de922 100644
> --- a/kernel/bpf/bpf_lsm.c
> +++ b/kernel/bpf/bpf_lsm.c
[ ... ]
> @@ -87,6 +87,13 @@ BTF_ID(func, bpf_lsm_socket_socketpair)
>  #endif
>  BTF_SET_END(bpf_lsm_unlocked_sockopt_hooks)
>  
> +BTF_SET_START(bpf_lsm_negative_set_retval_hooks)
> +#ifdef CONFIG_SECURITY_NETWORK
> +BTF_ID(func, bpf_lsm_socket_create)
> +#endif
> +BTF_SET_END(bpf_lsm_negative_set_retval_hooks)
> +

Does this fix leave other cgroup LSM hooks vulnerable to the same class
of bypasses?

For example, if a program attaches to bpf_lsm_socket_post_create and calls
bpf_set_retval(1), won't __sock_create() return 1 without initializing the
out-parameter? This would cause __sys_socket_create() to bypass its error
check and pass an uninitialized stack pointer to sock_map_fd(), triggering
the exact memory corruption this patch intends to fix.

Similarly, if a program returns 1 from bpf_lsm_socket_accept, do_accept()
will return ERR_PTR(1). The caller __sys_accept4_file() checks for errors
using IS_ERR(). Since IS_ERR only evaluates true for values >= -MAX_ERRNO,
ERR_PTR(1) bypasses the check, and the invalid pointer is installed into
the file descriptor table.

>  #ifdef CONFIG_CGROUP_BPF
>  void bpf_lsm_find_cgroup_shim(const struct bpf_prog *prog,
>  			     bpf_func_t *bpf_func)
[ ... ]
> @@ -221,12 +228,37 @@ static const struct bpf_func_proto bpf_get_attach_cookie_proto = {
>  	.arg1_type	= ARG_PTR_TO_CTX,
>  };
>  
> +BPF_CALL_1(bpf_lsm_set_retval, int, retval)
> +{
> +	struct bpf_cg_run_ctx *ctx;
> +
> +	if (retval > 0)
> +		return -EINVAL;

Can a BPF program still bypass caller error checks by passing a large
negative value?

Because this check only blocks values greater than 0, a program could call
bpf_set_retval(-5000). Since -5000 is less than -MAX_ERRNO (-4095),
ERR_PTR(-5000) also evaluates to false under IS_ERR(). This completely
bypasses error checks in callers expecting standard error codes.

Should bpf_set_retval for cgroup LSM programs globally validate the return
value against the allowed range for the specific hook being called, rather
than relying on a single-hook allowlist that only checks for positive values?

> +
> +	ctx = container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
> +	ctx->retval = retval;
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260421084806.1785541-1-kuniyu@google.com?part=1