Return-path: Received: from wolverine01.qualcomm.com ([199.106.114.254]:38370 "EHLO wolverine01.qualcomm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754303Ab2JIAK6 (ORCPT ); Mon, 8 Oct 2012 20:10:58 -0400 Date: Mon, 8 Oct 2012 17:12:40 -0700 From: "Luis R. Rodriguez" To: Felix Fietkau CC: , , , Subject: Re: [PATCH v2 3.7 2/2] cfg80211: fix initialization of chan->max_reg_power Message-ID: <20121009001240.GM3354@lenteja.do-not-panic.com> (sfid-20121009_021109_024861_72137FB3) References: <1349527254-41080-1-git-send-email-nbd@openwrt.org> <1349527254-41080-2-git-send-email-nbd@openwrt.org> <20121008190156.GK3354@lenteja.do-not-panic.com> <50733F2A.2060306@openwrt.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" In-Reply-To: <50733F2A.2060306@openwrt.org> Sender: linux-wireless-owner@vger.kernel.org List-ID: On Mon, Oct 08, 2012 at 11:01:30PM +0200, Felix Fietkau wrote: > On 2012-10-08 9:01 PM, Luis R. Rodriguez wrote: > > On Sat, Oct 06, 2012 at 02:40:54PM +0200, Felix Fietkau wrote: > >> A few places touch chan->max_power based on updated tx power rules, but > >> forget to do the same to chan->max_reg_power. > >> > >> Signed-off-by: Felix Fietkau > >> Cc: stable@vger.kernel.org > >> --- > >> net/wireless/reg.c | 5 +++-- > >> 1 file changed, 3 insertions(+), 2 deletions(-) > >> > >> diff --git a/net/wireless/reg.c b/net/wireless/reg.c > >> index 3b8cbbc..bcc7d7e 100644 > >> --- a/net/wireless/reg.c > >> +++ b/net/wireless/reg.c > >> @@ -908,7 +908,7 @@ static void handle_channel(struct wiphy *wiphy, > >> map_regdom_flags(reg_rule->flags) | bw_flags; > >> chan->max_antenna_gain = chan->orig_mag = > >> (int) MBI_TO_DBI(power_rule->max_antenna_gain); > >> - chan->max_power = chan->orig_mpwr = > >> + chan->max_reg_power = chan->max_power = chan->orig_mpwr = > >> (int) MBM_TO_DBM(power_rule->max_eirp); > >> return; > >> } > >> @@ -1331,7 +1331,8 @@ static void handle_channel_custom(struct wiphy *wiphy, > >> > >> chan->flags |= map_regdom_flags(reg_rule->flags) | bw_flags; > >> chan->max_antenna_gain = (int) MBI_TO_DBI(power_rule->max_antenna_gain); > >> - chan->max_power = (int) MBM_TO_DBM(power_rule->max_eirp); > >> + chan->max_reg_power = chan->max_power = > >> + (int) MBM_TO_DBM(power_rule->max_eirp); > > > > This looks good to me, good catch! The commit log could use some love, > > given that this is a stable patch it is worthy to describe the > > consequences of not applying this patch. Can you describe what you > > observed that makes this a critical patch? The only piece of code > > that uses max_reg_power in cfg80211, mac80211 or drivers is on > > net/wireless/reg.c and drivers/net/wireless/mwifiex/cfg80211.c. > > In either case the issue the code you are patching is for code > > that deals with drivers that have a custom regulatory domain > > in which orig_mpwr would have been initialized to a non-zero value > > upon registration. In such cases we could only potenially run into > > an issue on this piece of code on handle_channel(): > > > > chan->max_reg_power = (int) MBM_TO_DBM(power_rule->max_eirp); > > if (chan->orig_mpwr) { > > /* > > * Devices that have their own custom regulatory domain > > * but also use WIPHY_FLAG_STRICT_REGULATORY will follow the > > * passed country IE power settings. > > */ > > if (initiator == NL80211_REGDOM_SET_BY_COUNTRY_IE && > > wiphy->flags & WIPHY_FLAG_CUSTOM_REGULATORY && > > wiphy->flags & WIPHY_FLAG_STRICT_REGULATORY) > > chan->max_power = chan->max_reg_power; > > else > > chan->max_power = min(chan->orig_mpwr, > > chan->max_reg_power); > > } else > > chan->max_power = chan->max_reg_power; > > > > The issue would happen if orig_mpwr is non zero (custom) and > > then max_reg_power would not have been initialized. This runs > > when you change a regulatory domain on a card with a custom > > regulatory domain and this would be an issue if max_reg_power > > would not be initialized. This however does not happen due to > > the first line above. > > > > So I agree with this patch but do not see the requirement for > > it to go in as a stable fix to older stable kernels. > OK, I'll extend the commit log and resend without the stable-Cc. Great in that case please feel free to add: Acked-by: Luis R. Rodriguez Thanks! Luis