RE: [PATCH 4.9 094/206] nl80211: Use different attrs for BSSID and random MAC addr in scan req

[Krishna Vamsi]

Hello Greg,

Really sorry that I couldn't find this earlier but there is one issue with this cherry-pick. In "enum nl80211_commands", there are more variables between NL80211_ATTR_NAN_MATCH and NL80211_ATTR_BSSID. But they are missing in this cherry-pick. The missing variables are 
NL80211_ATTR_FILS_KEK, NL80211_ATTR_FILS_NONCES andNL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED. Because of this NL80211_ATTR_BSSID value might not be consistent across different releases. Could you please let me know the next steps on this?

Thanks,
Vamsi

> -----Original Message-----
> From: Greg Kroah-Hartman [mailto:gregkh@xxxxxxxxxxxxxxxxxxx]
> Sent: Tuesday, January 10, 2017 7:06 PM
> To: linux-kernel@xxxxxxxxxxxxxxx
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>;
> stable@xxxxxxxxxxxxxxx; Vamsi, Krishna <vamsin@xxxxxxxxxxxxxxxx>;
> Malinen, Jouni <jouni@xxxxxxxxxxxxxxxx>; Johannes Berg
> <johannes.berg@xxxxxxxxx>
> Subject: [PATCH 4.9 094/206] nl80211: Use different attrs for BSSID and
> random MAC addr in scan req
> 
> 4.9-stable review patch.  If anyone has any objections, please let me know.
> 
> ------------------
> 
> From: Vamsi Krishna <vamsin@xxxxxxxxxxxxxxxx>
> 
> commit 2fa436b3a2a7009c11a3bc03fe0ff4c26e80fd87 upstream.
> 
> NL80211_ATTR_MAC was used to set both the specific BSSID to be scanned and
> the random MAC address to be used when privacy is enabled. When both the
> features are enabled, both the BSSID and the local MAC address were getting
> same value causing Probe Request frames to go with unintended DA. Hence,
> this has been fixed by using a different NL80211_ATTR_BSSID attribute to set
> the specific BSSID (which was the more recent addition in cfg80211) for a scan.
> 
> Backwards compatibility with old userspace software is maintained to some
> extent by allowing NL80211_ATTR_MAC to be used to set the specific BSSID
> when scanning without enabling random MAC address use.
> 
> Scanning with random source MAC address was introduced by commit
> ad2b26abc157 ("cfg80211: allow drivers to support random MAC addresses for
> scan") and the issue was introduced with the addition of the second user for the
> same attribute in commit 818965d39177 ("cfg80211: Allow a scan request for a
> specific BSSID").
> 
> Fixes: 818965d39177 ("cfg80211: Allow a scan request for a specific BSSID")
> Signed-off-by: Vamsi Krishna <vamsin@xxxxxxxxxxxxxxxx>
> Signed-off-by: Jouni Malinen <jouni@xxxxxxxxxxxxxxxx>
> Signed-off-by: Johannes Berg <johannes.berg@xxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> 
> ---
>  include/uapi/linux/nl80211.h |    7 ++++++-
>  net/wireless/nl80211.c       |   16 +++++++++++++++-
>  2 files changed, 21 insertions(+), 2 deletions(-)
> 
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -323,7 +323,7 @@
>   * @NL80211_CMD_GET_SCAN: get scan results
>   * @NL80211_CMD_TRIGGER_SCAN: trigger a new scan with the given
> parameters
>   *	%NL80211_ATTR_TX_NO_CCK_RATE is used to decide whether to send
> the
> - *	probe requests at CCK rate or not. %NL80211_ATTR_MAC can be used
> to
> + *	probe requests at CCK rate or not. %NL80211_ATTR_BSSID can be used
> to
>   *	specify a BSSID to scan for; if not included, the wildcard BSSID will
>   *	be used.
>   * @NL80211_CMD_NEW_SCAN_RESULTS: scan notification (as a reply to @@
> -1937,6 +1937,9 @@ enum nl80211_commands {
>   * @NL80211_ATTR_NAN_MATCH: used to report a match. This is a nested
> attribute.
>   *	See &enum nl80211_nan_match_attributes.
>   *
> + * @NL80211_ATTR_BSSID: The BSSID of the AP. Note that
> %NL80211_ATTR_MAC is also
> + *	used in various commands/events for specifying the BSSID.
> + *
>   * @NUM_NL80211_ATTR: total number of nl80211_attrs available
>   * @NL80211_ATTR_MAX: highest attribute number currently defined
>   * @__NL80211_ATTR_AFTER_LAST: internal use @@ -2336,6 +2339,8 @@
> enum nl80211_attrs {
>  	NL80211_ATTR_NAN_FUNC,
>  	NL80211_ATTR_NAN_MATCH,
> 
> +	NL80211_ATTR_BSSID,
> +
>  	/* add attributes here, update the policy in nl80211.c */
> 
>  	__NL80211_ATTR_AFTER_LAST,
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -414,6 +414,7 @@ static const struct nla_policy nl80211_p
>  	[NL80211_ATTR_NAN_MASTER_PREF] = { .type = NLA_U8 },
>  	[NL80211_ATTR_NAN_DUAL] = { .type = NLA_U8 },
>  	[NL80211_ATTR_NAN_FUNC] = { .type = NLA_NESTED },
> +	[NL80211_ATTR_BSSID] = { .len = ETH_ALEN },
>  };
> 
>  /* policy for the key attributes */
> @@ -6677,7 +6678,20 @@ static int nl80211_trigger_scan(struct s
>  	request->no_cck =
>  		nla_get_flag(info->attrs[NL80211_ATTR_TX_NO_CCK_RATE]);
> 
> -	if (info->attrs[NL80211_ATTR_MAC])
> +	/* Initial implementation used NL80211_ATTR_MAC to set the specific
> +	 * BSSID to scan for. This was problematic because that same attribute
> +	 * was already used for another purpose (local random MAC address).
> The
> +	 * NL80211_ATTR_BSSID attribute was added to fix this. For backwards
> +	 * compatibility with older userspace components, also use the
> +	 * NL80211_ATTR_MAC value here if it can be determined to be used
> for
> +	 * the specific BSSID use case instead of the random MAC address
> +	 * (NL80211_ATTR_SCAN_FLAGS is used to enable random MAC
> address use).
> +	 */
> +	if (info->attrs[NL80211_ATTR_BSSID])
> +		memcpy(request->bssid,
> +		       nla_data(info->attrs[NL80211_ATTR_BSSID]), ETH_ALEN);
> +	else if (!(request->flags & NL80211_SCAN_FLAG_RANDOM_ADDR) &&
> +		 info->attrs[NL80211_ATTR_MAC])
>  		memcpy(request->bssid, nla_data(info-
> >attrs[NL80211_ATTR_MAC]),
>  		       ETH_ALEN);
>  	else
>