Date:   Fri, 2 Nov 2018 10:19:54 +0100
From:   Greg KH <greg@...ah.com>
To:     Sasha Levin <sashal@...nel.org>
Cc:     stable@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jouni Malinen <jouni@...eaurora.org>,
        Johannes Berg <johannes.berg@...el.com>
Subject: Re: [PATCH AUTOSEL 4.4 05/25] cfg80211: Address some corner cases in
 scan result channel updating

Sasha,

The patch below causes a build warning:
net/wireless/scan.c: In function ‘cfg80211_get_bss_channel’:
net/wireless/scan.c:1015:21: warning: comparison between ‘enum ieee80211_band’ and ‘enum nl80211_band’ [-Wenum-compare]
   if (channel->band == NL80211_BAND_2GHZ) {
                     ^~

So I'm going to drop it from the tree now.  Feel free to resubmit it if
you want to fix it up :)

thanks,

greg k-h


On Tue, Oct 16, 2018 at 12:15:46AM -0400, Sasha Levin wrote:
> From: Jouni Malinen <jouni@...eaurora.org>
> 
> [ Upstream commit 119f94a6fefcc76d47075b83d2b73d04c895df78 ]
> 
> cfg80211_get_bss_channel() is used to update the RX channel based on the
> available frame payload information (channel number from DSSS Parameter
> Set element or HT Operation element). This is needed on 2.4 GHz channels
> where frames may be received on neighboring channels due to overlapping
> frequency range.
> 
> This might of some use on the 5 GHz band in some corner cases, but
> things are more complex there since there is no n:1 or 1:n mapping
> between channel numbers and frequencies due to multiple different
> starting frequencies in different operating classes. This could result
> in ieee80211_channel_to_frequency() returning incorrect frequency and
> ieee80211_get_channel() returning incorrect channel information (or
> indication of no match). In the previous implementation, this could
> result in some scan results being dropped completely, e.g., for the 4.9
> GHz channels. That prevented connection to such BSSs.
> 
> Fix this by using the driver-provided channel pointer if
> ieee80211_get_channel() does not find matching channel data for the
> channel number in the frame payload and if the scan is done with 5 MHz
> or 10 MHz channel bandwidth. While doing this, also add comments
> describing what the function is trying to achieve to make it easier to
> understand what happens here and why.
> 
> Signed-off-by: Jouni Malinen <jouni@...eaurora.org>
> Signed-off-by: Johannes Berg <johannes.berg@...el.com>
> Signed-off-by: Sasha Levin <sashal@...nel.org>
> ---
>  net/wireless/scan.c | 58 ++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 49 insertions(+), 9 deletions(-)
> 
> diff --git a/net/wireless/scan.c b/net/wireless/scan.c
> index 8dde12a11725..00219f386283 100644
> --- a/net/wireless/scan.c
> +++ b/net/wireless/scan.c
> @@ -974,13 +974,23 @@ cfg80211_bss_update(struct cfg80211_registered_device *rdev,
>  	return NULL;
>  }
>  
> +/*
> + * Update RX channel information based on the available frame payload
> + * information. This is mainly for the 2.4 GHz band where frames can be received
> + * from neighboring channels and the Beacon frames use the DSSS Parameter Set
> + * element to indicate the current (transmitting) channel, but this might also
> + * be needed on other bands if RX frequency does not match with the actual
> + * operating channel of a BSS.
> + */
>  static struct ieee80211_channel *
>  cfg80211_get_bss_channel(struct wiphy *wiphy, const u8 *ie, size_t ielen,
> -			 struct ieee80211_channel *channel)
> +			 struct ieee80211_channel *channel,
> +			 enum nl80211_bss_scan_width scan_width)
>  {
>  	const u8 *tmp;
>  	u32 freq;
>  	int channel_number = -1;
> +	struct ieee80211_channel *alt_channel;
>  
>  	tmp = cfg80211_find_ie(WLAN_EID_DS_PARAMS, ie, ielen);
>  	if (tmp && tmp[1] == 1) {
> @@ -994,16 +1004,45 @@ cfg80211_get_bss_channel(struct wiphy *wiphy, const u8 *ie, size_t ielen,
>  		}
>  	}
>  
> -	if (channel_number < 0)
> +	if (channel_number < 0) {
> +		/* No channel information in frame payload */
>  		return channel;
> +	}
>  
>  	freq = ieee80211_channel_to_frequency(channel_number, channel->band);
> -	channel = ieee80211_get_channel(wiphy, freq);
> -	if (!channel)
> -		return NULL;
> -	if (channel->flags & IEEE80211_CHAN_DISABLED)
> +	alt_channel = ieee80211_get_channel(wiphy, freq);
> +	if (!alt_channel) {
> +		if (channel->band == NL80211_BAND_2GHZ) {
> +			/*
> +			 * Better not allow unexpected channels when that could
> +			 * be going beyond the 1-11 range (e.g., discovering
> +			 * BSS on channel 12 when radio is configured for
> +			 * channel 11.
> +			 */
> +			return NULL;
> +		}
> +
> +		/* No match for the payload channel number - ignore it */
> +		return channel;
> +	}
> +
> +	if (scan_width == NL80211_BSS_CHAN_WIDTH_10 ||
> +	    scan_width == NL80211_BSS_CHAN_WIDTH_5) {
> +		/*
> +		 * Ignore channel number in 5 and 10 MHz channels where there
> +		 * may not be an n:1 or 1:n mapping between frequencies and
> +		 * channel numbers.
> +		 */
> +		return channel;
> +	}
> +
> +	/*
> +	 * Use the channel determined through the payload channel number
> +	 * instead of the RX channel reported by the driver.
> +	 */
> +	if (alt_channel->flags & IEEE80211_CHAN_DISABLED)
>  		return NULL;
> -	return channel;
> +	return alt_channel;
>  }
>  
>  /* Returned bss is reference counted and must be cleaned up appropriately. */
> @@ -1028,7 +1067,8 @@ cfg80211_inform_bss_data(struct wiphy *wiphy,
>  		    (data->signal < 0 || data->signal > 100)))
>  		return NULL;
>  
> -	channel = cfg80211_get_bss_channel(wiphy, ie, ielen, data->chan);
> +	channel = cfg80211_get_bss_channel(wiphy, ie, ielen, data->chan,
> +					   data->scan_width);
>  	if (!channel)
>  		return NULL;
>  
> @@ -1126,7 +1166,7 @@ cfg80211_inform_bss_frame_data(struct wiphy *wiphy,
>  		return NULL;
>  
>  	channel = cfg80211_get_bss_channel(wiphy, mgmt->u.beacon.variable,
> -					   ielen, data->chan);
> +					   ielen, data->chan, data->scan_width);
>  	if (!channel)
>  		return NULL;
>  
> -- 
> 2.17.1
> 

Powered by blists - more mailing lists