linux-kernel - [PATCH] net: Fix potential RCU dereference issue in tcp_assign_congestion

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <tencent_2A17499A4FFA4D830F7D2F72A95A4ADAB308@qq.com>
Date: Fri, 20 Sep 2024 09:08:58 +0000
From: Jiawei Ye <jiawei.ye@...mail.com>
To: edumazet@...gle.com,
	davem@...emloft.net,
	dsahern@...nel.org,
	kuba@...nel.org,
	pabeni@...hat.com
Cc: netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH] net: Fix potential RCU dereference issue in tcp_assign_congestion_control

In the `tcp_assign_congestion_control` function, the `ca->flags` is
accessed after the RCU read-side critical section is unlocked. According
to RCU usage rules, this is illegal. Reusing this pointer can lead to
unpredictable behavior, including accessing memory that has been updated
or causing use-after-free issues.

This possible bug was identified using a static analysis tool developed
by myself, specifically designed to detect RCU-related issues.

To resolve this issue, the `rcu_read_unlock` call has been moved to the
end of the function.

Signed-off-by: Jiawei Ye <jiawei.ye@...mail.com>
---
In another part of the file, `tcp_set_congestion_control` calls
`tcp_reinit_congestion_control`, ensuring that the congestion control
reinitialization process is protected by RCU. The
`tcp_reinit_congestion_control` function contains operations almost
identical to those in `tcp_assign_congestion_control`, but the former
operates under full RCU protection, whereas the latter is only partially
protected. The differing protection strategies between the two may
warrant further unification.
---
 net/ipv4/tcp_cong.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
index 0306d257fa64..356a59d316e3 100644
--- a/net/ipv4/tcp_cong.c
+++ b/net/ipv4/tcp_cong.c
@@ -223,13 +223,13 @@ void tcp_assign_congestion_control(struct sock *sk)
 	if (unlikely(!bpf_try_module_get(ca, ca->owner)))
 		ca = &tcp_reno;
 	icsk->icsk_ca_ops = ca;
-	rcu_read_unlock();
 
 	memset(icsk->icsk_ca_priv, 0, sizeof(icsk->icsk_ca_priv));
 	if (ca->flags & TCP_CONG_NEEDS_ECN)
 		INET_ECN_xmit(sk);
 	else
 		INET_ECN_dontxmit(sk);
+	rcu_read_unlock();
 }
 
 void tcp_init_congestion_control(struct sock *sk)
-- 
2.34.1