public inbox for bpf@vger.kernel.org
 help / color / mirror / Atom feed
From: Kuniyuki Iwashima <kuniyu@google.com>
To: mattbobrowski@google.com
Cc: 2022090917019@std.uestc.edu.cn, M202472210@hust.edu.cn,
	 bpf@vger.kernel.org, daniel@iogearbox.net, dddddd@hust.edu.cn,
	 dzm91@hust.edu.cn, edumazet@google.com,
	 hust-os-kernel-patches@googlegroups.com, jiayuan.chen@linux.dev,
	 kuniyu@google.com
Subject: Re: Uninitialized Stack Variable / NULL Pointer Dereference in __sys_socket_create via BPF LSM hooks
Date: Tue, 21 Apr 2026 09:59:12 +0000	[thread overview]
Message-ID: <20260421100231.1834988-1-kuniyu@google.com> (raw)
In-Reply-To: <aedAqdeuVqLq4hDQ@google.com>

From: Matt Bobrowski <mattbobrowski@google.com>
Date: Tue, 21 Apr 2026 09:17:29 +0000
> On Tue, Apr 21, 2026 at 08:47:31AM +0000, Kuniyuki Iwashima wrote:
> > From: Quan Sun <2022090917019@std.uestc.edu.cn>
> > Date: Tue, 21 Apr 2026 14:32:35 +0800
> > > Our fuzzing found an Uninitialized Stack Variable vulnerability in the 
> > > Linux Socket Subsystem. The issue is triggered when a 
> > > `BPF_PROG_TYPE_LSM` attached to the `bpf_lsm_socket_create` cgroup hook 
> > > uses the `bpf_set_retval()` helper to return a value greater than 0 
> > > (e.g., `1`). This bypasses the actual `socket_create` execution but 
> > > returns a success status back to `__sys_socket_create`, which then 
> > > accesses the uninitialized stack variable `sock` leading to a NULL 
> > > pointer dereference or potential privilege escalation.
> > > 
> > > Reported-by: Quan Sun <2022090917019@std.uestc.edu.cn>
> > > Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
> > > Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
> > > Reviewed-by: Dongliang Mu <dzm91@hust.edu.cn>
> > > 
> > > ## Root Cause
> > > 
> > > This vulnerability is caused by treating a manipulated return value from 
> > > an LSM hook as complete success in `__sys_socket_create` without 
> > > ensuring that the underlying object has been fully initialized.
> > > 
> > > 1. `__sys_socket_create()` in `net/socket.c` reserves a pointer variable 
> > > `struct socket *sock;` on the kernel stack without zero-initializing it.
> > > 2. It calls `sock_create(family, type, protocol, &sock);` to allocate 
> > > and initialize the socket object.
> > > 3. `sock_create` proceeds to allocate the socket internally and invokes 
> > > the `security_socket_create` LSM hook (which triggers 
> > > `bpf_lsm_socket_create`).
> > > 4. If a BPF program is attached to the cgroup LSM hook for 
> > > `bpf_lsm_socket_create`, it can call `bpf_set_retval(1)` and return `1`.
> > > 5. Because the BPF hook completes without an error code (< 0), 
> > > `sock_create` interprets this bypass as success and promptly returns `1` 
> > > (or a positive bypass value) back to `__sys_socket_create()`. However, 
> > > the critical variable `sock` is never populated.
> > > 6. The caller `__sys_socket_create()` checks if `sock_create()`'s return 
> > > value is `< 0`. Since `1` is not less than `0`, it assumes the `sock` 
> > > pointer is valid.
> > > 7. Subsequent socket operations inside `__sys_socket_create()`, such as 
> > > `sock_map_fd(sock, ...)`, attempt to dereference the uninitialized 
> > > `sock` stack pointer.
> > > 
> > > #### Execution Flow Visualization
> > > 
> > > ```text
> > > Vulnerability Execution Flow
> > > |
> > > |--- 1. `__sys_socket_create(...)`
> > > |    |\
> > > |    | `-- struct socket *sock; (Uninitialized pointer on stack)
> > > |    |
> > > |--- 2. `sock_create(...)` -> `security_socket_create(...)`
> > > |    |\
> > > |    | `-- BPF LSM CGROUP hook for `bpf_lsm_socket_create` triggered.
> > > |    |     |
> > > |    |     `-- bpf_set_retval(1); return 1;
> > > |    |
> > > |--- 3. Context switches back to `sock_create`
> > > |    |\
> > > |    | `-- LSM hook returns `1`. `sock_create` skips allocation and 
> > > returns `1`.
> > > |    |
> > > |--- 4. Context switches back to `__sys_socket_create`
> > > |    |\
> > > |    | `-- Check return value (1 < 0 is False). Treated as SUCCESS.
> > > |    |     |
> > > |    |     `-- `sock_map_fd(sock, ...)`
> > > |    |         |
> > > |    |         `-- Dereferences `sock` leading to KERNEL PANIC or Hijack.
> > > ```
> > > 
> > > ## Reproduction Steps
> > > 
> > > 1. Load a `BPF_PROG_TYPE_LSM` BPF program that:
> > >     - Sets the target BTF ID to `bpf_lsm_socket_create`.
> > >     - Calls the `bpf_set_retval(1)` helper inside the hook.
> > > 2. Attach the loaded program to a chosen cgroup directory using 
> > > `BPF_LSM_CGROUP`.
> > > 3. Add the current process to the targeted cgroup.
> > > 4. Trigger the creation of a socket by invoking the `socket(AF_INET, 
> > > SOCK_STREAM, 0)` system call from within the cgroup.
> > > 5. The `sock_create` function skips allocation, returns 1, and 
> > > `__sys_socket_create` dereferences the uninitialized stack pointer.
> > 
> > Thanks for the report.
> > 
> > Maybe we could fix like below.
> > I guess all (most?) places suppose LSM to return 0 or
> > a negative value, and the btf_id_set_contains() check
> > would be unnecessary ?
> > 
> > ---8<---
> > diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
> > index c5c925f00202..c4b9e9d4de92 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)
> > +
> > +
> >  #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;
> > +
> > +	ctx = container_of(current->bpf_ctx, struct bpf_cg_run_ctx, run_ctx);
> > +	ctx->retval = retval;
> > +
> > +	return 0;
> > +}
> > +
> > +const struct bpf_func_proto bpf_lsm_set_retval_proto = {
> > +	.func		= bpf_lsm_set_retval,
> > +	.gpl_only	= false,
> > +	.ret_type	= RET_INTEGER,
> > +	.arg1_type	= ARG_ANYTHING,
> > +};
> > +
> >  static const struct bpf_func_proto *
> >  bpf_lsm_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> >  {
> >  	const struct bpf_func_proto *func_proto;
> >  
> >  	if (prog->expected_attach_type == BPF_LSM_CGROUP) {
> > +		if (func_id == BPF_FUNC_set_retval &&
> > +		    btf_id_set_contains(&bpf_lsm_negative_set_retval_hooks,
> > +					prog->aux->attach_btf_id))
> > +			return &bpf_lsm_set_retval_proto;
> > +
> >  		func_proto = cgroup_common_func_proto(func_id, prog);
> >  		if (func_proto)
> >  			return func_proto;
> 
> Sorry, I missed this response. I was probably typing out my response
> (https://lore.kernel.org/bpf/aec6Q1g2K8vP_jck@google.com/) whilst this
> had already come through.
> 
> I guess this is kinda on the right path and how I'd probably go about
> fixing part of the underlying issue. My slight concern/reservation is
> that it's highly BPF LSM specific as it's focused only on preventing
> the BPF LSM from violating the expected semantics of the LSM hook.
> 
> The other immediate issue here which I think also needs to be
> addressed is that __sys_socket_create() operates on an uninitialized
> pointer and assumes that any non-negative return value from
> sock_create() means that the pointer is valid. I'd probably also
> propose making __sys_socket_create() a little more fail-safe by
> initializing sock to NULL and explicitly checking both the return code
> and the poitner validity post calling into sock_create(). This makes
> the caller robust against any failure of sock_create() to fulfill its
> implied contract (initializing the sock pointer on success),
> regardless of whether that failure was caused by BPF, some other LSM,
> or a bug in a protocols family's ->create() function.
> 
> What do you think?

I think the issue here is that __sock_create() handles
a positive retval as error only for two LSM hooks,
security_socket_create() and security_socket_post_create(),
which is different from __sys_socket_create()'s expectation.

sock_create()(_kern()) callers are supposed to check the retval
first before accessing the passed &sock pointer, so I don't
think leaving it uninitialised itself is a problem and I'd
rather ignore >0 value as allowed.

And I found a good place in verifier to reject the repro.

---8<---
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 9e4980128151..19cc4ed8c389 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -10448,6 +10448,11 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 				verbose(env, "BPF_LSM_CGROUP that attach to void LSM hooks can't modify return value!\n");
 				return -EINVAL;
 			}
+
+			if (regs[BPF_REG_1].s32_max_value > 0) {
+				verbose(env, "BPF_LSM_CGROUP can't return a positive value!\n");
+				return -EINVAL;
+			}
 		}
 		break;
 	case BPF_FUNC_dynptr_data:
diff --git a/net/socket.c b/net/socket.c
index 22a412fdec07..d40aea527f66 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1617,7 +1617,7 @@ int __sock_create(struct net *net, int family, int type, int protocol,
 	}
 
 	err = security_socket_create(family, type, protocol, kern);
-	if (err)
+	if (err < 0)
 		return err;
 
 	/*
@@ -1685,7 +1685,7 @@ int __sock_create(struct net *net, int family, int type, int protocol,
 	 */
 	module_put(pf->owner);
 	err = security_socket_post_create(sock, family, type, protocol, kern);
-	if (err)
+	if (err < 0)
 		goto out_sock_release;
 	*res = sock;
 
---8<---


  reply	other threads:[~2026-04-21 10:02 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-21  6:32 Uninitialized Stack Variable / NULL Pointer Dereference in __sys_socket_create via BPF LSM hooks Quan Sun
2026-04-21  8:47 ` Kuniyuki Iwashima
2026-04-21  9:17   ` Matt Bobrowski
2026-04-21  9:59     ` Kuniyuki Iwashima [this message]
2026-04-21 13:05       ` Xu Kuohai
2026-04-21 20:20         ` Matt Bobrowski
2026-04-21  8:50 ` Matt Bobrowski
2026-04-21  9:11 ` Jiayuan Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260421100231.1834988-1-kuniyu@google.com \
    --to=kuniyu@google.com \
    --cc=2022090917019@std.uestc.edu.cn \
    --cc=M202472210@hust.edu.cn \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=dddddd@hust.edu.cn \
    --cc=dzm91@hust.edu.cn \
    --cc=edumazet@google.com \
    --cc=hust-os-kernel-patches@googlegroups.com \
    --cc=jiayuan.chen@linux.dev \
    --cc=mattbobrowski@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox