All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 6.12.y] tcp: clear sock_ops cb flags before force-closing a child socket
@ 2026-09-07 17:02 Miguel Gazquez (Schneider Electric)
  2026-09-07 17:14 ` sashiko-bot
  2026-09-08  0:53 ` Sasha Levin
  0 siblings, 2 replies; 3+ messages in thread
From: Miguel Gazquez (Schneider Electric) @ 2026-09-07 17:02 UTC (permalink / raw)
  To: stable, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, David Ahern, Alexei Starovoitov,
	Lawrence Brakmo
  Cc: Thomas Petazzoni, netdev, linux-kernel, bpf, Sechang Lim,
	Jiayuan Chen, Kuniyuki Iwashima, Sasha Levin,
	Miguel Gazquez (Schneider Electric)

From: Sechang Lim <rhkrqnwk98@gmail.com>

[ Upstream commit 990348e5bb457697c2f1f7f7b65154a3334d9d2b ]

A child socket inherits the listener's bpf_sock_ops_cb_flags via
sk_clone_lock(). If its setup fails in tcp_v4_syn_recv_sock() /
tcp_v6_syn_recv_sock(), the child is freed through put_and_exit, where
inet_csk_prepare_forced_close() drops the socket lock and tcp_done() runs
without it.

If BPF_SOCK_OPS_STATE_CB_FLAG was inherited, tcp_done() -> tcp_set_state()
calls tcp_call_bpf(), which expects the lock and trips sock_owned_by_me():

  WARNING: include/net/sock.h:1799 at tcp_set_state+0x433/0x550
  RIP: 0010:tcp_set_state+0x433/0x550 include/net/sock.h:1799
  Call Trace:
   <IRQ>
   tcp_done+0xba/0x250 net/ipv4/tcp.c:5095
   tcp_v4_syn_recv_sock+0x850/0xa50 net/ipv4/tcp_ipv4.c:1787
   tcp_check_req+0xf30/0x1360 net/ipv4/tcp_minisocks.c:926
   tcp_v4_rcv+0x1047/0x1b50 net/ipv4/tcp_ipv4.c:2164
   </IRQ>

The child is freed before it is ever established, so it should run no
sock_ops callback. Clear its cb flags in inet_csk_prepare_for_destroy_sock(),
the common point for the IPv4, IPv6 and chtls forced-close paths and for the
MPTCP ->syn_recv_sock() failure path (dispose_child), which reaches tcp_done()
on a child that was never established too.

Suggested-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Fixes: d44874910a26 ("bpf: Add BPF_SOCK_OPS_STATE_CB")
Signed-off-by: Sechang Lim <rhkrqnwk98@gmail.com>
Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Link: https://patch.msgid.link/20260611092923.1895982-1-rhkrqnwk98@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
[ Moved inet_csk_prepare_for_destroy_sock() to inet_connection_sock.c due to header dependency ]
Signed-off-by: Miguel Gazquez (Schneider Electric) <miguel.gazquez@bootlin.com>
---
 include/net/inet_connection_sock.h | 8 +-------
 include/net/tcp.h                  | 9 +++++++++
 net/ipv4/inet_connection_sock.c    | 8 ++++++++
 3 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
index bcc138ff087b..534f4fafad42 100644
--- a/include/net/inet_connection_sock.h
+++ b/include/net/inet_connection_sock.h
@@ -296,14 +296,8 @@ reqsk_timeout(struct request_sock *req, unsigned long max_timeout)
 	return (unsigned long)min_t(u64, timeout, max_timeout);
 }
 
-static inline void inet_csk_prepare_for_destroy_sock(struct sock *sk)
-{
-	/* The below has to be done to allow calling inet_csk_destroy_sock */
-	sock_set_flag(sk, SOCK_DEAD);
-	this_cpu_inc(*sk->sk_prot->orphan_count);
-}
-
 void inet_csk_destroy_sock(struct sock *sk);
+void inet_csk_prepare_for_destroy_sock(struct sock *sk);
 void inet_csk_prepare_forced_close(struct sock *sk);
 
 /*
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 1cc25e0b3bde..c472c1cdd3b8 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -2702,6 +2702,11 @@ static inline int tcp_call_bpf_3arg(struct sock *sk, int op, u32 arg1, u32 arg2,
 	return tcp_call_bpf(sk, op, 3, args);
 }
 
+static inline void tcp_clear_sock_ops_cb_flags(struct sock *sk)
+{
+	tcp_sk(sk)->bpf_sock_ops_cb_flags = 0;
+}
+
 #else
 static inline int tcp_call_bpf(struct sock *sk, int op, u32 nargs, u32 *args)
 {
@@ -2719,6 +2724,10 @@ static inline int tcp_call_bpf_3arg(struct sock *sk, int op, u32 arg1, u32 arg2,
 	return -EPERM;
 }
 
+static inline void tcp_clear_sock_ops_cb_flags(struct sock *sk)
+{
+}
+
 #endif
 
 static inline u32 tcp_timeout_init(struct sock *sk)
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 91dcdb7567ac..e0bd8e76fa78 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -1315,6 +1315,14 @@ void inet_csk_destroy_sock(struct sock *sk)
 }
 EXPORT_SYMBOL(inet_csk_destroy_sock);
 
+void inet_csk_prepare_for_destroy_sock(struct sock *sk)
+{
+	/* The below has to be done to allow calling inet_csk_destroy_sock */
+	tcp_clear_sock_ops_cb_flags(sk);
+	sock_set_flag(sk, SOCK_DEAD);
+	this_cpu_inc(*sk->sk_prot->orphan_count);
+}
+
 /* This function allows to force a closure of a socket after the call to
  * tcp/dccp_create_openreq_child().
  */

---
base-commit: 064531c7e30cd67b79c0c694ac97f00c7796f4d6
change-id: 20260907-cve-2026-74268-1e7715cc3ece

Best regards,
--  
Miguel Gazquez (Schneider Electric) <miguel.gazquez@bootlin.com>


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 6.12.y] tcp: clear sock_ops cb flags before force-closing a child socket
  2026-09-07 17:02 [PATCH 6.12.y] tcp: clear sock_ops cb flags before force-closing a child socket Miguel Gazquez (Schneider Electric)
@ 2026-09-07 17:14 ` sashiko-bot
  2026-09-08  0:53 ` Sasha Levin
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-09-07 17:14 UTC (permalink / raw)
  To: Miguel Gazquez (Schneider Electric); +Cc: bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Calling tcp_clear_sock_ops_cb_flags() unconditionally on non-TCP sockets (like DCCP) causes an out-of-bounds write due to an invalid struct tcp_sock cast.
--

commit a1e4666cc30271c086f6d9e64c6ebfaddd6750ca
Author: Sechang Lim <rhkrqnwk98@gmail.com>

tcp: clear sock_ops cb flags before force-closing a child socket

A child socket inherits the listener's bpf_sock_ops_cb_flags via
sk_clone_lock(). If its setup fails in tcp_v4_syn_recv_sock() /
tcp_v6_syn_recv_sock(), the child is freed through put_and_exit, where
inet_csk_prepare_forced_close() drops the socket lock and tcp_done() runs
without it.

Link: https://patch.msgid.link/20260611092923.1895982-1-rhkrqnwk98@gmail.com

> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 1cc25e0b3bdea..c472c1cdd3b83 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
[ ... ]
> @@ -2702,6 +2702,11 @@ static inline int tcp_call_bpf_3arg(struct sock *sk, int op, u32 arg1, u32 arg2,
>  	return tcp_call_bpf(sk, op, 3, args);
>  }
>  
> +static inline void tcp_clear_sock_ops_cb_flags(struct sock *sk)
> +{
> +	tcp_sk(sk)->bpf_sock_ops_cb_flags = 0;

[Severity: Critical]
Does this code perform an unsafe cast and memory write for non-TCP sockets?

This accesses an offset that exceeds the bounds of a dccp_sock when
invoked on DCCP sockets, which writes outside the slab allocation for the
socket structure.

> +}
> +
[ ... ]
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 91dcdb7567ace..e0bd8e76fa781 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
[ ... ]
> @@ -1315,6 +1315,14 @@ void inet_csk_destroy_sock(struct sock *sk)
>  }
>  EXPORT_SYMBOL(inet_csk_destroy_sock);
>  
> +void inet_csk_prepare_for_destroy_sock(struct sock *sk)
> +{
> +	/* The below has to be done to allow calling inet_csk_destroy_sock */
> +	tcp_clear_sock_ops_cb_flags(sk);

[Severity: Critical]
Can calling this unconditionally on non-TCP sockets cause memory regressions?

This generic connection socket destruction path in
inet_csk_prepare_for_destroy_sock() blindly invokes a TCP-specific helper,
passing it sockets (like DCCP) that are incompatible with the TCP socket
structure.

This can be triggered by a remote, unauthenticated attacker sending a
malicious or unroutable DCCP connection request to a listening DCCP port.

The regression is triggered on the failure path of DCCP connection setup,
for example when routing fails:

net/dccp/ipv4.c:dccp_v4_request_recv_sock()
   ...
   put_and_exit:
        ...
        inet_csk_prepare_forced_close(newsk);
            inet_csk_prepare_for_destroy_sock(newsk);
                tcp_clear_sock_ops_cb_flags(newsk); /* out-of-bounds write */

> +	sock_set_flag(sk, SOCK_DEAD);
> +	this_cpu_inc(*sk->sk_prot->orphan_count);
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-cve-2026-74268-v1-1-cacea38d7fb3@bootlin.com?part=1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 6.12.y] tcp: clear sock_ops cb flags before force-closing a child socket
  2026-09-07 17:02 [PATCH 6.12.y] tcp: clear sock_ops cb flags before force-closing a child socket Miguel Gazquez (Schneider Electric)
  2026-09-07 17:14 ` sashiko-bot
@ 2026-09-08  0:53 ` Sasha Levin
  1 sibling, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2026-09-08  0:53 UTC (permalink / raw)
  To: stable, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, David Ahern, Alexei Starovoitov,
	Lawrence Brakmo
  Cc: Sasha Levin, Thomas Petazzoni, netdev, linux-kernel, bpf,
	Sechang Lim, Jiayuan Chen, Kuniyuki Iwashima,
	Miguel Gazquez (Schneider Electric)

> [ Upstream commit 990348e5bb457697c2f1f7f7b65154a3334d9d2b ]
>
> A child socket inherits the listener's bpf_sock_ops_cb_flags via
> sk_clone_lock(). If its setup fails in tcp_v4_syn_recv_sock() /
> tcp_v6_syn_recv_sock(), the child is freed through put_and_exit, where
> inet_csk_prepare_forced_close() drops the socket lock and tcp_done()
> runs without it.

6.12.y still ships DCCP. Its put_and_exit path (dccp_v4_request_recv_sock() ->
inet_csk_prepare_forced_close() -> inet_csk_prepare_for_destroy_sock()) reaches
the same function you relocated, which now unconditionally casts sk to struct
tcp_sock and writes through it. On a DCCP child that's a write past the end of
the smaller struct dccp_sock. Upstream is only safe here because DCCP was
retired by 2a63dd0edf38 ("net: Retire DCCP socket."), about a year before this
fix landed.

Could you send a v2 that guards the call, e.g. only when sk->sk_protocol ==
IPPROTO_TCP, or keep tcp_clear_sock_ops_cb_flags() out of the generic inet_csk
teardown path?

-- 
Thanks,
Sasha

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-08  0:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-07 17:02 [PATCH 6.12.y] tcp: clear sock_ops cb flags before force-closing a child socket Miguel Gazquez (Schneider Electric)
2026-09-07 17:14 ` sashiko-bot
2026-09-08  0:53 ` Sasha Levin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.