Re: [PATCH AUTOSEL 4.4 05/25] cfg80211: Address some corner cases in scan result channel updating

Greg KH
2018-11-02T09:19:59Z

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@codeaurora.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@codeaurora.org>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> Signed-off-by: Sasha Levin <sashal@kernel.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
>