| Message ID | 20241213093909.629-2-quic_kangyang@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | wifi: ath11k: fix data out of sync for channel list for reg update | expand |
Kang Yang <quic_kangyang@quicinc.com> writes: > From: Wen Gong <quic_wgong@quicinc.com> > > Currently when ath11k gets new channel list, it will mainly do two things > in ath11k_regd_update(): > 1. update channel list to cfg80211 by reg_work. > 2. update cfg80211's channel list to firmware by > ath11k_reg_update_chan_list(). > > Flow: > ath11k_regd_update > ->regulatory_set_wiphy_regd > -> schedule_work(®_work) > ->reg_work->reg_process_self_managed_hint > ->handle_band_custom(update to cfg80211) > -> ath11k_reg_update_chan_list(update to firmware) > > But ath11k_reg_update_chan_list() is immediately called after reg_work > is queued. They are running in different threads. At this time, > ath11k_reg_update_chan_list() may use a wrong channel list because > handle_band_custom() may not be finished. > This may result in out-of-bounds write errors: > 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 > > So should make sure ath11k_reg_update_chan_list() is called after > handle_band_custom() is finished. > > reg_process_self_managed_hint() will call reg_call_notifier() after > handle_band_custom(). This function will call ath11k_reg_notifier(), so > move ath11k_reg_update_chan_list() to ath11k_reg_notifier(). Then > ath11k can update correct channel list to firmware. > > 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> I think the commit message should be completely rewritten, the idea here is not to list functions and their call orders.
diff --git a/drivers/net/wireless/ath/ath11k/reg.c b/drivers/net/wireless/ath/ath11k/reg.c index b0f289784dd3..cb2cf9b63d18 100644 --- a/drivers/net/wireless/ath/ath11k/reg.c +++ b/drivers/net/wireless/ath/ath11k/reg.c @@ -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; }