2023-10-19 11:37:30

by Johan Hovold

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

As was reported here:

https://lore.kernel.org/lkml/[email protected]/
https://lore.kernel.org/lkml/[email protected]/

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


2023-10-19 11:37:30

by Johan Hovold

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

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: [email protected] # v6.2
Signed-off-by: Johan Hovold <[email protected]>
---
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

2023-10-19 11:37:41

by Johan Hovold

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

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: [email protected] # v6.2
Signed-off-by: Johan Hovold <[email protected]>
---
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

2023-10-19 17:31:16

by Jeff Johnson

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

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: [email protected] # v6.2
> Signed-off-by: Johan Hovold <[email protected]>
Acked-by: Jeff Johnson <[email protected]>

2023-10-25 10:02:24

by Kalle Valo

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

Johan Hovold <[email protected]> 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: [email protected] # v6.2
> Signed-off-by: Johan Hovold <[email protected]>
> Acked-by: Jeff Johnson <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>

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/[email protected]/

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