linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Luis R. Rodriguez" <lrodriguez@atheros.com>
To: Mark Mentovai <mark@moxienet.com>
Cc: Luis Rodriguez <Luis.Rodriguez@Atheros.com>,
	"linville@tuxdriver.com" <linville@tuxdriver.com>,
	"linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
	"stable@kernel.org" <stable@kernel.org>,
	Johannes Berg <johannes@sipsolutions.net>
Subject: Re: [PATCH] cfg80211: Fix regulatory bug with multiple cards with a delayed CRDA response
Date: Fri, 12 Nov 2010 12:27:32 -0800	[thread overview]
Message-ID: <20101112202732.GI25089@tux> (raw)
In-Reply-To: <AANLkTimKdqoWx2FtvJ9HG3Yj0ofROMDNo30=FJv1vVGG@mail.gmail.com>

On Thu, Nov 11, 2010 at 10:05:21PM -0800, Mark Mentovai wrote:
> Luis R. Rodriguez wrote:
> > When two cards are connected with the same regulatory domain
> > if CRDA had a delayed response then cfg80211's own set regulatory
> > domain would still be the world regulatory domain. There was a bug
> > on cfg80211's ignore_request() when analyzing incoming driver
> > regulatory hints as it was only checking against the currently
> > set cfg80211 regulatory domain and not for any other new
> > pending requests. This is easily fixed by also checking against
> > the pending request.
> 
> Luis, thanks for taking a look at this bug.
> 
> Capsule summary: you’ve overloaded -EALREADY, which until now seemed
> to mean “that’s the regdomain I’m already using,” to now also mean
> “I’m going to be using that regdomain soon but I’m waiting on CRDA.”
> The two cases need to be handled differently.
> 
> In my case, this patch makes things a little bit worse. I tested it
> with compat_wireless 20101110. I’ve included what I see after boot
> below the explanation.
> 
> Your patch changes things so that, according to the steps in my
> original message, steps 3 and 4 become:
> 
> 3. The second card’s driver calls regulatory_hint to provide US as a
> driver hint. ignore_request sees that the last request came from a
> driver and that the current and last request sought to set the same
> hint, so it returns -EALREADY. This triggers the “if the regulatory
> domain being requested by the driver” block, which calls reg_copy_regd
> on the apparent assumption that cfg80211_regdomain contains the
> regdomain that the wiphy actually wants, although it doesn’t, and it’s
> very wrong to do this copy. cfg80211_regdomain is still on 00, because
> CRDA hasn’t responded to the first card’s request yet.
> 
> 4. When CRDA finally responds to the first card’s request from #2, it
> gets plumbed to set_regdom, which calls __set_regdom. __set_regdom
> sees that it’s not intersecting, and enters the block where it would
> normally copy reg_copy_regd to set the wiphy’s regd, but the wiphy it
> uses is the one saved from last_request (in step 3), and it already
> had something copied to it (also in step 3). Since __set_regdom checks
> for this and bails out with -EALREADY in the “userspace could have
> sent two replies with only one kernel request” case. Because
> __set_regdom fails, set_regdom returns early and never makes it to the
> update_all_wiphy_regulatory or print_regdomain steps. The regdomain
> remains unchanged. I’m still stuck at 00.
> 
> What about using something other than -EALREADY to signal “that
> request is already pending?” On its face, this works, but I think
> there’s a deeper flaw that also needs to be addressed. I’m concerned
> that the wiphys that fall into this state won’t see a reg_copy_regd
> call at all. The idea is that any such wiphy shouldn’t really be
> ignored, but it should be joined to a group of wiphys waiting on the
> pending request, and when the request is satisfied, the regd field
> should be populated for each of them.

Thanks for testing and your review.

We need to address a queue of requests with the associated wiphys,
as I originally had developed, I'll go ahead and add this and
treat these, it should take care of even multiple cards with
different regulatory hints. I expect the amount of code will be
a bit to large for stable though so likely we may only be able
to fix this for new kernels.

I'll take a stab at this now.

  Luis

  reply	other threads:[~2010-11-12 20:27 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-12  2:27 [PATCH] cfg80211: Fix regulatory bug with multiple cards with a delayed CRDA response Luis R. Rodriguez
2010-11-12  2:45 ` Luis R. Rodriguez
2010-11-12  2:53   ` Luis R. Rodriguez
2010-11-12  6:05 ` Mark Mentovai
2010-11-12 20:27   ` Luis R. Rodriguez [this message]
2010-11-16  0:06     ` Luis R. Rodriguez
2010-11-16  0:33       ` Luis R. Rodriguez
2010-11-16  3:34       ` Mark Mentovai
2010-11-16 20:27         ` Luis R. Rodriguez
2010-11-16 21:34           ` Mark Mentovai

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=20101112202732.GI25089@tux \
    --to=lrodriguez@atheros.com \
    --cc=Luis.Rodriguez@Atheros.com \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    --cc=mark@moxienet.com \
    --cc=stable@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 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).