[PATCH ath-current v2] wifi: ath11k: fix locking problem in ath11k_dp_rx_tid_del_func()
Nicolas Escande <[email protected]>
| Newsgroups | org.infradead.lists.ath11k,org.kernel.vger.linux-wireless |
|---|---|
| Message-ID | <[email protected]> |
In this function, we iterate over dp->reo_cmd_cache_flush_list using
list_for_each_entry_safe(), under dp->reo_cmd_lock for concurrent access,
and for each expired entries we :
- drop the lock
- call ath11k_dp_reo_cache_flush()
- kfree() the entry
- retake the lock to keep on iterating
list_for_each_entry_safe() protects us from deleting the entry during
iteration but doesn't protect for concurrent access. So another thread can
take the lock and modify the list in between and crash like below.
To fix the issue, move all entries that needs to be freed to a local list
while under the lock and then iterate over the list to free the entries
without holding the lock.
BUG: Unable to handle kernel paging request at virtual address 00000010ddbeef8c
Call trace:
ath11k_dp_rx_tid_del_func+0x164/0x3c8
ath11k_dp_process_reo_status+0x1d4/0x2fc
ath11k_dp_service_srng+0x334/0x338
ath11k_pcic_ext_grp_napi_poll+0x30/0xc0
__napi_poll+0x34/0x184
napi_threaded_poll+0xb4/0x1d8
kthread+0xdc/0xe0
ret_from_fork+0x10/0x20
Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.9.0.1-01977-QCAHKSWPL_SILICONZ-1
Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
Suggested-by: Baochen Qiang <[email protected]>
Signed-off-by: Nicolas Escande <[email protected]>
---
Changes in v2:
- Dropped original patch from Maxime
- Switched to a new design suggested by Baochen
We iterate the list and move the aged entries to a local list under lock
Then commit the entries to the hardware & free all aged entries from the
local list without holding no lock
- Added harware revision to Tested-on
- Link to v1: https://patch.msgid.link/[email protected]
---
drivers/net/wireless/ath/ath11k/dp_rx.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
index 33425707c084..5b717b731197 100644
--- a/drivers/net/wireless/ath/ath11k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
@@ -762,6 +762,9 @@ static void ath11k_dp_rx_tid_del_func(struct ath11k_dp *dp, void *ctx,
struct ath11k_base *ab = dp->ab;
struct dp_rx_tid *rx_tid = ctx;
struct dp_reo_cache_flush_elem *elem, *tmp;
+ struct list_head flush_list;
+
+ INIT_LIST_HEAD(&flush_list);
if (status == HAL_REO_CMD_DRAIN) {
goto free_desc;
@@ -783,23 +786,25 @@ static void ath11k_dp_rx_tid_del_func(struct ath11k_dp *dp, void *ctx,
list_add_tail(&elem->list, &dp->reo_cmd_cache_flush_list);
dp->reo_cmd_cache_flush_count++;
- /* Flush and invalidate aged REO desc from HW cache */
+ /* identify aged REO desc that needs removal */
list_for_each_entry_safe(elem, tmp, &dp->reo_cmd_cache_flush_list,
list) {
if (dp->reo_cmd_cache_flush_count > DP_REO_DESC_FREE_THRESHOLD ||
time_after(jiffies, elem->ts +
msecs_to_jiffies(DP_REO_DESC_FREE_TIMEOUT_MS))) {
- list_del(&elem->list);
+ list_move_tail(&elem->list, &flush_list);
dp->reo_cmd_cache_flush_count--;
- spin_unlock_bh(&dp->reo_cmd_lock);
-
- ath11k_dp_reo_cache_flush(ab, &elem->data);
- kfree(elem);
- spin_lock_bh(&dp->reo_cmd_lock);
}
}
spin_unlock_bh(&dp->reo_cmd_lock);
+ /* remove aged REO desc from HW */
+ list_for_each_entry_safe(elem, tmp, &flush_list, list) {
+ ath11k_dp_reo_cache_flush(ab, &elem->data);
+ list_del(&elem->list);
+ kfree(elem);
+ }
+
return;
free_desc:
dma_free_noncoherent(ab->dev, rx_tid->unaligned_size,
---
base-commit: e07447e654476262558bee570f4cf456e2b32565
change-id: 20260812-ath11k-locking-52f5e3cc6937
Best regards,
--
Nicolas Escande <[email protected]>