Re: [PATCH v3 1/2] wifi: ath11k: move update channel list from update reg worker to reg notifier

[Kalle Valo <kvalo@kernel.org>]

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(&reg_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.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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