MPTCP Linux Development
 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)
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ 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] 11+ 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)
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 11+ 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] 11+ 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
  2026-08-26 17:50 ` [PATCH net v2 0/2] tcp: fix use-after-free in do_tcp_getsockopt() MPTCP CI
  2026-08-26 18:01 ` MPTCP CI
  3 siblings, 1 reply; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ messages in thread

* Re: [PATCH net v2 0/2] tcp: fix use-after-free in do_tcp_getsockopt()
  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 ` [PATCH net v2 2/2] tcp: fix use-after-free in do_tcp_getsockopt(TCP_CC_INFO) Cen Zhang (Microsoft)
@ 2026-08-26 17:50 ` MPTCP CI
  2026-08-27 23:34   ` Cen Zhang (Microsoft)
  2026-08-26 18:01 ` MPTCP CI
  3 siblings, 1 reply; 11+ messages in thread
From: MPTCP CI @ 2026-08-26 17:50 UTC (permalink / raw)
  To: Cen Zhang; +Cc: mptcp

Hi Cen,

Thank you for your modifications, that's great!

But sadly, our CI spotted some issues with it when trying to build it.

You can find more details there:

  https://github.com/multipath-tcp/mptcp_net-next/actions/runs/32995824879

Status: failure
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/054504491348
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1152216

Feel free to reply to this email if you cannot access logs, if you need
some support to fix the error, if this doesn't seem to be caused by your
modifications or if the error is a false positive one.

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

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

* Re: [PATCH net v2 0/2] tcp: fix use-after-free in do_tcp_getsockopt()
  2026-08-26 17:13 [PATCH net v2 0/2] tcp: fix use-after-free in do_tcp_getsockopt() Cen Zhang (Microsoft)
                   ` (2 preceding siblings ...)
  2026-08-26 17:50 ` [PATCH net v2 0/2] tcp: fix use-after-free in do_tcp_getsockopt() MPTCP CI
@ 2026-08-26 18:01 ` MPTCP CI
  3 siblings, 0 replies; 11+ messages in thread
From: MPTCP CI @ 2026-08-26 18:01 UTC (permalink / raw)
  To: Cen Zhang; +Cc: mptcp

Hi Cen,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Script error! ❓
- KVM Validation: normal (only selftest_mptcp_join): Script error! ❓
- KVM Validation: debug (except selftest_mptcp_join): Script error! ❓
- KVM Validation: debug (only selftest_mptcp_join): Script error! ❓
- KVM Validation: btf-normal (only bpftest_all): Script error! ❓
- KVM Validation: btf-debug (only bpftest_all): Script error! ❓
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/32995824882

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/054504491348
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1152216


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

^ permalink raw reply	[flat|nested] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ messages in thread

* Re: [PATCH net v2 0/2] tcp: fix use-after-free in do_tcp_getsockopt()
  2026-08-26 17:50 ` [PATCH net v2 0/2] tcp: fix use-after-free in do_tcp_getsockopt() MPTCP CI
@ 2026-08-27 23:34   ` Cen Zhang (Microsoft)
  0 siblings, 0 replies; 11+ messages in thread
From: Cen Zhang (Microsoft) @ 2026-08-27 23:34 UTC (permalink / raw)
  To: wpasupplicant.patchew; +Cc: blbllhy, mptcp, AutonomousCodeSecurity

On Wed, Aug 26, 2026 at 05:50:09PM +0000, MPTCP CI wrote:
> But sadly, our CI spotted some issues with it when trying to build it.

Thanks for the report. I will fix it in v3 shortly.

Thanks,
Cen

^ permalink raw reply	[flat|nested] 11+ 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; 11+ 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] 11+ messages in thread

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

Thread overview: 11+ 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
2026-08-26 17:50 ` [PATCH net v2 0/2] tcp: fix use-after-free in do_tcp_getsockopt() MPTCP CI
2026-08-27 23:34   ` Cen Zhang (Microsoft)
2026-08-26 18:01 ` MPTCP CI

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