BPF List
 help / color / mirror / Atom feed
* [PATCH net v2 0/2] tcp: fix use-after-free in do_tcp_getsockopt()
@ 2026-08-26 17:13 Cen Zhang (Microsoft)
  2026-08-26 17:13 ` [PATCH net v2 1/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CONGESTION) Cen Zhang (Microsoft)
  2026-08-26 17:13 ` [PATCH net v2 2/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CC_INFO) Cen Zhang (Microsoft)
  0 siblings, 2 replies; 8+ messages in thread
From: Cen Zhang (Microsoft) @ 2026-08-26 17:13 UTC (permalink / raw)
  To: edumazet, ncardwell, davem, kuba, pabeni
  Cc: kuniyu, horms, matttbe, martineau, geliang, yhs, kafai, andriin,
	ast, netdev, mptcp, bpf, linux-kernel, AutonomousCodeSecurity,
	xmei5, tgopinath, kys, blbllhy

From: "Cen Zhang (Microsoft Security FORGE Labs)" <blbllhy@gmail.com>

do_tcp_getsockopt() has two lockless reads of icsk_ca_ops. Since BPF
struct_ops congestion control made icsk_ca_ops point to dynamically
allocated memory, a concurrent setsockopt(TCP_CONGESTION) can replace
the pointer and free the old object while either reader is using it.

Patch 1 fixes the TCP_CONGESTION path by copying ca_ops->name to a
stack buffer while holding rcu_read_lock(). It also uses READ_ONCE()
for the lockless load and annotates every explicit icsk_ca_ops store
with WRITE_ONCE().

Patch 2 fixes the TCP_CC_INFO path by keeping the READ_ONCE() load,
ca_ops->get_info lookup, and call inside an RCU read-side critical
section.

Changes since v1 [1]:
- Add READ_ONCE() to both lockless icsk_ca_ops loads.
- Add WRITE_ONCE() to all six explicit icsk_ca_ops stores.
- Include the data-race annotations in the UAF series as requested
  by Eric Dumazet [2].

[1] https://lore.kernel.org/all/20260821182449.79785-1-blbllhy@gmail.com/
[2] https://lore.kernel.org/all/CANn89iKr1ypEFeL+7te3xWoNaK5AFyYQmr+pDh2zSZAxfgNKjw@mail.gmail.com/

Cen Zhang (Microsoft Security FORGE Labs) (2):
  tcp: fix use-after-free in do_tcp_getsockopt(TCP_CONGESTION)
  tcp: fix use-after-free in do_tcp_getsockopt(TCP_CC_INFO)

 net/ipv4/tcp.c           | 18 ++++++++++++++----
 net/ipv4/tcp_cong.c      |  4 ++--
 net/ipv4/tcp_dctcp.c     |  2 +-
 net/ipv4/tcp_minisocks.c |  2 +-
 net/ipv4/tcp_output.c    |  2 +-
 net/mptcp/protocol.c     |  2 +-
 6 files changed, 20 insertions(+), 10 deletions(-)


base-commit: f967455fb2a5a2079b9eb5823e9ccf359174bf9f
-- 
2.55.0

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

* [PATCH net v2 1/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CONGESTION)
  2026-08-26 17:13 [PATCH net v2 0/2] tcp: fix use-after-free in do_tcp_getsockopt() Cen Zhang (Microsoft)
@ 2026-08-26 17:13 ` Cen Zhang (Microsoft)
  2026-08-26 17:30   ` Eric Dumazet
                     ` (2 more replies)
  2026-08-26 17:13 ` [PATCH net v2 2/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CC_INFO) Cen Zhang (Microsoft)
  1 sibling, 3 replies; 8+ messages in thread
From: Cen Zhang (Microsoft) @ 2026-08-26 17:13 UTC (permalink / raw)
  To: edumazet, ncardwell, davem, kuba, pabeni
  Cc: kuniyu, horms, matttbe, martineau, geliang, yhs, kafai, andriin,
	ast, netdev, mptcp, bpf, linux-kernel, AutonomousCodeSecurity,
	xmei5, tgopinath, kys, blbllhy, stable

From: "Cen Zhang (Microsoft Security FORGE Labs)" <blbllhy@gmail.com>

do_tcp_getsockopt() reads icsk->icsk_ca_ops->name without holding
rcu_read_lock(). Since commit 0baf26b0fcd7 ("bpf: tcp: Support
tcp_congestion_ops in bpf"), icsk_ca_ops can point to dynamically
allocated BPF struct_ops memory that may be freed concurrently via
setsockopt(TCP_CONGESTION), leading to a use-after-free.

  BUG: KASAN: slab-use-after-free in _copy_to_user+0x37/0x60
  Read of size 16 at addr ffff888013505260 by task exploit/149
   _copy_to_user+0x37/0x60
   do_tcp_getsockopt+0x158a/0x2460 (net/ipv4/tcp.c:4585)
   tcp_getsockopt+0x91/0xf0
   __sys_getsockopt+0xf7/0x170

Fix this by holding rcu_read_lock() around the ca_ops->name access,
using READ_ONCE() to load icsk_ca_ops, and copying the name to a
stack buffer before releasing the lock. Also annotate every explicit
icsk_ca_ops store with WRITE_ONCE() to fix the accompanying KCSAN
data-race issue.

Fixes: 0baf26b0fcd7 ("bpf: tcp: Support tcp_congestion_ops in bpf")
Suggested-by: Eric Dumazet <edumazet@google.com>
Reported-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
Link: https://lore.kernel.org/all/20260821182449.79785-2-blbllhy@gmail.com/
Cc: AutonomousCodeSecurity@microsoft.com
Cc: stable@vger.kernel.org
Assisted-by: GitHub-Copilot:claude-opus-4.6
Signed-off-by: Cen Zhang (Microsoft Security FORGE Labs) <blbllhy@gmail.com>
---
Note: The adjacent TCP_ULP case has a similar read pattern on
icsk_ulp_ops->name, but icsk_ulp_ops always points to static
module data (no BPF struct_ops equivalent), so it is not affected.

Changes in v2:
- Add READ_ONCE() for the icsk_ca_ops load.
- Add WRITE_ONCE() for every explicit icsk_ca_ops store.
---
 net/ipv4/tcp.c           | 14 +++++++++++---
 net/ipv4/tcp_cong.c      |  4 ++--
 net/ipv4/tcp_dctcp.c     |  2 +-
 net/ipv4/tcp_minisocks.c |  2 +-
 net/ipv4/tcp_output.c    |  2 +-
 net/mptcp/protocol.c     |  2 +-
 6 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index b4237d0e994d..4918f48ba76d 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -4577,16 +4577,24 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 		val = !inet_csk_in_pingpong_mode(sk);
 		break;
 
-	case TCP_CONGESTION:
+	case TCP_CONGESTION: {
+		char ca_name[TCP_CA_NAME_MAX] = {};
+
 		if (copy_from_sockptr(&len, optlen, sizeof(int)))
 			return -EFAULT;
 		len = min_t(unsigned int, len, TCP_CA_NAME_MAX);
 		if (copy_to_sockptr(optlen, &len, sizeof(int)))
 			return -EFAULT;
-		if (copy_to_sockptr(optval, icsk->icsk_ca_ops->name, len))
+
+		rcu_read_lock();
+		ca_ops = READ_ONCE(icsk->icsk_ca_ops);
+		memcpy(ca_name, ca_ops->name, sizeof(ca_name));
+		rcu_read_unlock();
+
+		if (copy_to_sockptr(optval, ca_name, len))
 			return -EFAULT;
 		return 0;
-
+	}
 	case TCP_ULP:
 		if (copy_from_sockptr(&len, optlen, sizeof(int)))
 			return -EFAULT;
diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
index e9f6c77e0631..8e83ef81fc18 100644
--- a/net/ipv4/tcp_cong.c
+++ b/net/ipv4/tcp_cong.c
@@ -223,7 +223,7 @@ void tcp_assign_congestion_control(struct sock *sk)
 	ca = rcu_dereference(net->ipv4.tcp_congestion_control);
 	if (unlikely(!bpf_try_module_get(ca, ca->owner)))
 		ca = &tcp_reno;
-	icsk->icsk_ca_ops = ca;
+	WRITE_ONCE(icsk->icsk_ca_ops, ca);
 	rcu_read_unlock();
 
 	memset(icsk->icsk_ca_priv, 0, sizeof(icsk->icsk_ca_priv));
@@ -253,7 +253,7 @@ static void tcp_reinit_congestion_control(struct sock *sk,
 	struct inet_connection_sock *icsk = inet_csk(sk);
 
 	tcp_cleanup_congestion_control(sk);
-	icsk->icsk_ca_ops = ca;
+	WRITE_ONCE(icsk->icsk_ca_ops, ca);
 	icsk->icsk_ca_setsockopt = 1;
 	memset(icsk->icsk_ca_priv, 0, sizeof(icsk->icsk_ca_priv));
 
diff --git a/net/ipv4/tcp_dctcp.c b/net/ipv4/tcp_dctcp.c
index 274e628e7cf8..99f68c2992d0 100644
--- a/net/ipv4/tcp_dctcp.c
+++ b/net/ipv4/tcp_dctcp.c
@@ -111,7 +111,7 @@ __bpf_kfunc static void dctcp_init(struct sock *sk)
 	/* No ECN support? Fall back to Reno. Also need to clear
 	 * ECT from sk since it is set during 3WHS for DCTCP.
 	 */
-	inet_csk(sk)->icsk_ca_ops = &dctcp_reno;
+	WRITE_ONCE(inet_csk(sk)->icsk_ca_ops, &dctcp_reno);
 	INET_ECN_dontxmit(sk);
 }
 
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index f3fa0b18eda0..0ddfd5af6e58 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -507,7 +507,7 @@ void tcp_ca_openreq_child(struct sock *sk, const struct dst_entry *dst)
 		ca = tcp_ca_find_key(ca_key);
 		if (likely(ca && bpf_try_module_get(ca, ca->owner))) {
 			icsk->icsk_ca_dst_locked = tcp_ca_dst_locked(dst);
-			icsk->icsk_ca_ops = ca;
+			WRITE_ONCE(icsk->icsk_ca_ops, ca);
 			ca_got_dst = true;
 		}
 		rcu_read_unlock();
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f2709d585edb..74727b7eb446 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -4092,7 +4092,7 @@ static void tcp_ca_dst_init(struct sock *sk, const struct dst_entry *dst)
 	if (likely(ca && bpf_try_module_get(ca, ca->owner))) {
 		bpf_module_put(icsk->icsk_ca_ops, icsk->icsk_ca_ops->owner);
 		icsk->icsk_ca_dst_locked = tcp_ca_dst_locked(dst);
-		icsk->icsk_ca_ops = ca;
+		WRITE_ONCE(icsk->icsk_ca_ops, ca);
 	}
 	rcu_read_unlock();
 }
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index b474d03620a7..006de39e8165 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3218,7 +3218,7 @@ static void mptcp_ca_reset(struct sock *sk)
 
 	/* no need to keep a reference to the ops, the name will suffice */
 	tcp_cleanup_congestion_control(sk);
-	icsk->icsk_ca_ops = NULL;
+	WRITE_ONCE(icsk->icsk_ca_ops, NULL);
 }
 
 static int mptcp_init_sock(struct sock *sk)
-- 
2.55.0

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

* [PATCH net v2 2/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CC_INFO)
  2026-08-26 17:13 [PATCH net v2 0/2] tcp: fix use-after-free in do_tcp_getsockopt() Cen Zhang (Microsoft)
  2026-08-26 17:13 ` [PATCH net v2 1/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CONGESTION) Cen Zhang (Microsoft)
@ 2026-08-26 17:13 ` Cen Zhang (Microsoft)
  2026-08-26 17:31   ` Eric Dumazet
  1 sibling, 1 reply; 8+ messages in thread
From: Cen Zhang (Microsoft) @ 2026-08-26 17:13 UTC (permalink / raw)
  To: edumazet, ncardwell, davem, kuba, pabeni
  Cc: kuniyu, horms, matttbe, martineau, geliang, yhs, kafai, andriin,
	ast, netdev, mptcp, bpf, linux-kernel, AutonomousCodeSecurity,
	xmei5, tgopinath, kys, blbllhy, stable

From: "Cen Zhang (Microsoft Security FORGE Labs)" <blbllhy@gmail.com>

do_tcp_getsockopt() reads icsk->icsk_ca_ops and dereferences the
get_info function pointer without rcu_read_lock(). With BPF struct_ops
congestion control, ca_ops can point to dynamically allocated memory
that is freed concurrently, resulting in a use-after-free when the
kernel dereferences or calls through the stale pointer.

  BUG: KASAN: slab-use-after-free in do_tcp_getsockopt+0x2037/0x23e0
  Read of size 8 at addr ffff888013701258 by task exploit/149
   do_tcp_getsockopt+0x2037/0x23e0 (net/ipv4/tcp.c:4564)
   tcp_getsockopt+0x91/0xf0
   __sys_getsockopt+0xf7/0x170

Fix this by wrapping the ca_ops load and get_info call within
rcu_read_lock()/rcu_read_unlock(), and using READ_ONCE() to load
the icsk_ca_ops pointer.

Fixes: 0baf26b0fcd7 ("bpf: tcp: Support tcp_congestion_ops in bpf")
Suggested-by: Eric Dumazet <edumazet@google.com>
Cc: AutonomousCodeSecurity@microsoft.com
Cc: stable@vger.kernel.org
Assisted-by: GitHub-Copilot:claude-opus-4.6
Signed-off-by: Cen Zhang (Microsoft Security FORGE Labs) <blbllhy@gmail.com>
---
Link: https://lore.kernel.org/all/20260821182449.79785-3-blbllhy@gmail.com/ (v1)
Changes in v2:
- Add READ_ONCE() for the icsk_ca_ops load.
---
 net/ipv4/tcp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 4918f48ba76d..c45776d3ed90 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -4562,9 +4562,11 @@ int do_tcp_getsockopt(struct sock *sk, int level,
 		if (copy_from_sockptr(&len, optlen, sizeof(int)))
 			return -EFAULT;
 
-		ca_ops = icsk->icsk_ca_ops;
+		rcu_read_lock();
+		ca_ops = READ_ONCE(icsk->icsk_ca_ops);
 		if (ca_ops && ca_ops->get_info)
 			sz = ca_ops->get_info(sk, ~0U, &attr, &info);
+		rcu_read_unlock();
 
 		len = min_t(unsigned int, len, sz);
 		if (copy_to_sockptr(optlen, &len, sizeof(int)))
-- 
2.55.0

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

* Re: [PATCH net v2 1/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CONGESTION)
  2026-08-26 17:13 ` [PATCH net v2 1/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CONGESTION) Cen Zhang (Microsoft)
@ 2026-08-26 17:30   ` Eric Dumazet
  2026-08-27 10:38   ` Matthieu Baerts
  2026-08-27 17:14   ` sashiko-bot
  2 siblings, 0 replies; 8+ messages in thread
From: Eric Dumazet @ 2026-08-26 17:30 UTC (permalink / raw)
  To: Cen Zhang (Microsoft)
  Cc: ncardwell, davem, kuba, pabeni, kuniyu, horms, matttbe, martineau,
	geliang, yhs, kafai, andriin, ast, netdev, mptcp, bpf,
	linux-kernel, AutonomousCodeSecurity, xmei5, tgopinath, kys,
	stable

On Wed, Aug 26, 2026 at 7:14 PM Cen Zhang (Microsoft) <blbllhy@gmail.com> wrote:
>
> From: "Cen Zhang (Microsoft Security FORGE Labs)" <blbllhy@gmail.com>
>
> do_tcp_getsockopt() reads icsk->icsk_ca_ops->name without holding
> rcu_read_lock(). Since commit 0baf26b0fcd7 ("bpf: tcp: Support
> tcp_congestion_ops in bpf"), icsk_ca_ops can point to dynamically
> allocated BPF struct_ops memory that may be freed concurrently via
> setsockopt(TCP_CONGESTION), leading to a use-after-free.
>
>   BUG: KASAN: slab-use-after-free in _copy_to_user+0x37/0x60
>   Read of size 16 at addr ffff888013505260 by task exploit/149
>    _copy_to_user+0x37/0x60
>    do_tcp_getsockopt+0x158a/0x2460 (net/ipv4/tcp.c:4585)
>    tcp_getsockopt+0x91/0xf0
>    __sys_getsockopt+0xf7/0x170
>
> Fix this by holding rcu_read_lock() around the ca_ops->name access,
> using READ_ONCE() to load icsk_ca_ops, and copying the name to a
> stack buffer before releasing the lock. Also annotate every explicit
> icsk_ca_ops store with WRITE_ONCE() to fix the accompanying KCSAN
> data-race issue.
>
> Fixes: 0baf26b0fcd7 ("bpf: tcp: Support tcp_congestion_ops in bpf")
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
> Link: https://lore.kernel.org/all/20260821182449.79785-2-blbllhy@gmail.com/
> Cc: AutonomousCodeSecurity@microsoft.com
> Cc: stable@vger.kernel.org
> Assisted-by: GitHub-Copilot:claude-opus-4.6
> Signed-off-by: Cen Zhang (Microsoft Security FORGE Labs) <blbllhy@gmail.com>
> ---

Reviewed-by: Eric Dumazet <edumazet@google.com>

Thanks.

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

* Re: [PATCH net v2 2/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CC_INFO)
  2026-08-26 17:13 ` [PATCH net v2 2/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CC_INFO) Cen Zhang (Microsoft)
@ 2026-08-26 17:31   ` Eric Dumazet
  0 siblings, 0 replies; 8+ messages in thread
From: Eric Dumazet @ 2026-08-26 17:31 UTC (permalink / raw)
  To: Cen Zhang (Microsoft)
  Cc: ncardwell, davem, kuba, pabeni, kuniyu, horms, matttbe, martineau,
	geliang, yhs, kafai, andriin, ast, netdev, mptcp, bpf,
	linux-kernel, AutonomousCodeSecurity, xmei5, tgopinath, kys,
	stable

On Wed, Aug 26, 2026 at 7:14 PM Cen Zhang (Microsoft) <blbllhy@gmail.com> wrote:
>
> From: "Cen Zhang (Microsoft Security FORGE Labs)" <blbllhy@gmail.com>
>
> do_tcp_getsockopt() reads icsk->icsk_ca_ops and dereferences the
> get_info function pointer without rcu_read_lock(). With BPF struct_ops
> congestion control, ca_ops can point to dynamically allocated memory
> that is freed concurrently, resulting in a use-after-free when the
> kernel dereferences or calls through the stale pointer.
>
>   BUG: KASAN: slab-use-after-free in do_tcp_getsockopt+0x2037/0x23e0
>   Read of size 8 at addr ffff888013701258 by task exploit/149
>    do_tcp_getsockopt+0x2037/0x23e0 (net/ipv4/tcp.c:4564)
>    tcp_getsockopt+0x91/0xf0
>    __sys_getsockopt+0xf7/0x170
>
> Fix this by wrapping the ca_ops load and get_info call within
> rcu_read_lock()/rcu_read_unlock(), and using READ_ONCE() to load
> the icsk_ca_ops pointer.
>

Reviewed-by: Eric Dumazet <edumazet@google.com>

Thanks.

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

* Re: [PATCH net v2 1/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CONGESTION)
  2026-08-26 17:13 ` [PATCH net v2 1/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CONGESTION) Cen Zhang (Microsoft)
  2026-08-26 17:30   ` Eric Dumazet
@ 2026-08-27 10:38   ` Matthieu Baerts
  2026-08-27 23:50     ` Cen Zhang (Microsoft)
  2026-08-27 17:14   ` sashiko-bot
  2 siblings, 1 reply; 8+ messages in thread
From: Matthieu Baerts @ 2026-08-27 10:38 UTC (permalink / raw)
  To: Cen Zhang (Microsoft)
  Cc: kuniyu, horms, martineau, geliang, yhs, kafai, andriin, ast,
	netdev, mptcp, bpf, linux-kernel, AutonomousCodeSecurity, xmei5,
	tgopinath, kys, stable, edumazet, ncardwell, davem, kuba, pabeni

Hi Cen,

On 26/08/2026 19:13, Cen Zhang (Microsoft) wrote:
> From: "Cen Zhang (Microsoft Security FORGE Labs)" <blbllhy@gmail.com>
> 
> do_tcp_getsockopt() reads icsk->icsk_ca_ops->name without holding
> rcu_read_lock(). Since commit 0baf26b0fcd7 ("bpf: tcp: Support
> tcp_congestion_ops in bpf"), icsk_ca_ops can point to dynamically
> allocated BPF struct_ops memory that may be freed concurrently via
> setsockopt(TCP_CONGESTION), leading to a use-after-free.
> 
>   BUG: KASAN: slab-use-after-free in _copy_to_user+0x37/0x60
>   Read of size 16 at addr ffff888013505260 by task exploit/149
>    _copy_to_user+0x37/0x60
>    do_tcp_getsockopt+0x158a/0x2460 (net/ipv4/tcp.c:4585)
>    tcp_getsockopt+0x91/0xf0
>    __sys_getsockopt+0xf7/0x170
> 
> Fix this by holding rcu_read_lock() around the ca_ops->name access,
> using READ_ONCE() to load icsk_ca_ops, and copying the name to a
> stack buffer before releasing the lock. Also annotate every explicit
> icsk_ca_ops store with WRITE_ONCE() to fix the accompanying KCSAN
> data-race issue.

(...)

> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index b4237d0e994d..4918f48ba76d 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -4577,16 +4577,24 @@ int do_tcp_getsockopt(struct sock *sk, int level,
>  		val = !inet_csk_in_pingpong_mode(sk);
>  		break;
>  
> -	case TCP_CONGESTION:
> +	case TCP_CONGESTION: {
> +		char ca_name[TCP_CA_NAME_MAX] = {};
> +
>  		if (copy_from_sockptr(&len, optlen, sizeof(int)))
>  			return -EFAULT;
>  		len = min_t(unsigned int, len, TCP_CA_NAME_MAX);
>  		if (copy_to_sockptr(optlen, &len, sizeof(int)))
>  			return -EFAULT;
> -		if (copy_to_sockptr(optval, icsk->icsk_ca_ops->name, len))
> +
> +		rcu_read_lock();
> +		ca_ops = READ_ONCE(icsk->icsk_ca_ops);

It looks like you (or your assistant) forgot to declare ca_ops:

  const struct tcp_congestion_ops *ca_ops;

Also, I *guess* a 'rcu_read_lock' is needed in mptcp_ca_reset() from
net/mptcp/protocol.c, above the modification you did, to get the name, no?

Apart from that, the modifications look good to me.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.


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

* Re: [PATCH net v2 1/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CONGESTION)
  2026-08-26 17:13 ` [PATCH net v2 1/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CONGESTION) Cen Zhang (Microsoft)
  2026-08-26 17:30   ` Eric Dumazet
  2026-08-27 10:38   ` Matthieu Baerts
@ 2026-08-27 17:14   ` sashiko-bot
  2 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-08-27 17:14 UTC (permalink / raw)
  To: Cen Zhang (Microsoft); +Cc: bpf, mptcp

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The variable `ca_ops` is used but never declared in the `TCP_CONGESTION` block.
--

commit 7bf16b98f97797b307198d9ae8e078d4cbb59158
Author: Cen Zhang (Microsoft Security FORGE Labs) <blbllhy@gmail.com>

tcp: fix use-after-free in do_tcp_getsockopt(TCP_CONGESTION)

do_tcp_getsockopt() reads icsk->icsk_ca_ops->name without holding
rcu_read_lock(). Since commit 0baf26b0fcd7, icsk_ca_ops can point to
dynamically allocated BPF struct_ops memory that may be freed concurrently via
setsockopt(TCP_CONGESTION), leading to a use-after-free.

Link: https://lore.kernel.org/all/20260821182449.79785-2-blbllhy@gmail.com/

> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index b4237d0e994d6..4918f48ba76d5 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -4577,16 +4577,24 @@ int do_tcp_getsockopt(struct sock *sk, int level,
>  		val = !inet_csk_in_pingpong_mode(sk);
>  		break;
>  
> -	case TCP_CONGESTION:
> +	case TCP_CONGESTION: {
> +		char ca_name[TCP_CA_NAME_MAX] = {};
> +
>  		if (copy_from_sockptr(&len, optlen, sizeof(int)))
>  			return -EFAULT;
>  		len = min_t(unsigned int, len, TCP_CA_NAME_MAX);
>  		if (copy_to_sockptr(optlen, &len, sizeof(int)))
>  			return -EFAULT;
> -		if (copy_to_sockptr(optval, icsk->icsk_ca_ops->name, len))
> +
> +		rcu_read_lock();
> +		ca_ops = READ_ONCE(icsk->icsk_ca_ops);

[Severity: Low]
Does this introduce a compilation error in do_tcp_getsockopt() because ca_ops
is used but never declared in the TCP_CONGESTION block?

It appears ca_ops is scoped only inside the adjacent TCP_CC_INFO case block
and is not declared at the function level or within this block, which will
cause a build failure.

> +		memcpy(ca_name, ca_ops->name, sizeof(ca_name));
> +		rcu_read_unlock();
> +
> +		if (copy_to_sockptr(optval, ca_name, len))
>  			return -EFAULT;
>  		return 0;
> -
> +	}
>  	case TCP_ULP:
>  		if (copy_from_sockptr(&len, optlen, sizeof(int)))
>  			return -EFAULT;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826171344.4133-1-blbllhy@gmail.com?part=1

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

* Re: [PATCH net v2 1/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CONGESTION)
  2026-08-27 10:38   ` Matthieu Baerts
@ 2026-08-27 23:50     ` Cen Zhang (Microsoft)
  0 siblings, 0 replies; 8+ messages in thread
From: Cen Zhang (Microsoft) @ 2026-08-27 23:50 UTC (permalink / raw)
  To: matttbe
  Cc: AutonomousCodeSecurity, andriin, ast, blbllhy, bpf, davem,
	edumazet, geliang, horms, kafai, kuba, kuniyu, kys, linux-kernel,
	martineau, mptcp, ncardwell, netdev, pabeni, stable, tgopinath,
	xmei5, yhs

On Thu, Aug 27, 2026 at 12:38:36PM +0200, Matthieu Baerts wrote:
> It looks like you (or your assistant) forgot to declare ca_ops:
>
>   const struct tcp_congestion_ops *ca_ops;
>
> Also, I *guess* a 'rcu_read_lock' is needed in mptcp_ca_reset() from
> net/mptcp/protocol.c, above the modification you did, to get the name, no?

Thanks for catching this. I messed up the local version and missed the
undeclared ca_ops. I will fix it in v3.

I also checked mptcp_ca_reset(). It is only called before the socket is
published (mptcp_init_sock) or with the socket lock held
(mptcp_disconnect). The ca_ops->name copy sits between
tcp_assign_congestion_control() and tcp_cleanup_congestion_control(), so
the ops object stays referenced and that read is already safe. The v2
WRITE_ONCE() on the subsequent NULL store is on the MPTCP meta socket,
not the TCP sockets these two UAFs race with, so I will drop it in v3.

Thanks,
Cen

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

end of thread, other threads:[~2026-08-27 23:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-26 17:13 [PATCH net v2 0/2] tcp: fix use-after-free in do_tcp_getsockopt() Cen Zhang (Microsoft)
2026-08-26 17:13 ` [PATCH net v2 1/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CONGESTION) Cen Zhang (Microsoft)
2026-08-26 17:30   ` Eric Dumazet
2026-08-27 10:38   ` Matthieu Baerts
2026-08-27 23:50     ` Cen Zhang (Microsoft)
2026-08-27 17:14   ` sashiko-bot
2026-08-26 17:13 ` [PATCH net v2 2/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CC_INFO) Cen Zhang (Microsoft)
2026-08-26 17:31   ` Eric Dumazet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox