On 2/9/2024 5:50 AM, Vinayak Yadawad wrote: > In case of SAE authentication offload, the driver would need > the info of SAE DH groups for both STA connection and soft AP. > In the current change we update the SAE DH group support info who is we? <https://www.kernel.org/doc/html/latest/process/submitting-patches.html#describe-your-changes> Describe your changes in imperative mood, e.g. "make xyzzy do frotz" instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy to do frotz", as if you are giving orders to the codebase to change its behaviour. > to driver in the order/priority as provided by the supplicant/ > upper layer. > > Signed-off-by: Vinayak Yadawad <vinayak.yadawad@xxxxxxxxxxxx> > --- > include/net/cfg80211.h | 6 ++++++ > include/uapi/linux/nl80211.h | 7 +++++++ > net/wireless/nl80211.c | 22 ++++++++++++++++++++++ > 3 files changed, 35 insertions(+) > > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h > index 5b42bfc..0b2db0d 100644 > --- a/include/net/cfg80211.h > +++ b/include/net/cfg80211.h > @@ -1194,6 +1194,7 @@ struct survey_info { > }; > > #define CFG80211_MAX_NUM_AKM_SUITES 10 > +#define CFG80211_MAX_NUM_SAE_DH_GROUPS 10 > > /** > * struct cfg80211_crypto_settings - Crypto settings > @@ -1235,6 +1236,9 @@ struct survey_info { > * > * NL80211_SAE_PWE_BOTH > * Allow either hunting-and-pecking loop or hash-to-element > + * > + * @sae_dh_groups: SAE DH groups in preference order. > + * @n_sae_dhd_groups: No of SAE DH groups assigned. > */ > struct cfg80211_crypto_settings { > u32 wpa_versions; > @@ -1252,6 +1256,8 @@ struct cfg80211_crypto_settings { > const u8 *sae_pwd; > u8 sae_pwd_len; > enum nl80211_sae_pwe_mechanism sae_pwe; > + u32 sae_dh_groups[CFG80211_MAX_NUM_SAE_DH_GROUPS]; > + u8 n_sae_dh_groups; > }; > > /** > diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h > index 853ac53..7c1a7b4 100644 > --- a/include/uapi/linux/nl80211.h > +++ b/include/uapi/linux/nl80211.h > @@ -2855,6 +2855,11 @@ enum nl80211_commands { > * %NL80211_CMD_ASSOCIATE indicating the SPP A-MSDUs > * are used on this connection > * > + * @NL80211_ATTR_SAE_DH_GROUPS: Attribute to indicate the supported SAE DH suggest you be more specific and say this is a nest attribute containing zero or more u32 attributes containing a DH Group number > + * groups to driver. For STA role, the dh groups should be tried in the s/dh/DH/ > + * indicated order. For AP role, the order does not have any specific > + * significance. > + * > * @NUM_NL80211_ATTR: total number of nl80211_attrs available > * @NL80211_ATTR_MAX: highest attribute number currently defined > * @__NL80211_ATTR_AFTER_LAST: internal use > @@ -3400,6 +3405,8 @@ enum nl80211_attrs { > > NL80211_ATTR_ASSOC_SPP_AMSDU, > > + NL80211_ATTR_SAE_DH_GROUPS, > + > /* add attributes here, update the policy in nl80211.c */ > > __NL80211_ATTR_AFTER_LAST, > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c > index 68c2040..555eb0f 100644 > --- a/net/wireless/nl80211.c > +++ b/net/wireless/nl80211.c > @@ -826,6 +826,7 @@ static int validate_he_capa(const struct nlattr *attr, > [NL80211_ATTR_MLO_TTLM_DLINK] = NLA_POLICY_EXACT_LEN(sizeof(u16) * 8), > [NL80211_ATTR_MLO_TTLM_ULINK] = NLA_POLICY_EXACT_LEN(sizeof(u16) * 8), > [NL80211_ATTR_ASSOC_SPP_AMSDU] = { .type = NLA_FLAG }, > + [NL80211_ATTR_SAE_DH_GROUPS] = { .type = NLA_NESTED }, can/should this have a policy? not sure it can have a policy, so see below... > }; > > /* policy for the key attributes */ > @@ -10883,6 +10884,27 @@ static int nl80211_crypto_settings(struct cfg80211_registered_device *rdev, > else > settings->sae_pwe = NL80211_SAE_PWE_UNSPECIFIED; > > + if (info->attrs[NL80211_ATTR_SAE_DH_GROUPS]) { > + struct nlattr *dh_group; > + int tmp, i = 0; > + > + if (!wiphy_ext_feature_isset(&rdev->wiphy, > + NL80211_EXT_FEATURE_SAE_OFFLOAD) && > + !wiphy_ext_feature_isset(&rdev->wiphy, > + NL80211_EXT_FEATURE_SAE_OFFLOAD_AP)) > + return -EINVAL; > + > + nla_for_each_nested(dh_group, info->attrs[NL80211_ATTR_SAE_DH_GROUPS], > + tmp) { > + settings->sae_dh_groups[i] = nla_get_u32(dh_group); ...note that since you don't have a policy to ensure that each attribute actually has 4 bytes of data, this can result in a buffer overread if userspace provides less than 4 bytes of payload. perhaps add logic like the following (borrowed from validate_scan_freqs(): if (nla_len(dh_group) != sizeof(u32)) return -EINVAL; also if it passes the u32 size test, don't you also want to check each value to make sure it is a valid DH Group #? of course, this begs the question of whether cfg80211 or the individual drivers should validate the DH Group values. if we don't validate in cfg80211 then perhaps add to the documentation of @sae_dh_groups that the values have not been validated > + i++; > + > + if (i == CFG80211_MAX_NUM_SAE_DH_GROUPS) > + break; > + } > + settings->n_sae_dh_groups = i; > + } > + > return 0; > } >
