All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maximilian Luz <luzmaximilian@gmail.com>
To: Brian Norris <briannorris@chromium.org>,
	Johannes Berg <johannes@sipsolutions.net>
Cc: linux-wireless@vger.kernel.org,
	Marek Szyprowski <m.szyprowski@samsung.com>,
	Kalle Valo <kvalo@codeaurora.org>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	netdev@vger.kernel.org, Amitkumar Karwar <amitkarwar@gmail.com>,
	Ganapathi Bhat <ganapathi.bhat@nxp.com>,
	Xinming Hu <huxinming820@gmail.com>
Subject: Re: [BUG] Deadlock in _cfg80211_unregister_wdev()
Date: Sat, 15 May 2021 13:24:42 +0200	[thread overview]
Message-ID: <2dfd6cc1-c69e-c643-f2e9-5d95787b09b2@gmail.com> (raw)
In-Reply-To: <YJ81llg7EKFXUaIo@google.com>

On 5/15/21 4:44 AM, Brian Norris wrote:
> It would seem like _anyone_ that calls cfg80211_unregister_wdev() with
> an interface up will hit this -- not unique to mwifiex. In fact, apart
> from the fact that all his line numbers are wrong, Maximilian's original
> email points out exactly where the deadlock is.
> cfg80211_unregister_wdev() holds the wiphy lock, and the GOING_DOWN
> notification also tries to grab it.
> 
> It does happen that in many other paths, you've already ensured that you
> bring the interface down, so e.g., mac80211 drivers don't tend to hit
> this. But I wouldn't be surprised if a few other cfg80211 drivers hit
> this too.
> 
> The best solution I could figure was to do a similar lock dance done in
> nl80211_del_interface() -- close the netdev without holding the wiphy
> lock. I'll send out a patch shortly.

I believe that if we're going to fix that in the individual drivers,
there should be at least some sort of warning/documentation on
cfg80211_unregister_wdev().

Also someone might want to look at other WiFi drivers calling
cfg80211_unregister_wdev(). For example, I can see a locked call in the
brcm80211 driver, but no previous dev_close() call (see [1]). Haven't
looked in detail though, so I might just be wrong.

I can't help but think that this should maybe be addressed in that
common part instead. I know too little of that subsystem to tell if that
might be infeasible though.

Regards,
Max

[1]: https://elixir.bootlin.com/linux/v5.13-rc1/source/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c#L2445

  reply	other threads:[~2021-05-15 11:25 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-13 23:07 [BUG] Deadlock in _cfg80211_unregister_wdev() Maximilian Luz
2021-05-14  8:26 ` Johannes Berg
2021-05-14 11:40   ` Maximilian Luz
2021-05-15  2:44   ` Brian Norris
2021-05-15 11:24     ` Maximilian Luz [this message]
2021-05-14 13:46 ` Maximilian Luz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2dfd6cc1-c69e-c643-f2e9-5d95787b09b2@gmail.com \
    --to=luzmaximilian@gmail.com \
    --cc=amitkarwar@gmail.com \
    --cc=briannorris@chromium.org \
    --cc=davem@davemloft.net \
    --cc=ganapathi.bhat@nxp.com \
    --cc=huxinming820@gmail.com \
    --cc=johannes@sipsolutions.net \
    --cc=kuba@kernel.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.