2022-11-24 07:40:55

by Karthikeyan Kathirvel

[permalink] [raw]
Subject: [PATCHv2] wifi: ath11k: Fix race condition with htt_ppdu_stats_info

From: Govindaraj Saminathan <[email protected]>

The below crash happens when running the traffic with multiple clients

Crash Signature : Unable to handle kernel paging request at
virtual address ffffffd700970918 During the crash, PC points to
"ieee80211_tx_rate_update+0x30/0x68 [mac80211]"
LR points to "ath11k_dp_htt_htc_t2h_msg_handler+0x5a8/0x8a0 [ath11k]".

ppdu_stats_info is allocated and accessed from event callback via copy
engine tasklet, this has a problem when freeing it from ath11k_mac_op_stop.

Add spin lock to protect htt_ppdu_stats_info and to avoid race condition
when accessing it from ath11k_mac_op_stop.

Tested-on : IPQ8074 hw2.0 AHB WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1

Signed-off-by: Govindaraj Saminathan <[email protected]>
Co-developed-by: Karthikeyan Kathirvel <[email protected]>
Signed-off-by: Karthikeyan Kathirvel <[email protected]>
---
v2:
Rebased and Modified the author details

drivers/net/wireless/ath/ath11k/dp_rx.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/dp_rx.c b/drivers/net/wireless/ath/ath11k/dp_rx.c
index c5a4c34d7749..2384c8c3c55c 100644
--- a/drivers/net/wireless/ath/ath11k/dp_rx.c
+++ b/drivers/net/wireless/ath/ath11k/dp_rx.c
@@ -1535,11 +1535,11 @@ struct htt_ppdu_stats_info *ath11k_dp_htt_get_ppdu_desc(struct ath11k *ar,
{
struct htt_ppdu_stats_info *ppdu_info;

- spin_lock_bh(&ar->data_lock);
+ lockdep_assert_held(&ar->data_lock);
+
if (!list_empty(&ar->ppdu_stats_info)) {
list_for_each_entry(ppdu_info, &ar->ppdu_stats_info, list) {
if (ppdu_info->ppdu_id == ppdu_id) {
- spin_unlock_bh(&ar->data_lock);
return ppdu_info;
}
}
@@ -1553,16 +1553,13 @@ struct htt_ppdu_stats_info *ath11k_dp_htt_get_ppdu_desc(struct ath11k *ar,
kfree(ppdu_info);
}
}
- spin_unlock_bh(&ar->data_lock);

ppdu_info = kzalloc(sizeof(*ppdu_info), GFP_ATOMIC);
if (!ppdu_info)
return NULL;

- spin_lock_bh(&ar->data_lock);
list_add_tail(&ppdu_info->list, &ar->ppdu_stats_info);
ar->ppdu_stat_list_depth++;
- spin_unlock_bh(&ar->data_lock);

return ppdu_info;
}
@@ -1586,16 +1583,17 @@ static int ath11k_htt_pull_ppdu_stats(struct ath11k_base *ab,
ar = ath11k_mac_get_ar_by_pdev_id(ab, pdev_id);
if (!ar) {
ret = -EINVAL;
- goto exit;
+ goto out;
}

if (ath11k_debugfs_is_pktlog_lite_mode_enabled(ar))
trace_ath11k_htt_ppdu_stats(ar, skb->data, len);

+ spin_lock_bh(&ar->data_lock);
ppdu_info = ath11k_dp_htt_get_ppdu_desc(ar, ppdu_id);
if (!ppdu_info) {
ret = -EINVAL;
- goto exit;
+ goto out_unlock_data;
}

ppdu_info->ppdu_id = ppdu_id;
@@ -1604,10 +1602,13 @@ static int ath11k_htt_pull_ppdu_stats(struct ath11k_base *ab,
(void *)ppdu_info);
if (ret) {
ath11k_warn(ab, "Failed to parse tlv %d\n", ret);
- goto exit;
+ goto out_unlock_data;
}

-exit:
+out_unlock_data:
+ spin_unlock_bh(&ar->data_lock);
+
+out:
rcu_read_unlock();

return ret;

base-commit: d87a77cb16ca7c51f5ea67f345137ade24245153
--
2.38.0


2022-11-25 11:59:53

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCHv2] wifi: ath11k: Fix race condition with htt_ppdu_stats_info

Karthikeyan Kathirvel <[email protected]> wrote:

> The below crash happens when running the traffic with multiple clients
>
> Crash Signature : Unable to handle kernel paging request at
> virtual address ffffffd700970918 During the crash, PC points to
> "ieee80211_tx_rate_update+0x30/0x68 [mac80211]"
> LR points to "ath11k_dp_htt_htc_t2h_msg_handler+0x5a8/0x8a0 [ath11k]".
>
> ppdu_stats_info is allocated and accessed from event callback via copy
> engine tasklet, this has a problem when freeing it from ath11k_mac_op_stop.
>
> Add spin lock to protect htt_ppdu_stats_info and to avoid race condition
> when accessing it from ath11k_mac_op_stop.
>
> Tested-on : IPQ8074 hw2.0 AHB WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Govindaraj Saminathan <[email protected]>
> Co-developed-by: Karthikeyan Kathirvel <[email protected]>
> Signed-off-by: Karthikeyan Kathirvel <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>

This patch added a new checkpatch warning:

drivers/net/wireless/ath/ath11k/dp_rx.c:1542: braces {} are not necessary for single statement blocks

I fixed it in the pending branch.

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

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

2022-12-02 20:08:35

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCHv2] wifi: ath11k: Fix race condition with htt_ppdu_stats_info

Karthikeyan Kathirvel <[email protected]> wrote:

> A crash happens when running the traffic with multiple clients:
>
> Crash Signature : Unable to handle kernel paging request at
> virtual address ffffffd700970918 During the crash, PC points to
> "ieee80211_tx_rate_update+0x30/0x68 [mac80211]"
> LR points to "ath11k_dp_htt_htc_t2h_msg_handler+0x5a8/0x8a0 [ath11k]".
>
> Struct ppdu_stats_info is allocated and accessed from event callback via copy
> engine tasklet, this has a problem when freeing it from ath11k_mac_op_stop().
>
> Use data_lock during entire ath11k_dp_htt_get_ppdu_desc() call to protect
> struct htt_ppdu_stats_info access and to avoid race condition when accessing it
> from ath11k_mac_op_stop().
>
> Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Govindaraj Saminathan <[email protected]>
> Co-developed-by: Karthikeyan Kathirvel <[email protected]>
> Signed-off-by: Karthikeyan Kathirvel <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>

I cleaned up the commit log.

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

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

2022-12-02 20:08:43

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCHv2] wifi: ath11k: Fix race condition with htt_ppdu_stats_info

Karthikeyan Kathirvel <[email protected]> wrote:

> A crash happens when running the traffic with multiple clients:
>
> Crash Signature : Unable to handle kernel paging request at
> virtual address ffffffd700970918 During the crash, PC points to
> "ieee80211_tx_rate_update+0x30/0x68 [mac80211]"
> LR points to "ath11k_dp_htt_htc_t2h_msg_handler+0x5a8/0x8a0 [ath11k]".
>
> Struct ppdu_stats_info is allocated and accessed from event callback via copy
> engine tasklet, this has a problem when freeing it from ath11k_mac_op_stop().
>
> Use data_lock during entire ath11k_dp_htt_get_ppdu_desc() call to protect
> struct htt_ppdu_stats_info access and to avoid race condition when accessing it
> from ath11k_mac_op_stop().
>
> Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Govindaraj Saminathan <[email protected]>
> Co-developed-by: Karthikeyan Kathirvel <[email protected]>
> Signed-off-by: Karthikeyan Kathirvel <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>

Patch applied to ath-next branch of ath.git, thanks.

e44de90453bb wifi: ath11k: Fix race condition with struct htt_ppdu_stats_info

--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/

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