[PATCH 0/2] wifi: ath12k: fix event locking

[PATCH 0/2] wifi: ath12k: fix event locking

[Johan Hovold]

As was reported here:

	https://lore.kernel.org/lkml/20231019104211.13769-1-johan+linaro@kernel.org/
	https://lore.kernel.org/lkml/20231019112521.2071-1-johan+linaro@kernel.org/

RCU lockdep reported suspicious RCU usage in the ath11k temperature
event handling code and code review revealed a few more handlers with
similar problems.

Apparently these issues have also been reproduced in the ath12k driver. 

Note that these were found through inspection and that this series has
only been compile tested.

Johan


Johan Hovold (2):
  wifi: ath12k: fix dfs-radar and temperature event locking
  wifi: ath12k: fix htt mlo-offset event locking

 drivers/net/wireless/ath/ath12k/dp_rx.c | 7 +++++--
 drivers/net/wireless/ath/ath12k/wmi.c   | 8 +++++++-
 2 files changed, 12 insertions(+), 3 deletions(-)

-- 
2.41.0

[PATCH 1/2] wifi: ath12k: fix dfs-radar and temperature event locking

[Johan Hovold]

The ath12k active pdevs are protected by RCU but the DFS-radar and
temperature event handling code calling ath12k_mac_get_ar_by_pdev_id()
was not marked as a read-side critical section.

Mark the code in question as RCU read-side critical sections to avoid
any potential use-after-free issues.

Note that the temperature event handler looks like a place holder
currently but would still trigger an RCU lockdep splat.

Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
Cc: stable@vger.kernel.org	# v6.2
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/net/wireless/ath/ath12k/wmi.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
index ef0f3cf35cfd..1a1f57c7ac7e 100644
--- a/drivers/net/wireless/ath/ath12k/wmi.c
+++ b/drivers/net/wireless/ath/ath12k/wmi.c
@@ -6476,6 +6476,7 @@ ath12k_wmi_pdev_dfs_radar_detected_event(struct ath12k_base *ab, struct sk_buff
 		   ev->detector_id, ev->segment_id, ev->timestamp, ev->is_chirp,
 		   ev->freq_offset, ev->sidx);
 
+	rcu_read_lock();
 	ar = ath12k_mac_get_ar_by_pdev_id(ab, le32_to_cpu(ev->pdev_id));
 
 	if (!ar) {
@@ -6493,6 +6494,8 @@ ath12k_wmi_pdev_dfs_radar_detected_event(struct ath12k_base *ab, struct sk_buff
 		ieee80211_radar_detected(ar->hw);
 
 exit:
+	rcu_read_unlock();
+
 	kfree(tb);
 }
 
@@ -6511,11 +6514,14 @@ ath12k_wmi_pdev_temperature_event(struct ath12k_base *ab,
 	ath12k_dbg(ab, ATH12K_DBG_WMI,
 		   "pdev temperature ev temp %d pdev_id %d\n", ev.temp, ev.pdev_id);
 
+	rcu_read_lock();
 	ar = ath12k_mac_get_ar_by_pdev_id(ab, le32_to_cpu(ev.pdev_id));
 	if (!ar) {
 		ath12k_warn(ab, "invalid pdev id in pdev temperature ev %d", ev.pdev_id);
-		return;
+		goto exit;
 	}
+exit:
+	rcu_read_unlock();
 }
 
 static void ath12k_fils_discovery_event(struct ath12k_base *ab,
-- 
2.41.0

Re: [PATCH 1/2] wifi: ath12k: fix dfs-radar and temperature event locking

[Jeff Johnson]

On 10/19/2023 4:36 AM, Johan Hovold wrote:
> The ath12k active pdevs are protected by RCU but the DFS-radar and
> temperature event handling code calling ath12k_mac_get_ar_by_pdev_id()
> was not marked as a read-side critical section.
> 
> Mark the code in question as RCU read-side critical sections to avoid
> any potential use-after-free issues.
> 
> Note that the temperature event handler looks like a place holder
> currently but would still trigger an RCU lockdep splat.
> 
> Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
> Cc: stable@vger.kernel.org	# v6.2
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>

Re: [PATCH 1/2] wifi: ath12k: fix dfs-radar and temperature event locking

[Kalle Valo]

Johan Hovold <johan+linaro@kernel.org> wrote:

> The ath12k active pdevs are protected by RCU but the DFS-radar and
> temperature event handling code calling ath12k_mac_get_ar_by_pdev_id()
> was not marked as a read-side critical section.
> 
> Mark the code in question as RCU read-side critical sections to avoid
> any potential use-after-free issues.
> 
> Note that the temperature event handler looks like a place holder
> currently but would still trigger an RCU lockdep splat.
> 
> Compile tested only.
> 
> Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
> Cc: stable@vger.kernel.org      # v6.2
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>

2 patches applied to ath-next branch of ath.git, thanks.

69bd216e0493 wifi: ath12k: fix dfs-radar and temperature event locking
6afc57ea315e wifi: ath12k: fix htt mlo-offset event locking

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20231019113650.9060-2-johan+linaro@kernel.org/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH 2/2] wifi: ath12k: fix htt mlo-offset event locking

[Johan Hovold]

The ath12k active pdevs are protected by RCU but the htt mlo-offset
event handling code calling ath12k_mac_get_ar_by_pdev_id() was not
marked as a read-side critical section.

Mark the code in question as an RCU read-side critical section to avoid
any potential use-after-free issues.

Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
Cc: stable@vger.kernel.org      # v6.2
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/net/wireless/ath/ath12k/dp_rx.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/dp_rx.c b/drivers/net/wireless/ath/ath12k/dp_rx.c
index e6e64d437c47..3294625650dc 100644
--- a/drivers/net/wireless/ath/ath12k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath12k/dp_rx.c
@@ -1641,11 +1641,12 @@ static void ath12k_htt_mlo_offset_event_handler(struct ath12k_base *ab,
 	msg = (struct ath12k_htt_mlo_offset_msg *)skb->data;
 	pdev_id = u32_get_bits(__le32_to_cpu(msg->info),
 			       HTT_T2H_MLO_OFFSET_INFO_PDEV_ID);
-	ar = ath12k_mac_get_ar_by_pdev_id(ab, pdev_id);
 
+	rcu_read_lock();
+	ar = ath12k_mac_get_ar_by_pdev_id(ab, pdev_id);
 	if (!ar) {
 		ath12k_warn(ab, "invalid pdev id %d on htt mlo offset\n", pdev_id);
-		return;
+		goto exit;
 	}
 
 	spin_lock_bh(&ar->data_lock);
@@ -1661,6 +1662,8 @@ static void ath12k_htt_mlo_offset_event_handler(struct ath12k_base *ab,
 	pdev->timestamp.mlo_comp_timer = __le32_to_cpu(msg->mlo_comp_timer);
 
 	spin_unlock_bh(&ar->data_lock);
+exit:
+	rcu_read_unlock();
 }
 
 void ath12k_dp_htt_htc_t2h_msg_handler(struct ath12k_base *ab,
-- 
2.41.0

Re: [PATCH 2/2] wifi: ath12k: fix htt mlo-offset event locking

[Jeff Johnson]

On 10/19/2023 4:36 AM, Johan Hovold wrote:
> The ath12k active pdevs are protected by RCU but the htt mlo-offset
> event handling code calling ath12k_mac_get_ar_by_pdev_id() was not
> marked as a read-side critical section.
> 
> Mark the code in question as an RCU read-side critical section to avoid
> any potential use-after-free issues.
> 
> Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
> Cc: stable@vger.kernel.org      # v6.2
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>