| Message ID | 20241129070714.226-2-quic_kangyang@quicinc.com |
|---|---|
| State | Superseded |
| 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 ath11k call regulatory_set_wiphy_regd() in ath11k_regd_update() > to notify the reg domain change to cfg80211 and update channel list by > reg_work, then ath11k immediately update channel list to firmware by > ath11k_reg_update_chan_list(). > > callstack: > ath11k_regd_update > ->regulatory_set_wiphy_regd > -> schedule_work(®_work) > -> ath11k_reg_update_chan_list > > They are running in two threads, it leads the channel list data out of > sync caused by muti-threads without synchronization. At this time, > ath11k may update wrong channel list to firmware because the reg_work > still running or even hasn't started yet. In this case, if the > ath11k_reg_update_chan_list accesses an improperly updated channel list > before reg_work is completed, it may result in out of bounds write > errors, as shown in the KASAN report: > > 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 > > The correct flow is after reg_work update the channel list according to > new reg domain, ath11k call ath11k_reg_update_chan_list() and update the > new channel list to firmware. > > reg_call_notifier()(finally it will call ath11k_reg_notifier()) will be > called to by reg_work to notify ath11k when it finishes the channel > list update. So at this time, call ath11k_reg_update_chan_list() in > reg_call_notifier() with initiator type NL80211_REGDOM_SET_BY_DRIVER. > Then ath11k_reg_update_chan_list() will use the correct channel list. > > 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> The commit message would need significant work to make it more understandable, I feel that it's just explaining call flows. But clearly describing the problem and the design how it's solved would be a lot more helpful. Jeff had good guidance how to write a good commit message but I don't have a link at hand right now.
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; }