linux-kernel - [PATCH] ath11k: Fix potential RCU dereference issue in ath11k_debugfs_htt_ext_stats

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_A64CA96B962349E369B349EA01EBC53C3505@qq.com>
Date: Fri, 30 Aug 2024 12:02:27 +0000
From: Jiawei Ye <jiawei.ye@...mail.com>
To: kvalo@...nel.org,
	jjohnson@...nel.org,
	corbet@....net
Cc: linux-wireless@...r.kernel.org,
	ath11k@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH] ath11k: Fix potential RCU dereference issue in ath11k_debugfs_htt_ext_stats_handler

In the `ath11k_debugfs_htt_ext_stats_handler` function, the `ar` pointer
obtained via RCU lock is accessed after the RCU read-side critical
section might be 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.
The `ath12k_debugfs_htt_ext_stats_handler` function in the
`drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c` file provides a good
example to follow for addressing this issue.

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

To address this issue, the RCU read lock is now kept until all accesses
to the `ar` pointer are completed. A `goto exit` statement is introduced
to ensure that the RCU read unlock is called appropriately, regardless of
the function's exit path.

Signed-off-by: Jiawei Ye <jiawei.ye@...mail.com>
---
 drivers/net/wireless/ath/ath11k/debugfs_htt_stats.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/debugfs_htt_stats.c b/drivers/net/wireless/ath/ath11k/debugfs_htt_stats.c
index 870e86a31bf8..325377e00818 100644
--- a/drivers/net/wireless/ath/ath11k/debugfs_htt_stats.c
+++ b/drivers/net/wireless/ath/ath11k/debugfs_htt_stats.c
@@ -4572,15 +4572,14 @@ void ath11k_debugfs_htt_ext_stats_handler(struct ath11k_base *ab,
 	pdev_id = FIELD_GET(HTT_STATS_COOKIE_LSB, cookie);
 	rcu_read_lock();
 	ar = ath11k_mac_get_ar_by_pdev_id(ab, pdev_id);
-	rcu_read_unlock();
 	if (!ar) {
 		ath11k_warn(ab, "failed to get ar for pdev_id %d\n", pdev_id);
-		return;
+		goto exit;
 	}

 	stats_req = ar->debug.htt_stats.stats_req;
 	if (!stats_req)
-		return;
+		goto exit;

 	spin_lock_bh(&ar->debug.htt_stats.lock);

@@ -4599,6 +4598,8 @@ void ath11k_debugfs_htt_ext_stats_handler(struct ath11k_base *ab,

 	if (send_completion)
 		complete(&stats_req->cmpln);
+exit:
+	rcu_read_unlock();
 }

 static ssize_t ath11k_read_htt_stats_type(struct file *file,
-- 
2.34.1