| Message ID | 20210405205429.19304-2-hdegoede@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | brcmfmac: p2p: Fix recently introduced deadlock issue | expand |
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
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
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);
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(-)