Hi,
On 1/5/21 10:24 AM, Peer, Ilan wrote:
Hi,
-----Original Message----- From: Hans de Goede hdegoede@redhat.com Sent: Monday, January 04, 2021 19:07 To: Johannes Berg johannes@sipsolutions.net; David S . Miller davem@davemloft.net; Jakub Kicinski kuba@kernel.org; Rojewski, Cezary cezary.rojewski@intel.com; Pierre-Louis Bossart <pierre- louis.bossart@linux.intel.com>; Liam Girdwood liam.r.girdwood@linux.intel.com; Jie Yang yang.jie@linux.intel.com; Mark Brown broonie@kernel.org Cc: Hans de Goede hdegoede@redhat.com; linux- wireless@vger.kernel.org; netdev@vger.kernel.org; linux- kernel@vger.kernel.org; alsa-devel@alsa-project.org; Peer, Ilan ilan.peer@intel.com Subject: [PATCH] cfg80211: Fix "suspicious RCU usage in wiphy_apply_custom_regulatory" warning/backtrace
Commit beee24695157 ("cfg80211: Save the regulatory domain when setting custom regulatory") adds a get_wiphy_regdom call to wiphy_apply_custom_regulatory. But as the comment above wiphy_apply_custom_regulatory says: "/* Used by drivers prior to wiphy registration */" this function is used by driver's probe function before the wiphy is registered and at this point wiphy->regd will typically by NULL and calling rcu_dereference_rtnl on a NULL pointer causes the following warning/backtrace:
============================= WARNING: suspicious RCU usage 5.11.0-rc1+ #19 Tainted: G W
net/wireless/reg.c:144 suspicious rcu_dereference_check() usage!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1 2 locks held by kworker/2:0/22: #0: ffff9a4bc104df38 ((wq_completion)events){+.+.}-{0:0}, at: process_one_work+0x1ee/0x570 #1: ffffb6e94010be78 ((work_completion)(&fw_work->work)){+.+.}-{0:0}, at: process_one_work+0x1ee/0x570
stack backtrace: CPU: 2 PID: 22 Comm: kworker/2:0 Tainted: G W 5.11.0-rc1+ #19 Hardware name: LENOVO 60073/INVALID, BIOS 01WT17WW 08/01/2014 Workqueue: events request_firmware_work_func Call Trace: dump_stack+0x8b/0xb0 get_wiphy_regdom+0x57/0x60 [cfg80211] wiphy_apply_custom_regulatory+0xa0/0xf0 [cfg80211] brcmf_cfg80211_attach+0xb02/0x1360 [brcmfmac] brcmf_attach+0x189/0x460 [brcmfmac] brcmf_sdio_firmware_callback+0x78a/0x8f0 [brcmfmac] brcmf_fw_request_done+0x67/0xf0 [brcmfmac] request_firmware_work_func+0x3d/0x70 process_one_work+0x26e/0x570 worker_thread+0x55/0x3c0 ? process_one_work+0x570/0x570 kthread+0x137/0x150 ? __kthread_bind_mask+0x60/0x60 ret_from_fork+0x22/0x30
Add a check for wiphy->regd being NULL before calling get_wiphy_regdom (as is already done in other places) to fix this.
wiphy->regd will likely always be NULL when wiphy->wiphy_apply_custom_regulatory gets called, so arguably the tmp = get_wiphy_regdom() and rcu_free_regdom(tmp) calls should simply be dropped, this patch keeps the 2 calls, to allow drivers to call wiphy_apply_custom_regulatory more then once if necessary.
Cc: Ilan Peer ilan.peer@intel.com Fixes: beee24695157 ("cfg80211: Save the regulatory domain when setting custom regulator") Signed-off-by: Hans de Goede hdegoede@redhat.com
net/wireless/reg.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/net/wireless/reg.c b/net/wireless/reg.c index bb72447ad960..9254b9cbaa21 100644 --- a/net/wireless/reg.c +++ b/net/wireless/reg.c @@ -2547,7 +2547,7 @@ static void handle_band_custom(struct wiphy *wiphy, void wiphy_apply_custom_regulatory(struct wiphy *wiphy, const struct ieee80211_regdomain *regd) {
- const struct ieee80211_regdomain *new_regd, *tmp;
- const struct ieee80211_regdomain *new_regd, *tmp = NULL; enum nl80211_band band; unsigned int bands_set = 0;
@@ -2571,7 +2571,8 @@ void wiphy_apply_custom_regulatory(struct wiphy *wiphy, if (IS_ERR(new_regd)) return;
- tmp = get_wiphy_regdom(wiphy);
- if (wiphy->regd)
tmp = get_wiphy_regdom(wiphy);This only fixes the first case where the pointer in NULL and does not handle the wrong RCU usage in other cases.
I'll prepare a fix for this.
Any luck with this? This is a regression in 5.11, so this really should be fixed in a future 5.11-rc and the clock is running out.
Regards,
Hans