RE: [PATCH 4.9 094/206] nl80211: Use different attrs for BSSID and random MAC addr in scan req
From: Krishna Vamsi
Date: Fri Jul 21 2017 - 05:40:16 EST
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
>