linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Arik Nemtsov <arik@wizery.com>
Cc: linux-wireless@vger.kernel.org, "Luis R. Rodriguez" <mcgrof@suse.com>
Subject: Re: [PATCH] cfg80211: fix deadlock during reg chan check
Date: Tue, 06 Jan 2015 11:51:54 +0100	[thread overview]
Message-ID: <1420541514.1966.16.camel@sipsolutions.net> (raw)
In-Reply-To: <1419847199-25493-1-git-send-email-arik@wizery.com> (sfid-20141229_110002_459476_3F75B800)

On Mon, 2014-12-29 at 11:59 +0200, Arik Nemtsov wrote:
> If a P2P GO is active, the cfg80211_reg_can_beacon function will take
> the wdev lock, in its call to cfg80211_go_permissive_chan. But the wdev lock
> is already taken by the parent channel-checking function, causing a
> deadlock.
> Split the checking code into two parts. The first part will check if the
> wdev is active and saves the channel under the wdev lock. The second part
> will check actual channel validity according to type.

I'm not convinced this is the right thing to do. When checking for the
current wdev that it can use a channel, then it seems that it's own
current BSS connection (if any) shouldn't actually be taken into account
- ergo the lock shouldn't have to be taken, that interface should be
excluded from the "can beacon due to concurrent check" anyway.

Also, the only reason this can happen anyway is when you call "can
beacon" for a station interface - which seems nonsensical. Given that
this is now really becoming far more complex than originally designed
("can beacon") with TDLS ("can IR") perhaps this should be
 * renamed to reg_can_ir()
 * passed an "exclude wdev"
 * (and use mutex_lock_nested with a big comment to explain to lockdep
what's
   going on)

or so.

johannes


  reply	other threads:[~2015-01-06 10:51 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-29  9:59 [PATCH] cfg80211: fix deadlock during reg chan check Arik Nemtsov
2015-01-06 10:51 ` Johannes Berg [this message]
2015-01-07 13:34   ` Arik Nemtsov
2015-01-07 13:37     ` Johannes Berg
2015-01-07 13:42       ` Arik Nemtsov
2015-01-07 13:46         ` Johannes Berg
2015-01-07 13:48           ` Arik Nemtsov
2015-01-07 13:50             ` Johannes Berg
2015-01-07 13:52               ` Arik Nemtsov
2015-01-07 13:54 ` Johannes Berg

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=1420541514.1966.16.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=arik@wizery.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=mcgrof@suse.com \
    /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 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).