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
next prev parent 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).