On 5/5/2022 2:24 AM, Johannes Berg wrote:
I don't know why, but this patch seems to cause memory corruption and
various issues when I run the hwsim tests.
Oh.. I ran some hwsim tests from hostap.git to verify the changes but
didn't face any issues. Are you seeing the issues with any specific
hwsim tests? Also, the issues are coming without any of the below
changes also(i.e. with actual patch i sent)?
I did make a few minor changes, but I think those were OK.
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1112,7 +1112,7 @@ struct cfg80211_crypto_settings {
int n_ciphers_pairwise;
u32 ciphers_pairwise[NL80211_MAX_NR_CIPHER_SUITES];
int n_akm_suites;
- u32 akm_suites[NL80211_MAX_NR_AKM_SUITES];
+ u32 *akm_suites;
I made this const, that required a bit of juggling in the wext code, but
it's probably still better overall.
+ /*
+ * Maximum number of AKM suites allowed for NL80211_CMD_CONECT,
+ * NL80211_CMD_ASSOCIATE and NL80211_CMD_START_AP, must be at least
+ * NL80211_MAX_NR_AKM_SUITES to avoid issues with legacy userspace.
+ */
+ if (WARN_ON(wiphy->max_num_akm_suites &&
+ wiphy->max_num_akm_suites < NL80211_MAX_NR_AKM_SUITES))
+ return -EINVAL;
I made it so here if it's 0 we set it to NL80211_MAX_NR_AKM_SUITES, that
way we don't need to have all the questions about it being 0 later.
+ if (nla_put_u16(msg, NL80211_ATTR_MAX_NUM_AKM_SUITES,
+ rdev->wiphy.max_num_akm_suites ?
+ rdev->wiphy.max_num_akm_suites :
+ NL80211_MAX_NR_AKM_SUITES))
+ goto nla_put_failure;
e.g. here, that was then without the ternary operator
+ int max_num_akm_suites = NL80211_MAX_NR_AKM_SUITES;
data = nla_data(info->attrs[NL80211_ATTR_AKM_SUITES]);
len = nla_len(info->attrs[NL80211_ATTR_AKM_SUITES]);
@@ -10261,10 +10269,13 @@ static int nl80211_crypto_settings(struct cfg80211_registered_device *rdev,
if (len % sizeof(u32))
return -EINVAL;
- if (settings->n_akm_suites > NL80211_MAX_NR_AKM_SUITES)
+ if (rdev->wiphy.max_num_akm_suites)
+ max_num_akm_suites = rdev->wiphy.max_num_akm_suites;
+
+ if (settings->n_akm_suites > max_num_akm_suites)
return -EINVAL;
- memcpy(settings->akm_suites, data, len);
+ settings->akm_suites = data;
and all that complexity also goes away
+ if (connect->crypto.n_akm_suites) {
+ wdev->conn->params.crypto.akm_suites =
+ kcalloc(connect->crypto.n_akm_suites, sizeof(u32),
+ GFP_KERNEL);
+ if (!wdev->conn->params.crypto.akm_suites) {
+ cfg80211_sme_free(wdev);
+ return -ENOMEM;
+ }
+
+ wdev->conn->params.crypto.n_akm_suites =
+ connect->crypto.n_akm_suites;
+ memcpy(wdev->conn->params.crypto.akm_suites,
+ connect->crypto.akm_suites,
+ sizeof(u32) * connect->crypto.n_akm_suites);
I made this use kmemdup(), I don't think it makes a big difference.
Anyway, it crashes. Perhaps an invalid free, because the middle of some
netlink packet is being freed or so? >
Please check.
Sure, I will try with suggested changes and update you
johannes
--
veeru
