[PATCH bpf] bpf: reject mini-sockets in bpf_sock_destroy()

[PATCH bpf] bpf: reject mini-sockets in bpf_sock_destroy()

[Xiang Mei (Microsoft)]

bpf_sock_destroy() casts its struct sock_common * argument to a full
struct sock and reads sk->sk_protocol. The BPF tcp iterator can pass a
TIME_WAIT or NEW_SYN_RECV mini-socket, which only embeds a sock_common
prefix. Unlike sk_prot (which aliases skc_prot inside sock_common),
sk_protocol lives beyond that prefix, so the read goes out of bounds of
the small tw_sock_TCP object (type confusion).

Reject non-full sockets with sk_fullsock() before touching any
full-sock field. sk_fullsock() only reads sk_state (in sock_common),
and these mini-sockets have no ->diag_destroy(), This matches the other
sock_common consumers in this file, e.g. bpf_skc_to_tcp_sock(), which 
already sk_fullsock() before reading sk_protocol.

  BUG: KASAN: slab-out-of-bounds in bpf_sock_destroy (net/core/filter.c:12673)
  Read of size 2 at addr ffff888013ffc71c by task exploit/143
  Call Trace:
   kasan_report (mm/kasan/report.c:595)
   bpf_sock_destroy (net/core/filter.c:12673)
   bpf_prog_8b5bd55c189cabc9_sock_destroy_tw+0x31/0x3e
   bpf_iter_run_prog (kernel/bpf/bpf_iter.c:697)
   bpf_iter_tcp_seq_show (net/ipv4/tcp_ipv4.c:3247)
   bpf_seq_read (kernel/bpf/bpf_iter.c:184)
   vfs_read (fs/read_write.c:572)
   ksys_read (fs/read_write.c:716)
   do_syscall_64 (arch/x86/entry/syscall_64.c:94)
   entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
  ...
  The buggy address belongs to the object at ffff888013ffc640
   which belongs to the cache tw_sock_TCP of size 256
  The buggy address is located 220 bytes inside of
   allocated 256-byte region [ffff888013ffc640, ffff888013ffc740)

Fixes: 4ddbcb886268 ("bpf: Add bpf_sock_destroy kfunc")
Reported-by: AutonomousCodeSecurity@microsoft.com
Signed-off-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
---
 net/core/filter.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index 69ec1a4c0f9d..a0fcafb08b96 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -12665,6 +12665,9 @@ __bpf_kfunc int bpf_sock_destroy(struct sock_common *sock)
 {
 	struct sock *sk = (struct sock *)sock;
 
+	if (!sk_fullsock(sk))
+		return -EOPNOTSUPP;
+
 	/* The locking semantics that allow for synchronous execution of the
 	 * destroy handlers are only supported for TCP and UDP.
 	 * Supporting protocols will need to acquire sock lock in the BPF context
-- 
2.43.0

Re: [PATCH bpf] bpf: reject mini-sockets in bpf_sock_destroy()

[sashiko-bot]

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

New issues:
- [High] The patch causes a functional regression by completely disabling bpf_sock_destroy() for TCP mini-sockets (TIME_WAIT and NEW_SYN_RECV).

Pre-existing issues:
- [High] The helpers bpf_tcp_check_syncookie and bpf_tcp_gen_syncookie suffer from the same type confusion and out-of-bounds read as bpf_sock_destroy, but remain unfixed.
--

commit cf46da8728e7f1fca4584ccdb089c82b7fd3061f
Author: Xiang Mei (Microsoft) <xmei5@asu.edu>

bpf: reject mini-sockets in bpf_sock_destroy()

The patch tries to fix a KASAN slab-out-of-bounds read in
bpf_sock_destroy() by checking sk_fullsock(sk) before accessing
sk->sk_protocol. It casts sock_common to sock and early exits if it is
a mini-socket.

Fixes: 4ddbcb886268 ("bpf: Add bpf_sock_destroy kfunc")
Reported-by: AutonomousCodeSecurity@microsoft.com
Signed-off-by: Xiang Mei (Microsoft) <xmei5@asu.edu>

> diff --git a/net/core/filter.c b/net/core/filter.c
> index b446aa8be5c37..3e70c8ab5aab1 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -12665,6 +12665,9 @@ __bpf_kfunc int bpf_sock_destroy(struct sock_common *sock)
>  {
>  	struct sock *sk = (struct sock *)sock;
>  
> +	if (!sk_fullsock(sk))
> +		return -EOPNOTSUPP;
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
Does this unconditionally reject mini-sockets?

The commit message states that mini-sockets have no diag_destroy() handler,
but as documented in the original commit 4ddbcb886268, request sockets are
explicitly supported because their protocol diag_destroy handler
(tcp_abort) correctly handles TCP_TIME_WAIT and TCP_NEW_SYN_RECV states.

If we unconditionally reject non-full sockets, wouldn't this break BPF
programs that rely on bpf_sock_destroy() to forcefully terminate pending
connections or TIME_WAIT sockets?

[Severity: High]
This is a pre-existing issue, but does this same type confusion exist
in bpf_tcp_check_syncookie() and bpf_tcp_gen_syncookie()?

Both of those helpers take ARG_PTR_TO_BTF_ID_SOCK_COMMON and cast it to
a full struct sock *. They then immediately dereference sk->sk_protocol
without an sk_fullsock() check:

net/core/filter.c:bpf_tcp_check_syncookie() {
    ...
	if (sk->sk_protocol != IPPROTO_TCP || sk->sk_state != TCP_LISTEN)
		return -EINVAL;
    ...
}

If an XDP or TC BPF program passes a tw_sock or req_sock from
bpf_skc_lookup_tcp(), won't this result in the same KASAN
slab-out-of-bounds read since sk_protocol resides outside the bounds
of struct sock_common?

> +
>  	/* The locking semantics that allow for synchronous execution of the
>  	 * destroy handlers are only supported for TCP and UDP.
>  	 * Supporting protocols will need to acquire sock lock in the BPF context

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260702224519.800135-1-xmei5@asu.edu?part=1

Re: [PATCH bpf] bpf: reject mini-sockets in bpf_sock_destroy()

[Jiayuan Chen]

On 7/3/26 7:05 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [High] The patch causes a functional regression by completely disabling bpf_sock_destroy() for TCP mini-sockets (TIME_WAIT and NEW_SYN_RECV).
>
> Pre-existing issues:
> - [High] The helpers bpf_tcp_check_syncookie and bpf_tcp_gen_syncookie suffer from the same type confusion and out-of-bounds read as bpf_sock_destroy, but remain unfixed.
> --
>
> commit cf46da8728e7f1fca4584ccdb089c82b7fd3061f
> Author: Xiang Mei (Microsoft) <xmei5@asu.edu>
>
> bpf: reject mini-sockets in bpf_sock_destroy()
>
> The patch tries to fix a KASAN slab-out-of-bounds read in
> bpf_sock_destroy() by checking sk_fullsock(sk) before accessing
> sk->sk_protocol. It casts sock_common to sock and early exits if it is
> a mini-socket.
>
> Fixes: 4ddbcb886268 ("bpf: Add bpf_sock_destroy kfunc")
> Reported-by: AutonomousCodeSecurity@microsoft.com
> Signed-off-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
>
>> diff --git a/net/core/filter.c b/net/core/filter.c
>> index b446aa8be5c37..3e70c8ab5aab1 100644
>> --- a/net/core/filter.c
>> +++ b/net/core/filter.c
>> @@ -12665,6 +12665,9 @@ __bpf_kfunc int bpf_sock_destroy(struct sock_common *sock)
>>   {
>>   	struct sock *sk = (struct sock *)sock;
>>   
>> +	if (!sk_fullsock(sk))
>> +		return -EOPNOTSUPP;
>          ^^^^^^^^^^^^^^^^^^^^^^^^^^^
> [Severity: High]
> Does this unconditionally reject mini-sockets?
>
> The commit message states that mini-sockets have no diag_destroy() handler,
> but as documented in the original commit 4ddbcb886268, request sockets are
> explicitly supported because their protocol diag_destroy handler
> (tcp_abort) correctly handles TCP_TIME_WAIT and TCP_NEW_SYN_RECV states.
>
> If we unconditionally reject non-full sockets, wouldn't this break BPF
> programs that rely on bpf_sock_destroy() to forcefully terminate pending
> connections or TIME_WAIT sockets?

It's true. Please also include a selftest to reproduce the issue and, 
another one to cover the non-full socket.


>
> [Severity: High]
> This is a pre-existing issue, but does this same type confusion exist
> in bpf_tcp_check_syncookie() and bpf_tcp_gen_syncookie()?
>
> Both of those helpers take ARG_PTR_TO_BTF_ID_SOCK_COMMON and cast it to
> a full struct sock *. They then immediately dereference sk->sk_protocol
> without an sk_fullsock() check:
>
> net/core/filter.c:bpf_tcp_check_syncookie() {
>      ...
> 	if (sk->sk_protocol != IPPROTO_TCP || sk->sk_state != TCP_LISTEN)
> 		return -EINVAL;
>      ...
> }
>
> If an XDP or TC BPF program passes a tw_sock or req_sock from
> bpf_skc_lookup_tcp(), won't this result in the same KASAN
> slab-out-of-bounds read since sk_protocol resides outside the bounds
> of struct sock_common?
>
>> +
>>   	/* The locking semantics that allow for synchronous execution of the
>>   	 * destroy handlers are only supported for TCP and UDP.
>>   	 * Supporting protocols will need to acquire sock lock in the BPF context