* [PATCH v5 0/2] wifi: ath11k: fix data out of sync for channel list for reg update
@ 2025-01-17 6:17 Kang Yang
2025-01-17 6:17 ` [PATCH v5 1/2] wifi: ath11k: update channel list in reg notifier instead reg worker Kang Yang
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Kang Yang @ 2025-01-17 6:17 UTC (permalink / raw)
To: ath11k; +Cc: linux-wireless, quic_kangyang
Currently there are two threads to updating/fetch data of channel
list, and there are no synchronization for the data, it leads data out
of sync for channel list when doing reg update.
So change the call flow to make sure the fetch data running after the
update data finished, then data of channel list become synchronization.
Note: This patch-set is an old patch-set in public review written by
Wen Gong. Just continue sending it for him.
Link: https://lore.kernel.org/linux-wireless/20230329091235.19500-1-quic_wgong@quicinc.com/
v5:
1. rewrite commit message in patch#1.
2. update copyright in patch#1, #2.
v4:
1. rewrite commit message in patch#1, #2.
2. use list_splice_tail_init() to move list entry.
3. use data_lock instead of adding a new lock.
v3:
1. rebase on tag ath-202411251703.
2. add KASAN BUG report in patch #1.
v2:
1. rewrite commit message for patch #1 and #2.
2. use a local list without the spinlock held for patch #2.
Wen Gong (2):
wifi: ath11k: update channel list in reg notifier instead reg worker
wifi: ath11k: update channel list in worker when wait flag is set
drivers/net/wireless/ath/ath11k/core.c | 3 +-
drivers/net/wireless/ath/ath11k/core.h | 7 +-
drivers/net/wireless/ath/ath11k/mac.c | 16 +++-
drivers/net/wireless/ath/ath11k/reg.c | 107 +++++++++++++++++--------
drivers/net/wireless/ath/ath11k/reg.h | 3 +-
drivers/net/wireless/ath/ath11k/wmi.h | 3 +-
6 files changed, 100 insertions(+), 39 deletions(-)
base-commit: d7bef42fc98f2d8f67546d1ea1a3f2c2932fd72b
--
2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v5 1/2] wifi: ath11k: update channel list in reg notifier instead reg worker
2025-01-17 6:17 [PATCH v5 0/2] wifi: ath11k: fix data out of sync for channel list for reg update Kang Yang
@ 2025-01-17 6:17 ` Kang Yang
2025-01-21 9:03 ` Aditya Kumar Singh
2025-01-17 6:17 ` [PATCH v5 2/2] wifi: ath11k: update channel list in worker when wait flag is set Kang Yang
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Kang Yang @ 2025-01-17 6:17 UTC (permalink / raw)
To: ath11k; +Cc: linux-wireless, quic_kangyang
From: Wen Gong <quic_wgong@quicinc.com>
Currently when ath11k gets a new channel list, it will be processed
according to the following steps:
1. update new channel list to cfg80211 and queue reg_work.
2. cfg80211 handles new channel list during reg_work.
3. update cfg80211's handled channel list to firmware by
ath11k_reg_update_chan_list().
But ath11k will immediately execute step 3 after reg_work is just
queued. Since step 2 is asynchronous, cfg80211 may not have completed
handling the new channel list, which may leading to an out-of-bounds
write error:
BUG: KASAN: slab-out-of-bounds in ath11k_reg_update_chan_list
Call Trace:
ath11k_reg_update_chan_list+0xbfe/0xfe0 [ath11k]
kfree+0x109/0x3a0
ath11k_regd_update+0x1cf/0x350 [ath11k]
ath11k_regd_update_work+0x14/0x20 [ath11k]
process_one_work+0xe35/0x14c0
Should ensure step 2 is completely done before executing step 3. Thus
Wen raised patch[1]. When flag NL80211_REGDOM_SET_BY_DRIVER is set,
cfg80211 will notify ath11k after step 2 is done.
So enable the flag NL80211_REGDOM_SET_BY_DRIVER then cfg80211 will
notify ath11k after step 2 is done. At this time, there will be no
KASAN bug during the execution of the step 3.
[1] https://patchwork.kernel.org/project/linux-wireless/patch/20230201065313.27203-1-quic_wgong@quicinc.com/
Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3
Fixes: f45cb6b29cd3 ("wifi: ath11k: avoid deadlock during regulatory update in ath11k_regd_update()")
Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
Signed-off-by: Kang Yang <quic_kangyang@quicinc.com>
---
drivers/net/wireless/ath/ath11k/reg.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/ath/ath11k/reg.c b/drivers/net/wireless/ath/ath11k/reg.c
index b0f289784dd3..7bfe47ad62a0 100644
--- a/drivers/net/wireless/ath/ath11k/reg.c
+++ b/drivers/net/wireless/ath/ath11k/reg.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: BSD-3-Clause-Clear
/*
* Copyright (c) 2018-2019 The Linux Foundation. All rights reserved.
- * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved.
*/
#include <linux/rtnetlink.h>
@@ -55,6 +55,19 @@ ath11k_reg_notifier(struct wiphy *wiphy, struct regulatory_request *request)
ath11k_dbg(ar->ab, ATH11K_DBG_REG,
"Regulatory Notification received for %s\n", wiphy_name(wiphy));
+ if (request->initiator == NL80211_REGDOM_SET_BY_DRIVER) {
+ ath11k_dbg(ar->ab, ATH11K_DBG_REG,
+ "driver initiated regd update\n");
+ if (ar->state != ATH11K_STATE_ON)
+ return;
+
+ ret = ath11k_reg_update_chan_list(ar, true);
+ if (ret)
+ ath11k_warn(ar->ab, "failed to update channel list: %d\n", ret);
+
+ return;
+ }
+
/* Currently supporting only General User Hints. Cell base user
* hints to be handled later.
* Hints from other sources like Core, Beacons are not expected for
@@ -293,12 +306,6 @@ int ath11k_regd_update(struct ath11k *ar)
if (ret)
goto err;
- if (ar->state == ATH11K_STATE_ON) {
- ret = ath11k_reg_update_chan_list(ar, true);
- if (ret)
- goto err;
- }
-
return 0;
err:
ath11k_warn(ab, "failed to perform regd update : %d\n", ret);
@@ -977,6 +984,7 @@ void ath11k_regd_update_work(struct work_struct *work)
void ath11k_reg_init(struct ath11k *ar)
{
ar->hw->wiphy->regulatory_flags = REGULATORY_WIPHY_SELF_MANAGED;
+ ar->hw->wiphy->flags |= WIPHY_FLAG_NOTIFY_REGDOM_BY_DRIVER;
ar->hw->wiphy->reg_notifier = ath11k_reg_notifier;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v5 2/2] wifi: ath11k: update channel list in worker when wait flag is set
2025-01-17 6:17 [PATCH v5 0/2] wifi: ath11k: fix data out of sync for channel list for reg update Kang Yang
2025-01-17 6:17 ` [PATCH v5 1/2] wifi: ath11k: update channel list in reg notifier instead reg worker Kang Yang
@ 2025-01-17 6:17 ` Kang Yang
2025-01-21 9:03 ` Aditya Kumar Singh
2025-01-21 8:00 ` [PATCH v5 0/2] wifi: ath11k: fix data out of sync for channel list for reg update Kang Yang
2025-02-06 0:16 ` Jeff Johnson
3 siblings, 1 reply; 8+ messages in thread
From: Kang Yang @ 2025-01-17 6:17 UTC (permalink / raw)
To: ath11k; +Cc: linux-wireless, quic_kangyang
From: Wen Gong <quic_wgong@quicinc.com>
With previous patch "wifi: ath11k: move update channel list from update
reg worker to reg notifier", ath11k_reg_update_chan_list() will be
called during reg_process_self_managed_hint().
reg_process_self_managed_hint() will hold rtnl_lock all the time.
But ath11k_reg_update_chan_list() may increase the occupation time of
rtnl_lock, because when wait flag is set, wait_for_completion_timeout()
will be called during 11d/hw scan.
Should minimize the occupation time of rtnl_lock as much as possible
to avoid interfering with rest of the system. So move the update channel
list operation to a new worker, so that wait_for_completion_timeout()
won't be called and will not increase the occupation time of rtnl_lock.
Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3
Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
Co-developed-by: Kang Yang <quic_kangyang@quicinc.com>
Signed-off-by: Kang Yang <quic_kangyang@quicinc.com>
---
drivers/net/wireless/ath/ath11k/core.c | 3 +-
drivers/net/wireless/ath/ath11k/core.h | 7 ++-
drivers/net/wireless/ath/ath11k/mac.c | 16 ++++-
drivers/net/wireless/ath/ath11k/reg.c | 85 ++++++++++++++++++--------
drivers/net/wireless/ath/ath11k/reg.h | 3 +-
drivers/net/wireless/ath/ath11k/wmi.h | 3 +-
6 files changed, 85 insertions(+), 32 deletions(-)
diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
index c576bbba52bf..83ad78a1a983 100644
--- a/drivers/net/wireless/ath/ath11k/core.c
+++ b/drivers/net/wireless/ath/ath11k/core.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: BSD-3-Clause-Clear
/*
* Copyright (c) 2018-2019 The Linux Foundation. All rights reserved.
- * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved.
*/
#include <linux/module.h>
@@ -2056,6 +2056,7 @@ void ath11k_core_halt(struct ath11k *ar)
ath11k_mac_scan_finish(ar);
ath11k_mac_peer_cleanup_all(ar);
cancel_delayed_work_sync(&ar->scan.timeout);
+ cancel_work_sync(&ar->channel_update_work);
cancel_work_sync(&ar->regd_update_work);
cancel_work_sync(&ab->update_11d_work);
diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
index a9dc7fe7765a..6eeec63970ac 100644
--- a/drivers/net/wireless/ath/ath11k/core.h
+++ b/drivers/net/wireless/ath/ath11k/core.h
@@ -1,7 +1,7 @@
/* SPDX-License-Identifier: BSD-3-Clause-Clear */
/*
* Copyright (c) 2018-2019 The Linux Foundation. All rights reserved.
- * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved.
*/
#ifndef ATH11K_CORE_H
@@ -685,7 +685,7 @@ struct ath11k {
struct mutex conf_mutex;
/* protects the radio specific data like debug stats, ppdu_stats_info stats,
* vdev_stop_status info, scan data, ath11k_sta info, ath11k_vif info,
- * channel context data, survey info, test mode data.
+ * channel context data, survey info, test mode data, channel_update_queue.
*/
spinlock_t data_lock;
@@ -743,6 +743,9 @@ struct ath11k {
struct completion bss_survey_done;
struct work_struct regd_update_work;
+ struct work_struct channel_update_work;
+ /* protected with data_lock */
+ struct list_head channel_update_queue;
struct work_struct wmi_mgmt_tx_work;
struct sk_buff_head wmi_mgmt_tx_queue;
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index 1556392f7ad4..25303584c57a 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: BSD-3-Clause-Clear
/*
* Copyright (c) 2018-2019 The Linux Foundation. All rights reserved.
- * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved.
*/
#include <net/mac80211.h>
@@ -6288,6 +6288,7 @@ static void ath11k_mac_op_stop(struct ieee80211_hw *hw, bool suspend)
{
struct ath11k *ar = hw->priv;
struct htt_ppdu_stats_info *ppdu_stats, *tmp;
+ struct scan_chan_list_params *params;
int ret;
ath11k_mac_drain_tx(ar);
@@ -6303,6 +6304,7 @@ static void ath11k_mac_op_stop(struct ieee80211_hw *hw, bool suspend)
mutex_unlock(&ar->conf_mutex);
cancel_delayed_work_sync(&ar->scan.timeout);
+ cancel_work_sync(&ar->channel_update_work);
cancel_work_sync(&ar->regd_update_work);
cancel_work_sync(&ar->ab->update_11d_work);
@@ -6312,10 +6314,19 @@ static void ath11k_mac_op_stop(struct ieee80211_hw *hw, bool suspend)
}
spin_lock_bh(&ar->data_lock);
+
list_for_each_entry_safe(ppdu_stats, tmp, &ar->ppdu_stats_info, list) {
list_del(&ppdu_stats->list);
kfree(ppdu_stats);
}
+
+ while ((params = list_first_entry_or_null(&ar->channel_update_queue,
+ struct scan_chan_list_params,
+ list))) {
+ list_del(¶ms->list);
+ kfree(params);
+ }
+
spin_unlock_bh(&ar->data_lock);
rcu_assign_pointer(ar->ab->pdevs_active[ar->pdev_idx], NULL);
@@ -10019,6 +10030,7 @@ static const struct wiphy_iftype_ext_capab ath11k_iftypes_ext_capa[] = {
static void __ath11k_mac_unregister(struct ath11k *ar)
{
+ cancel_work_sync(&ar->channel_update_work);
cancel_work_sync(&ar->regd_update_work);
ieee80211_unregister_hw(ar->hw);
@@ -10418,6 +10430,8 @@ int ath11k_mac_allocate(struct ath11k_base *ab)
init_completion(&ar->thermal.wmi_sync);
INIT_DELAYED_WORK(&ar->scan.timeout, ath11k_scan_timeout_work);
+ INIT_WORK(&ar->channel_update_work, ath11k_regd_update_chan_list_work);
+ INIT_LIST_HEAD(&ar->channel_update_queue);
INIT_WORK(&ar->regd_update_work, ath11k_regd_update_work);
INIT_WORK(&ar->wmi_mgmt_tx_work, ath11k_mgmt_over_wmi_tx_work);
diff --git a/drivers/net/wireless/ath/ath11k/reg.c b/drivers/net/wireless/ath/ath11k/reg.c
index 7bfe47ad62a0..d62a2014315a 100644
--- a/drivers/net/wireless/ath/ath11k/reg.c
+++ b/drivers/net/wireless/ath/ath11k/reg.c
@@ -124,32 +124,7 @@ int ath11k_reg_update_chan_list(struct ath11k *ar, bool wait)
struct channel_param *ch;
enum nl80211_band band;
int num_channels = 0;
- int i, ret, left;
-
- if (wait && ar->state_11d != ATH11K_11D_IDLE) {
- left = wait_for_completion_timeout(&ar->completed_11d_scan,
- ATH11K_SCAN_TIMEOUT_HZ);
- if (!left) {
- ath11k_dbg(ar->ab, ATH11K_DBG_REG,
- "failed to receive 11d scan complete: timed out\n");
- ar->state_11d = ATH11K_11D_IDLE;
- }
- ath11k_dbg(ar->ab, ATH11K_DBG_REG,
- "11d scan wait left time %d\n", left);
- }
-
- if (wait &&
- (ar->scan.state == ATH11K_SCAN_STARTING ||
- ar->scan.state == ATH11K_SCAN_RUNNING)) {
- left = wait_for_completion_timeout(&ar->scan.completed,
- ATH11K_SCAN_TIMEOUT_HZ);
- if (!left)
- ath11k_dbg(ar->ab, ATH11K_DBG_REG,
- "failed to receive hw scan complete: timed out\n");
-
- ath11k_dbg(ar->ab, ATH11K_DBG_REG,
- "hw scan wait left time %d\n", left);
- }
+ int i, ret = 0;
if (ar->state == ATH11K_STATE_RESTARTING)
return 0;
@@ -231,6 +206,16 @@ int ath11k_reg_update_chan_list(struct ath11k *ar, bool wait)
}
}
+ if (wait) {
+ spin_lock_bh(&ar->data_lock);
+ list_add_tail(¶ms->list, &ar->channel_update_queue);
+ spin_unlock_bh(&ar->data_lock);
+
+ queue_work(ar->ab->workqueue, &ar->channel_update_work);
+
+ return 0;
+ }
+
ret = ath11k_wmi_send_scan_chan_list_cmd(ar, params);
kfree(params);
@@ -811,6 +796,54 @@ ath11k_reg_build_regd(struct ath11k_base *ab,
return new_regd;
}
+void ath11k_regd_update_chan_list_work(struct work_struct *work)
+{
+ struct ath11k *ar = container_of(work, struct ath11k,
+ channel_update_work);
+ struct scan_chan_list_params *params;
+ struct list_head local_update_list;
+ int left;
+
+ INIT_LIST_HEAD(&local_update_list);
+
+ spin_lock_bh(&ar->data_lock);
+ list_splice_tail_init(&ar->channel_update_queue, &local_update_list);
+ spin_unlock_bh(&ar->data_lock);
+
+ while ((params = list_first_entry_or_null(&local_update_list,
+ struct scan_chan_list_params,
+ list))) {
+ if (ar->state_11d != ATH11K_11D_IDLE) {
+ left = wait_for_completion_timeout(&ar->completed_11d_scan,
+ ATH11K_SCAN_TIMEOUT_HZ);
+ if (!left) {
+ ath11k_dbg(ar->ab, ATH11K_DBG_REG,
+ "failed to receive 11d scan complete: timed out\n");
+ ar->state_11d = ATH11K_11D_IDLE;
+ }
+
+ ath11k_dbg(ar->ab, ATH11K_DBG_REG,
+ "reg 11d scan wait left time %d\n", left);
+ }
+
+ if ((ar->scan.state == ATH11K_SCAN_STARTING ||
+ ar->scan.state == ATH11K_SCAN_RUNNING)) {
+ left = wait_for_completion_timeout(&ar->scan.completed,
+ ATH11K_SCAN_TIMEOUT_HZ);
+ if (!left)
+ ath11k_dbg(ar->ab, ATH11K_DBG_REG,
+ "failed to receive hw scan complete: timed out\n");
+
+ ath11k_dbg(ar->ab, ATH11K_DBG_REG,
+ "reg hw scan wait left time %d\n", left);
+ }
+
+ ath11k_wmi_send_scan_chan_list_cmd(ar, params);
+ list_del(¶ms->list);
+ kfree(params);
+ }
+}
+
static bool ath11k_reg_is_world_alpha(char *alpha)
{
if (alpha[0] == '0' && alpha[1] == '0')
diff --git a/drivers/net/wireless/ath/ath11k/reg.h b/drivers/net/wireless/ath/ath11k/reg.h
index 263ea9061948..72b483594015 100644
--- a/drivers/net/wireless/ath/ath11k/reg.h
+++ b/drivers/net/wireless/ath/ath11k/reg.h
@@ -1,7 +1,7 @@
/* SPDX-License-Identifier: BSD-3-Clause-Clear */
/*
* Copyright (c) 2019 The Linux Foundation. All rights reserved.
- * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2022-2025 Qualcomm Innovation Center, Inc. All rights reserved.
*/
#ifndef ATH11K_REG_H
@@ -33,6 +33,7 @@ void ath11k_reg_init(struct ath11k *ar);
void ath11k_reg_reset_info(struct cur_regulatory_info *reg_info);
void ath11k_reg_free(struct ath11k_base *ab);
void ath11k_regd_update_work(struct work_struct *work);
+void ath11k_regd_update_chan_list_work(struct work_struct *work);
struct ieee80211_regdomain *
ath11k_reg_build_regd(struct ath11k_base *ab,
struct cur_regulatory_info *reg_info, bool intersect,
diff --git a/drivers/net/wireless/ath/ath11k/wmi.h b/drivers/net/wireless/ath/ath11k/wmi.h
index 8982b909c821..b7f7602e25a4 100644
--- a/drivers/net/wireless/ath/ath11k/wmi.h
+++ b/drivers/net/wireless/ath/ath11k/wmi.h
@@ -1,7 +1,7 @@
/* SPDX-License-Identifier: BSD-3-Clause-Clear */
/*
* Copyright (c) 2018-2019 The Linux Foundation. All rights reserved.
- * Copyright (c) 2021-2024 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright (c) 2021-2025 Qualcomm Innovation Center, Inc. All rights reserved.
*/
#ifndef ATH11K_WMI_H
@@ -3817,6 +3817,7 @@ struct wmi_stop_scan_cmd {
};
struct scan_chan_list_params {
+ struct list_head list;
u32 pdev_id;
u16 nallchans;
struct channel_param ch_param[];
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v5 0/2] wifi: ath11k: fix data out of sync for channel list for reg update
2025-01-17 6:17 [PATCH v5 0/2] wifi: ath11k: fix data out of sync for channel list for reg update Kang Yang
2025-01-17 6:17 ` [PATCH v5 1/2] wifi: ath11k: update channel list in reg notifier instead reg worker Kang Yang
2025-01-17 6:17 ` [PATCH v5 2/2] wifi: ath11k: update channel list in worker when wait flag is set Kang Yang
@ 2025-01-21 8:00 ` Kang Yang
2025-02-05 1:44 ` Jeff Johnson
2025-02-06 0:16 ` Jeff Johnson
3 siblings, 1 reply; 8+ messages in thread
From: Kang Yang @ 2025-01-21 8:00 UTC (permalink / raw)
To: ath11k, Jeff Johnson, Aditya Kumar Singh,
Vasanthakumar Thiagarajan
Cc: linux-wireless
On 1/17/2025 2:17 PM, Kang Yang wrote:
> Currently there are two threads to updating/fetch data of channel
> list, and there are no synchronization for the data, it leads data out
> of sync for channel list when doing reg update.
>
> So change the call flow to make sure the fetch data running after the
> update data finished, then data of channel list become synchronization.
>
> Note: This patch-set is an old patch-set in public review written by
> Wen Gong. Just continue sending it for him.
> Link: https://lore.kernel.org/linux-wireless/20230329091235.19500-1-quic_wgong@quicinc.com/
>
> v5:
> 1. rewrite commit message in patch#1.
> 2. update copyright in patch#1, #2.
> v4:
> 1. rewrite commit message in patch#1, #2.
> 2. use list_splice_tail_init() to move list entry.
> 3. use data_lock instead of adding a new lock.
> v3:
> 1. rebase on tag ath-202411251703.
> 2. add KASAN BUG report in patch #1.
> v2:
> 1. rewrite commit message for patch #1 and #2.
> 2. use a local list without the spinlock held for patch #2.
>
Hi, jeff, aditya, vasanth, any comments on this patch-set?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 1/2] wifi: ath11k: update channel list in reg notifier instead reg worker
2025-01-17 6:17 ` [PATCH v5 1/2] wifi: ath11k: update channel list in reg notifier instead reg worker Kang Yang
@ 2025-01-21 9:03 ` Aditya Kumar Singh
0 siblings, 0 replies; 8+ messages in thread
From: Aditya Kumar Singh @ 2025-01-21 9:03 UTC (permalink / raw)
To: Kang Yang, ath11k; +Cc: linux-wireless
On 1/17/25 11:47, Kang Yang wrote:
> From: Wen Gong <quic_wgong@quicinc.com>
>
> Currently when ath11k gets a new channel list, it will be processed
> according to the following steps:
> 1. update new channel list to cfg80211 and queue reg_work.
> 2. cfg80211 handles new channel list during reg_work.
> 3. update cfg80211's handled channel list to firmware by
> ath11k_reg_update_chan_list().
>
> But ath11k will immediately execute step 3 after reg_work is just
> queued. Since step 2 is asynchronous, cfg80211 may not have completed
> handling the new channel list, which may leading to an out-of-bounds
> write error:
> BUG: KASAN: slab-out-of-bounds in ath11k_reg_update_chan_list
> Call Trace:
> ath11k_reg_update_chan_list+0xbfe/0xfe0 [ath11k]
> kfree+0x109/0x3a0
> ath11k_regd_update+0x1cf/0x350 [ath11k]
> ath11k_regd_update_work+0x14/0x20 [ath11k]
> process_one_work+0xe35/0x14c0
>
> Should ensure step 2 is completely done before executing step 3. Thus
> Wen raised patch[1]. When flag NL80211_REGDOM_SET_BY_DRIVER is set,
> cfg80211 will notify ath11k after step 2 is done.
>
> So enable the flag NL80211_REGDOM_SET_BY_DRIVER then cfg80211 will
> notify ath11k after step 2 is done. At this time, there will be no
> KASAN bug during the execution of the step 3.
>
> [1] https://patchwork.kernel.org/project/linux-wireless/patch/20230201065313.27203-1-quic_wgong@quicinc.com/
>
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3
>
> Fixes: f45cb6b29cd3 ("wifi: ath11k: avoid deadlock during regulatory update in ath11k_regd_update()")
> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
> Signed-off-by: Kang Yang <quic_kangyang@quicinc.com>
> ---
Reviewed-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
--
Aditya
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 2/2] wifi: ath11k: update channel list in worker when wait flag is set
2025-01-17 6:17 ` [PATCH v5 2/2] wifi: ath11k: update channel list in worker when wait flag is set Kang Yang
@ 2025-01-21 9:03 ` Aditya Kumar Singh
0 siblings, 0 replies; 8+ messages in thread
From: Aditya Kumar Singh @ 2025-01-21 9:03 UTC (permalink / raw)
To: Kang Yang, ath11k; +Cc: linux-wireless
On 1/17/25 11:47, Kang Yang wrote:
> From: Wen Gong <quic_wgong@quicinc.com>
>
> With previous patch "wifi: ath11k: move update channel list from update
> reg worker to reg notifier", ath11k_reg_update_chan_list() will be
> called during reg_process_self_managed_hint().
>
> reg_process_self_managed_hint() will hold rtnl_lock all the time.
> But ath11k_reg_update_chan_list() may increase the occupation time of
> rtnl_lock, because when wait flag is set, wait_for_completion_timeout()
> will be called during 11d/hw scan.
>
> Should minimize the occupation time of rtnl_lock as much as possible
> to avoid interfering with rest of the system. So move the update channel
> list operation to a new worker, so that wait_for_completion_timeout()
> won't be called and will not increase the occupation time of rtnl_lock.
>
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3
>
> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
> Co-developed-by: Kang Yang <quic_kangyang@quicinc.com>
> Signed-off-by: Kang Yang <quic_kangyang@quicinc.com>
> ---
Reviewed-by: Aditya Kumar Singh <quic_adisi@quicinc.com>
--
Aditya
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 0/2] wifi: ath11k: fix data out of sync for channel list for reg update
2025-01-21 8:00 ` [PATCH v5 0/2] wifi: ath11k: fix data out of sync for channel list for reg update Kang Yang
@ 2025-02-05 1:44 ` Jeff Johnson
0 siblings, 0 replies; 8+ messages in thread
From: Jeff Johnson @ 2025-02-05 1:44 UTC (permalink / raw)
To: Kang Yang, ath11k, Jeff Johnson, Aditya Kumar Singh,
Vasanthakumar Thiagarajan
Cc: linux-wireless
On 1/21/2025 12:00 AM, Kang Yang wrote:
> On 1/17/2025 2:17 PM, Kang Yang wrote:
>> Currently there are two threads to updating/fetch data of channel
>> list, and there are no synchronization for the data, it leads data out
>> of sync for channel list when doing reg update.
>>
>> So change the call flow to make sure the fetch data running after the
>> update data finished, then data of channel list become synchronization.
>>
>> Note: This patch-set is an old patch-set in public review written by
>> Wen Gong. Just continue sending it for him.
>> Link: https://lore.kernel.org/linux-wireless/20230329091235.19500-1-quic_wgong@quicinc.com/
>>
>> v5:
>> 1. rewrite commit message in patch#1.
>> 2. update copyright in patch#1, #2.
>> v4:
>> 1. rewrite commit message in patch#1, #2.
>> 2. use list_splice_tail_init() to move list entry.
>> 3. use data_lock instead of adding a new lock.
>> v3:
>> 1. rebase on tag ath-202411251703.
>> 2. add KASAN BUG report in patch #1.
>> v2:
>> 1. rewrite commit message for patch #1 and #2.
>> 2. use a local list without the spinlock held for patch #2.
>>
>
> Hi, jeff, aditya, vasanth, any comments on this patch-set?
Now pushed to 'pending'
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5 0/2] wifi: ath11k: fix data out of sync for channel list for reg update
2025-01-17 6:17 [PATCH v5 0/2] wifi: ath11k: fix data out of sync for channel list for reg update Kang Yang
` (2 preceding siblings ...)
2025-01-21 8:00 ` [PATCH v5 0/2] wifi: ath11k: fix data out of sync for channel list for reg update Kang Yang
@ 2025-02-06 0:16 ` Jeff Johnson
3 siblings, 0 replies; 8+ messages in thread
From: Jeff Johnson @ 2025-02-06 0:16 UTC (permalink / raw)
To: ath11k, Kang Yang; +Cc: linux-wireless
On Fri, 17 Jan 2025 14:17:35 +0800, Kang Yang wrote:
> Currently there are two threads to updating/fetch data of channel
> list, and there are no synchronization for the data, it leads data out
> of sync for channel list when doing reg update.
>
> So change the call flow to make sure the fetch data running after the
> update data finished, then data of channel list become synchronization.
>
> [...]
Applied, thanks!
[1/2] wifi: ath11k: update channel list in reg notifier instead reg worker
commit: 933ab187e679e6fbdeea1835ae39efcc59c022d2
[2/2] wifi: ath11k: update channel list in worker when wait flag is set
commit: 02aae8e2f957adc1b15b6b8055316f8a154ac3f5
Best regards,
--
Jeff Johnson <jeff.johnson@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-02-06 0:16 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-17 6:17 [PATCH v5 0/2] wifi: ath11k: fix data out of sync for channel list for reg update Kang Yang
2025-01-17 6:17 ` [PATCH v5 1/2] wifi: ath11k: update channel list in reg notifier instead reg worker Kang Yang
2025-01-21 9:03 ` Aditya Kumar Singh
2025-01-17 6:17 ` [PATCH v5 2/2] wifi: ath11k: update channel list in worker when wait flag is set Kang Yang
2025-01-21 9:03 ` Aditya Kumar Singh
2025-01-21 8:00 ` [PATCH v5 0/2] wifi: ath11k: fix data out of sync for channel list for reg update Kang Yang
2025-02-05 1:44 ` Jeff Johnson
2025-02-06 0:16 ` Jeff Johnson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox