[PATCH 5.12 regression fix resend 0/1] brcmfmac: p2p: Fix recently introduced deadlock issue

[PATCH 5.12 regression fix resend 0/1] brcmfmac: p2p: Fix recently introduced deadlock issue

[Hans de Goede]

Hi,

Here is a resend of a regression/deadlock fix for 5.12 which I send out
around rc1 but which still is not in Linus' tree.

This is quite a bad bug, breaking things for most broadcom wifi users.

Can we get this regression fix on its way to Linus for 5.12-rc7 ASAP
please ?

Regards,

Hans



Hans de Goede (1):
  brcmfmac: p2p: Fix recently introduced deadlock issue

 drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.30.2

[PATCH 5.12 regression fix resend 1/1] brcmfmac: p2p: Fix recently introduced deadlock issue

[Hans de Goede]

Commit a05829a7222e ("cfg80211: avoid holding the RTNL when calling the
driver") replaced the rtnl_lock parameter passed to various brcmf
functions with just lock, because since that commit it is not just
about the rtnl_lock but also about the wiphy_lock .

During this search/replace the "if (!rtnl_locked)" check in brcmfmac/p2p.c
was accidentally replaced with "if (locked)", dropping the inversion of
the check. This causes the code to now call rtnl_lock() while already
holding the lock, causing a deadlock.

Add back the "!" to the if-condition to fix this.

Cc: Johannes Berg <johannes.berg@intel.com>
Fixes: a05829a7222e ("cfg80211: avoid holding the RTNL when calling the driver")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index 6d30a0fcecea..34cd8a7401fe 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -2439,7 +2439,7 @@ void brcmf_p2p_ifp_removed(struct brcmf_if *ifp, bool locked)
 	vif = ifp->vif;
 	cfg = wdev_to_cfg(&vif->wdev);
 	cfg->p2p.bss_idx[P2PAPI_BSSCFG_DEVICE].vif = NULL;
-	if (locked) {
+	if (!locked) {
 		rtnl_lock();
 		wiphy_lock(cfg->wiphy);
 		cfg80211_unregister_wdev(&vif->wdev);
-- 
2.30.2

Re: [PATCH 5.12 regression fix resend 1/1] brcmfmac: p2p: Fix recently introduced deadlock issue

[Kalle Valo]

Hans de Goede <hdegoede@redhat.com> wrote:

> Commit a05829a7222e ("cfg80211: avoid holding the RTNL when calling the
> driver") replaced the rtnl_lock parameter passed to various brcmf
> functions with just lock, because since that commit it is not just
> about the rtnl_lock but also about the wiphy_lock .
> 
> During this search/replace the "if (!rtnl_locked)" check in brcmfmac/p2p.c
> was accidentally replaced with "if (locked)", dropping the inversion of
> the check. This causes the code to now call rtnl_lock() while already
> holding the lock, causing a deadlock.
> 
> Add back the "!" to the if-condition to fix this.
> 
> Cc: Johannes Berg <johannes.berg@intel.com>
> Fixes: a05829a7222e ("cfg80211: avoid holding the RTNL when calling the driver")
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

This is already applied:

https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git/commit/?id=bd83a2fc05ed323d7ae38443a6d2e253ef1c4237

Patch set to Superseded.

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20210405205429.19304-2-hdegoede@redhat.com/

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

Re: [PATCH 5.12 regression fix resend 1/1] brcmfmac: p2p: Fix recently introduced deadlock issue

[Hans de Goede]

Hi,

On 4/6/21 12:17 PM, Kalle Valo wrote:
> Hans de Goede <hdegoede@redhat.com> wrote:
> 
>> Commit a05829a7222e ("cfg80211: avoid holding the RTNL when calling the
>> driver") replaced the rtnl_lock parameter passed to various brcmf
>> functions with just lock, because since that commit it is not just
>> about the rtnl_lock but also about the wiphy_lock .
>>
>> During this search/replace the "if (!rtnl_locked)" check in brcmfmac/p2p.c
>> was accidentally replaced with "if (locked)", dropping the inversion of
>> the check. This causes the code to now call rtnl_lock() while already
>> holding the lock, causing a deadlock.
>>
>> Add back the "!" to the if-condition to fix this.
>>
>> Cc: Johannes Berg <johannes.berg@intel.com>
>> Fixes: a05829a7222e ("cfg80211: avoid holding the RTNL when calling the driver")
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> 
> This is already applied:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git/commit/?id=bd83a2fc05ed323d7ae38443a6d2e253ef1c4237

But it is not yet in 5.12-rc6, even though this is a regression fix for a
regression introduced in 5.12. What is the plan to get this into 5.12 ?

Regards,

Hans

Re: [PATCH 5.12 regression fix resend 0/1] brcmfmac: p2p: Fix recently introduced deadlock issue

[Kalle Valo]

Hans de Goede <hdegoede@redhat.com> writes:

> Hi,
>
> Here is a resend of a regression/deadlock fix for 5.12 which I send out
> around rc1 but which still is not in Linus' tree.
>
> This is quite a bad bug, breaking things for most broadcom wifi users.
>
> Can we get this regression fix on its way to Linus for 5.12-rc7 ASAP
> please ?

The patch has been already applied but I have not yet sent a pull
request, the plan is to send it later this week.

Please don't resend patches which have been already applied, that
creates more work for me.

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

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