* [PATCH] cfg80211: Fix "suspicious RCU usage in wiphy_apply_custom_regulatory" warning/backtrace
@ 2021-01-04 17:07 Hans de Goede
2021-01-05 9:24 ` Peer, Ilan
0 siblings, 1 reply; 5+ messages in thread
From: Hans de Goede @ 2021-01-04 17:07 UTC (permalink / raw)
To: Johannes Berg, David S . Miller, Jakub Kicinski, Cezary Rojewski,
Pierre-Louis Bossart, Liam Girdwood, Jie Yang, Mark Brown
Cc: Hans de Goede, linux-wireless, netdev, linux-kernel, alsa-devel,
Ilan Peer
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_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);
rcu_assign_pointer(wiphy->regd, new_regd);
rcu_free_regdom(tmp);
}
--
2.28.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH] cfg80211: Fix "suspicious RCU usage in wiphy_apply_custom_regulatory" warning/backtrace
2021-01-04 17:07 [PATCH] cfg80211: Fix "suspicious RCU usage in wiphy_apply_custom_regulatory" warning/backtrace Hans de Goede
@ 2021-01-05 9:24 ` Peer, Ilan
2021-01-18 21:09 ` Hans de Goede
0 siblings, 1 reply; 5+ messages in thread
From: Peer, Ilan @ 2021-01-05 9:24 UTC (permalink / raw)
To: Hans de Goede, Johannes Berg, David S . Miller, Jakub Kicinski,
Rojewski, Cezary, Pierre-Louis Bossart, Liam Girdwood, Jie Yang,
Mark Brown
Cc: linux-wireless, netdev, linux-kernel, alsa-devel
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);
> rcu_assign_pointer(wiphy->regd, new_regd);
> rcu_free_regdom(tmp);
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.
Thanks for addressing the bug,
Ilan.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] cfg80211: Fix "suspicious RCU usage in wiphy_apply_custom_regulatory" warning/backtrace
2021-01-05 9:24 ` Peer, Ilan
@ 2021-01-18 21:09 ` Hans de Goede
2021-01-19 7:19 ` Peer, Ilan
2021-01-19 15:24 ` Peer, Ilan
0 siblings, 2 replies; 5+ messages in thread
From: Hans de Goede @ 2021-01-18 21:09 UTC (permalink / raw)
To: Peer, Ilan, Johannes Berg, David S . Miller, Jakub Kicinski,
Rojewski, Cezary, Pierre-Louis Bossart, Liam Girdwood, Jie Yang,
Mark Brown
Cc: linux-wireless, netdev, linux-kernel, alsa-devel
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);
>> rcu_assign_pointer(wiphy->regd, new_regd);
>> rcu_free_regdom(tmp);
>
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] cfg80211: Fix "suspicious RCU usage in wiphy_apply_custom_regulatory" warning/backtrace
2021-01-18 21:09 ` Hans de Goede
@ 2021-01-19 7:19 ` Peer, Ilan
2021-01-19 15:24 ` Peer, Ilan
1 sibling, 0 replies; 5+ messages in thread
From: Peer, Ilan @ 2021-01-19 7:19 UTC (permalink / raw)
To: Hans de Goede, Johannes Berg, David S . Miller, Jakub Kicinski,
Rojewski, Cezary, Pierre-Louis Bossart, Liam Girdwood, Jie Yang,
Mark Brown
Cc: linux-wireless, netdev, linux-kernel, alsa-devel
> -----Original Message-----
> From: Hans de Goede <hdegoede@redhat.com>
> Sent: Monday, January 18, 2021 23:09
> To: Peer, Ilan <ilan.peer@intel.com>; 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: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; alsa-devel@alsa-project.org
> Subject: Re: [PATCH] cfg80211: Fix "suspicious RCU usage in
> wiphy_apply_custom_regulatory" warning/backtrace
>
> 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);
> >> rcu_assign_pointer(wiphy->regd, new_regd);
> >> rcu_free_regdom(tmp);
> >
> > 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.
>
Yes. The fix is ready. We'll send it.
Regards,
Ilan.
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] cfg80211: Fix "suspicious RCU usage in wiphy_apply_custom_regulatory" warning/backtrace
2021-01-18 21:09 ` Hans de Goede
2021-01-19 7:19 ` Peer, Ilan
@ 2021-01-19 15:24 ` Peer, Ilan
1 sibling, 0 replies; 5+ messages in thread
From: Peer, Ilan @ 2021-01-19 15:24 UTC (permalink / raw)
To: Hans de Goede, Johannes Berg, David S . Miller, Jakub Kicinski,
Rojewski, Cezary, Pierre-Louis Bossart, Liam Girdwood, Jie Yang,
Mark Brown
Cc: linux-wireless, netdev, linux-kernel, alsa-devel
Hi,
This fix can be found here. It was merged to the mac80211 tree.
https://patchwork.kernel.org/project/linux-wireless/patch/iwlwifi.20210105165657.613e9a876829.Ia38d27dbebea28bf9c56d70691d243186ede70e7@changeid/
Regards,
Ilan.
> -----Original Message-----
> From: Hans de Goede <hdegoede@redhat.com>
> Sent: Monday, January 18, 2021 23:09
> To: Peer, Ilan <ilan.peer@intel.com>; 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: linux-wireless@vger.kernel.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; alsa-devel@alsa-project.org
> Subject: Re: [PATCH] cfg80211: Fix "suspicious RCU usage in
> wiphy_apply_custom_regulatory" warning/backtrace
>
> 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);
> >> rcu_assign_pointer(wiphy->regd, new_regd);
> >> rcu_free_regdom(tmp);
> >
> > 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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-19 15:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-04 17:07 [PATCH] cfg80211: Fix "suspicious RCU usage in wiphy_apply_custom_regulatory" warning/backtrace Hans de Goede
2021-01-05 9:24 ` Peer, Ilan
2021-01-18 21:09 ` Hans de Goede
2021-01-19 7:19 ` Peer, Ilan
2021-01-19 15:24 ` Peer, Ilan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).