All of lore.kernel.org
 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 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.