From: Johannes Berg <[email protected]>
Without further bloating the policy structs, we can overload
the `validation_data' pointer with a struct of s16 min, max
and use those to validate ranges in NLA_{U,S}{8,16,32,64}
attributes.
It may sound strange to validate NLA_U32 with a s16 max, but
in many cases NLA_U32 is used for enums etc. since there's no
size benefit in using a smaller attribute width anyway, due
to netlink attribute alignment; in cases like that it's still
useful, particularly when the attribute really transports an
enum value.
Doing so lets us remove quite a bit of validation code, if we
can be sure that these attributes aren't used by userspace in
places where they're ignored today.
Signed-off-by: Johannes Berg <[email protected]>
---
include/net/netlink.h | 25 +++++++++++++++++++++++--
lib/nlattr.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 72 insertions(+), 2 deletions(-)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index 3698ca8ff92c..ddabc832febc 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -238,7 +238,23 @@ enum {
* nested attributes directly inside, while an array has
* the nested attributes at another level down and the
* attributes directly in the nesting don't matter.
- * All other Unused
+ * All other Unused - but note that it's a union
+ *
+ * Meaning of `min' and `max' fields:
+ * NLA_U8,
+ * NLA_U16,
+ * NLA_U32,
+ * NLA_U64,
+ * NLA_S8,
+ * NLA_S16,
+ * NLA_S32,
+ * NLA_S64 If at least one of them is non-zero, they are the
+ * minimum and maximum value accepted for the attribute;
+ * note that in the interest of code simplicity and
+ * struct size both limits are s16, so you cannot
+ * enforce a range that doesn't fall within the range
+ * of s16 - do that as usual in the code instead.
+ * All other Unused - but note that it's a union
*
* Example:
* static const struct nla_policy my_policy[ATTR_MAX+1] = {
@@ -251,7 +267,12 @@ enum {
struct nla_policy {
u16 type;
u16 len;
- const void *validation_data;
+ union {
+ const void *validation_data;
+ struct {
+ s16 min, max;
+ };
+ };
};
#define NLA_POLICY_EXACT_LEN(_len) { .type = NLA_EXACT_LEN, .len = _len }
diff --git a/lib/nlattr.c b/lib/nlattr.c
index 2f8feff669a7..dd8d34c1ae19 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -230,6 +230,55 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
goto out_err;
}
+ /* validate range */
+ if (pt->min || pt->max) {
+ s64 value;
+ bool validate = true;
+
+ switch (pt->type) {
+ case NLA_U8:
+ value = nla_get_u8(nla);
+ break;
+ case NLA_U16:
+ value = nla_get_u16(nla);
+ break;
+ case NLA_U32:
+ value = nla_get_u32(nla);
+ break;
+ case NLA_S8:
+ value = nla_get_s8(nla);
+ break;
+ case NLA_S16:
+ value = nla_get_s16(nla);
+ break;
+ case NLA_S32:
+ value = nla_get_s32(nla);
+ break;
+ case NLA_S64:
+ value = nla_get_s64(nla);
+ break;
+ case NLA_U64:
+ /* treat this one specially, since it may not fit into s64 */
+ if (nla_get_u64(nla) < pt->min ||
+ nla_get_u64(nla) > pt->max) {
+ NL_SET_ERR_MSG_ATTR(extack, nla,
+ "integer out of range");
+ return -ERANGE;
+ }
+ /* fall through */
+ default:
+ /* no further validation */
+ validate = false;
+ break;
+ }
+
+ if (validate && (value < pt->min || value > pt->max)) {
+ NL_SET_ERR_MSG_ATTR(extack, nla,
+ "integer out of range");
+ return -ERANGE;
+ }
+ }
+
return 0;
out_err:
NL_SET_ERR_MSG_ATTR(extack, nla, "Attribute failed policy validation");
--
2.14.4
From: Johannes Berg <[email protected]>
Many range checks can be done in the policy, move them
there. A few in mesh are added in the code (taken out of
the macros) because they don't fit into the s16 range in
the policy validation.
Signed-off-by: Johannes Berg <[email protected]>
---
net/wireless/nl80211.c | 493 +++++++++++++++++++++----------------------------
1 file changed, 211 insertions(+), 282 deletions(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index e9550c5f5871..e3923c1cb5a5 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -265,14 +265,14 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_CENTER_FREQ1] = { .type = NLA_U32 },
[NL80211_ATTR_CENTER_FREQ2] = { .type = NLA_U32 },
- [NL80211_ATTR_WIPHY_RETRY_SHORT] = { .type = NLA_U8 },
- [NL80211_ATTR_WIPHY_RETRY_LONG] = { .type = NLA_U8 },
+ [NL80211_ATTR_WIPHY_RETRY_SHORT] = { .type = NLA_U8, .min = 1 },
+ [NL80211_ATTR_WIPHY_RETRY_LONG] = { .type = NLA_U8, .min = 1 },
[NL80211_ATTR_WIPHY_FRAG_THRESHOLD] = { .type = NLA_U32 },
[NL80211_ATTR_WIPHY_RTS_THRESHOLD] = { .type = NLA_U32 },
[NL80211_ATTR_WIPHY_COVERAGE_CLASS] = { .type = NLA_U8 },
[NL80211_ATTR_WIPHY_DYN_ACK] = { .type = NLA_FLAG },
- [NL80211_ATTR_IFTYPE] = { .type = NLA_U32 },
+ [NL80211_ATTR_IFTYPE] = { .type = NLA_U32, .max = NL80211_IFTYPE_MAX },
[NL80211_ATTR_IFINDEX] = { .type = NLA_U32 },
[NL80211_ATTR_IFNAME] = { .type = NLA_NUL_STRING, .len = IFNAMSIZ-1 },
@@ -282,11 +282,12 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_KEY] = { .type = NLA_NESTED, },
[NL80211_ATTR_KEY_DATA] = { .type = NLA_BINARY,
.len = WLAN_MAX_KEY_LEN },
- [NL80211_ATTR_KEY_IDX] = { .type = NLA_U8 },
+ [NL80211_ATTR_KEY_IDX] = { .type = NLA_U8, .max = 5 },
[NL80211_ATTR_KEY_CIPHER] = { .type = NLA_U32 },
[NL80211_ATTR_KEY_DEFAULT] = { .type = NLA_FLAG },
[NL80211_ATTR_KEY_SEQ] = { .type = NLA_BINARY, .len = 16 },
- [NL80211_ATTR_KEY_TYPE] = { .type = NLA_U32 },
+ [NL80211_ATTR_KEY_TYPE] = { .type = NLA_U32,
+ .max = NUM_NL80211_KEYTYPES - 1 },
[NL80211_ATTR_BEACON_INTERVAL] = { .type = NLA_U32 },
[NL80211_ATTR_DTIM_PERIOD] = { .type = NLA_U32 },
@@ -294,12 +295,19 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
.len = IEEE80211_MAX_DATA_LEN },
[NL80211_ATTR_BEACON_TAIL] = { .type = NLA_BINARY,
.len = IEEE80211_MAX_DATA_LEN },
- [NL80211_ATTR_STA_AID] = { .type = NLA_U16 },
+ [NL80211_ATTR_STA_AID] = {
+ .type = NLA_U16,
+ .min = 1,
+ .max = IEEE80211_MAX_AID,
+ },
[NL80211_ATTR_STA_FLAGS] = { .type = NLA_NESTED },
[NL80211_ATTR_STA_LISTEN_INTERVAL] = { .type = NLA_U16 },
[NL80211_ATTR_STA_SUPPORTED_RATES] = { .type = NLA_BINARY,
.len = NL80211_MAX_SUPP_RATES },
- [NL80211_ATTR_STA_PLINK_ACTION] = { .type = NLA_U8 },
+ [NL80211_ATTR_STA_PLINK_ACTION] = {
+ .type = NLA_U8,
+ .max = NUM_NL80211_PLINK_ACTIONS - 1
+ },
[NL80211_ATTR_STA_VLAN] = { .type = NLA_U32 },
[NL80211_ATTR_MNTR_FLAGS] = { /* NLA_NESTED can't be empty */ },
[NL80211_ATTR_MESH_ID] = { .type = NLA_BINARY,
@@ -333,7 +341,11 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_REASON_CODE] = { .type = NLA_U16 },
[NL80211_ATTR_FREQ_FIXED] = { .type = NLA_FLAG },
[NL80211_ATTR_TIMED_OUT] = { .type = NLA_FLAG },
- [NL80211_ATTR_USE_MFP] = { .type = NLA_U32 },
+ [NL80211_ATTR_USE_MFP] = {
+ .type = NLA_U32,
+ .min = NL80211_MFP_NO,
+ .max = NL80211_MFP_OPTIONAL,
+ },
[NL80211_ATTR_STA_FLAGS2] = {
.len = sizeof(struct nl80211_sta_flag_update),
},
@@ -353,7 +365,11 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_FRAME] = { .type = NLA_BINARY,
.len = IEEE80211_MAX_DATA_LEN },
[NL80211_ATTR_FRAME_MATCH] = { .type = NLA_BINARY, },
- [NL80211_ATTR_PS_STATE] = { .type = NLA_U32 },
+ [NL80211_ATTR_PS_STATE] = {
+ .type = NLA_U32,
+ .min = NL80211_PS_DISABLED,
+ .max = NL80211_PS_ENABLED,
+ },
[NL80211_ATTR_CQM] = { .type = NLA_NESTED, },
[NL80211_ATTR_LOCAL_STATE_CHANGE] = { .type = NLA_FLAG },
[NL80211_ATTR_AP_ISOLATE] = { .type = NLA_U8 },
@@ -366,11 +382,23 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_OFFCHANNEL_TX_OK] = { .type = NLA_FLAG },
[NL80211_ATTR_KEY_DEFAULT_TYPES] = { .type = NLA_NESTED },
[NL80211_ATTR_WOWLAN_TRIGGERS] = { .type = NLA_NESTED },
- [NL80211_ATTR_STA_PLINK_STATE] = { .type = NLA_U8 },
+ [NL80211_ATTR_STA_PLINK_STATE] = {
+ .type = NLA_U8,
+ .max = NUM_NL80211_PLINK_STATES - 1
+ },
+ [NL80211_ATTR_MESH_PEER_AID] = {
+ .type = NLA_U16,
+ .min = 1,
+ .max = IEEE80211_MAX_AID,
+ },
[NL80211_ATTR_SCHED_SCAN_INTERVAL] = { .type = NLA_U32 },
[NL80211_ATTR_REKEY_DATA] = { .type = NLA_NESTED },
[NL80211_ATTR_SCAN_SUPP_RATES] = { .type = NLA_NESTED },
- [NL80211_ATTR_HIDDEN_SSID] = { .type = NLA_U32 },
+ [NL80211_ATTR_HIDDEN_SSID] = {
+ .type = NLA_U32,
+ .min = NL80211_HIDDEN_SSID_NOT_IN_USE,
+ .max = NL80211_HIDDEN_SSID_ZERO_CONTENTS,
+ },
[NL80211_ATTR_IE_PROBE_RESP] = { .type = NLA_BINARY,
.len = IEEE80211_MAX_DATA_LEN },
[NL80211_ATTR_IE_ASSOC_RESP] = { .type = NLA_BINARY,
@@ -400,9 +428,13 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_AUTH_DATA] = { .type = NLA_BINARY, },
[NL80211_ATTR_VHT_CAPABILITY] = { .len = NL80211_VHT_CAPABILITY_LEN },
[NL80211_ATTR_SCAN_FLAGS] = { .type = NLA_U32 },
- [NL80211_ATTR_P2P_CTWINDOW] = { .type = NLA_U8 },
- [NL80211_ATTR_P2P_OPPPS] = { .type = NLA_U8 },
- [NL80211_ATTR_LOCAL_MESH_POWER_MODE] = {. type = NLA_U32 },
+ [NL80211_ATTR_P2P_CTWINDOW] = { .type = NLA_U8, .max = 127 },
+ [NL80211_ATTR_P2P_OPPPS] = { .type = NLA_U8, .max = 1 },
+ [NL80211_ATTR_LOCAL_MESH_POWER_MODE] = {
+ .type = NLA_U32,
+ .min = NL80211_MESH_POWER_UNKNOWN + 1,
+ .max = NL80211_MESH_POWER_MAX,
+ },
[NL80211_ATTR_ACL_POLICY] = {. type = NLA_U32 },
[NL80211_ATTR_MAC_ADDRS] = { .type = NLA_NESTED },
[NL80211_ATTR_STA_CAPABILITY] = { .type = NLA_U16 },
@@ -415,7 +447,11 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_MDID] = { .type = NLA_U16 },
[NL80211_ATTR_IE_RIC] = { .type = NLA_BINARY,
.len = IEEE80211_MAX_DATA_LEN },
- [NL80211_ATTR_PEER_AID] = { .type = NLA_U16 },
+ [NL80211_ATTR_PEER_AID] = {
+ .type = NLA_U16,
+ .min = 1,
+ .max = IEEE80211_MAX_AID,
+ },
[NL80211_ATTR_CH_SWITCH_COUNT] = { .type = NLA_U32 },
[NL80211_ATTR_CH_SWITCH_BLOCK_TX] = { .type = NLA_FLAG },
[NL80211_ATTR_CSA_IES] = { .type = NLA_NESTED },
@@ -436,8 +472,11 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_SOCKET_OWNER] = { .type = NLA_FLAG },
[NL80211_ATTR_CSA_C_OFFSETS_TX] = { .type = NLA_BINARY },
[NL80211_ATTR_USE_RRM] = { .type = NLA_FLAG },
- [NL80211_ATTR_TSID] = { .type = NLA_U8 },
- [NL80211_ATTR_USER_PRIO] = { .type = NLA_U8 },
+ [NL80211_ATTR_TSID] = { .type = NLA_U8, .max = IEEE80211_NUM_TIDS - 1 },
+ [NL80211_ATTR_USER_PRIO] = {
+ .type = NLA_U8,
+ .max = IEEE80211_NUM_UPS - 1,
+ },
[NL80211_ATTR_ADMITTED_TIME] = { .type = NLA_U16 },
[NL80211_ATTR_SMPS_MODE] = { .type = NLA_U8 },
[NL80211_ATTR_MAC_MASK] = { .len = ETH_ALEN },
@@ -447,12 +486,15 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_REG_INDOOR] = { .type = NLA_FLAG },
[NL80211_ATTR_PBSS] = { .type = NLA_FLAG },
[NL80211_ATTR_BSS_SELECT] = { .type = NLA_NESTED },
- [NL80211_ATTR_STA_SUPPORT_P2P_PS] = { .type = NLA_U8 },
+ [NL80211_ATTR_STA_SUPPORT_P2P_PS] = {
+ .type = NLA_U8,
+ .max = NUM_NL80211_P2P_PS_STATUS - 1
+ },
[NL80211_ATTR_MU_MIMO_GROUP_DATA] = {
.len = VHT_MUMIMO_GROUPS_DATA_LEN
},
[NL80211_ATTR_MU_MIMO_FOLLOW_MAC_ADDR] = { .len = ETH_ALEN },
- [NL80211_ATTR_NAN_MASTER_PREF] = { .type = NLA_U8 },
+ [NL80211_ATTR_NAN_MASTER_PREF] = { .type = NLA_U8, .min = 1 },
[NL80211_ATTR_BANDS] = { .type = NLA_U32 },
[NL80211_ATTR_NAN_FUNC] = { .type = NLA_NESTED },
[NL80211_ATTR_FILS_KEK] = { .type = NLA_BINARY,
@@ -496,7 +538,7 @@ static const struct nla_policy nl80211_key_policy[NL80211_KEY_MAX + 1] = {
[NL80211_KEY_SEQ] = { .type = NLA_BINARY, .len = 16 },
[NL80211_KEY_DEFAULT] = { .type = NLA_FLAG },
[NL80211_KEY_DEFAULT_MGMT] = { .type = NLA_FLAG },
- [NL80211_KEY_TYPE] = { .type = NLA_U32 },
+ [NL80211_KEY_TYPE] = { .type = NLA_U32, .max = NUM_NL80211_KEYTYPES - 1 },
[NL80211_KEY_DEFAULT_TYPES] = { .type = NLA_NESTED },
};
@@ -547,7 +589,11 @@ nl80211_wowlan_tcp_policy[NUM_NL80211_WOWLAN_TCP] = {
static const struct nla_policy
nl80211_coalesce_policy[NUM_NL80211_ATTR_COALESCE_RULE] = {
[NL80211_ATTR_COALESCE_RULE_DELAY] = { .type = NLA_U32 },
- [NL80211_ATTR_COALESCE_RULE_CONDITION] = { .type = NLA_U32 },
+ [NL80211_ATTR_COALESCE_RULE_CONDITION] = {
+ .type = NLA_U32,
+ .min = NL80211_COALESCE_CONDITION_MATCH,
+ .max = NL80211_COALESCE_CONDITION_NO_MATCH,
+ },
[NL80211_ATTR_COALESCE_RULE_PKT_PATTERN] = { .type = NLA_NESTED },
};
@@ -914,12 +960,8 @@ static int nl80211_parse_key_new(struct genl_info *info, struct nlattr *key,
if (tb[NL80211_KEY_CIPHER])
k->p.cipher = nla_get_u32(tb[NL80211_KEY_CIPHER]);
- if (tb[NL80211_KEY_TYPE]) {
+ if (tb[NL80211_KEY_TYPE])
k->type = nla_get_u32(tb[NL80211_KEY_TYPE]);
- if (k->type < 0 || k->type >= NUM_NL80211_KEYTYPES)
- return genl_err_attr(info, -EINVAL,
- tb[NL80211_KEY_TYPE]);
- }
if (tb[NL80211_KEY_DEFAULT_TYPES]) {
struct nlattr *kdt[NUM_NL80211_KEY_DEFAULT_TYPES];
@@ -966,13 +1008,8 @@ static int nl80211_parse_key_old(struct genl_info *info, struct key_parse *k)
if (k->defmgmt)
k->def_multi = true;
- if (info->attrs[NL80211_ATTR_KEY_TYPE]) {
+ if (info->attrs[NL80211_ATTR_KEY_TYPE])
k->type = nla_get_u32(info->attrs[NL80211_ATTR_KEY_TYPE]);
- if (k->type < 0 || k->type >= NUM_NL80211_KEYTYPES) {
- GENL_SET_ERR_MSG(info, "key type out of range");
- return -EINVAL;
- }
- }
if (info->attrs[NL80211_ATTR_KEY_DEFAULT_TYPES]) {
struct nlattr *kdt[NUM_NL80211_KEY_DEFAULT_TYPES];
@@ -2770,8 +2807,6 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
if (info->attrs[NL80211_ATTR_WIPHY_RETRY_SHORT]) {
retry_short = nla_get_u8(
info->attrs[NL80211_ATTR_WIPHY_RETRY_SHORT]);
- if (retry_short == 0)
- return -EINVAL;
changed |= WIPHY_PARAM_RETRY_SHORT;
}
@@ -2779,8 +2814,6 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
if (info->attrs[NL80211_ATTR_WIPHY_RETRY_LONG]) {
retry_long = nla_get_u8(
info->attrs[NL80211_ATTR_WIPHY_RETRY_LONG]);
- if (retry_long == 0)
- return -EINVAL;
changed |= WIPHY_PARAM_RETRY_LONG;
}
@@ -3272,8 +3305,6 @@ static int nl80211_set_interface(struct sk_buff *skb, struct genl_info *info)
ntype = nla_get_u32(info->attrs[NL80211_ATTR_IFTYPE]);
if (otype != ntype)
change = true;
- if (ntype > NL80211_IFTYPE_MAX)
- return -EINVAL;
}
if (info->attrs[NL80211_ATTR_MESH_ID]) {
@@ -3338,11 +3369,8 @@ static int nl80211_new_interface(struct sk_buff *skb, struct genl_info *info)
if (!info->attrs[NL80211_ATTR_IFNAME])
return -EINVAL;
- if (info->attrs[NL80211_ATTR_IFTYPE]) {
+ if (info->attrs[NL80211_ATTR_IFTYPE])
type = nla_get_u32(info->attrs[NL80211_ATTR_IFTYPE]);
- if (type > NL80211_IFTYPE_MAX)
- return -EINVAL;
- }
if (!rdev->ops->add_virtual_intf ||
!(rdev->wiphy.interface_modes & (1 << type)))
@@ -3531,9 +3559,6 @@ static int nl80211_get_key(struct sk_buff *skb, struct genl_info *info)
if (info->attrs[NL80211_ATTR_KEY_IDX])
key_idx = nla_get_u8(info->attrs[NL80211_ATTR_KEY_IDX]);
- if (key_idx > 5)
- return -EINVAL;
-
if (info->attrs[NL80211_ATTR_MAC])
mac_addr = nla_data(info->attrs[NL80211_ATTR_MAC]);
@@ -3541,8 +3566,6 @@ static int nl80211_get_key(struct sk_buff *skb, struct genl_info *info)
if (info->attrs[NL80211_ATTR_KEY_TYPE]) {
u32 kt = nla_get_u32(info->attrs[NL80211_ATTR_KEY_TYPE]);
- if (kt >= NUM_NL80211_KEYTYPES)
- return -EINVAL;
if (kt != NL80211_KEYTYPE_GROUP &&
kt != NL80211_KEYTYPE_PAIRWISE)
return -EINVAL;
@@ -4372,14 +4395,9 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;
}
- if (info->attrs[NL80211_ATTR_HIDDEN_SSID]) {
+ if (info->attrs[NL80211_ATTR_HIDDEN_SSID])
params.hidden_ssid = nla_get_u32(
info->attrs[NL80211_ATTR_HIDDEN_SSID]);
- if (params.hidden_ssid != NL80211_HIDDEN_SSID_NOT_IN_USE &&
- params.hidden_ssid != NL80211_HIDDEN_SSID_ZERO_LEN &&
- params.hidden_ssid != NL80211_HIDDEN_SSID_ZERO_CONTENTS)
- return -EINVAL;
- }
params.privacy = !!info->attrs[NL80211_ATTR_PRIVACY];
@@ -4409,8 +4427,6 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;
params.p2p_ctwindow =
nla_get_u8(info->attrs[NL80211_ATTR_P2P_CTWINDOW]);
- if (params.p2p_ctwindow > 127)
- return -EINVAL;
if (params.p2p_ctwindow != 0 &&
!(rdev->wiphy.features & NL80211_FEATURE_P2P_GO_CTWIN))
return -EINVAL;
@@ -4422,8 +4438,6 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO)
return -EINVAL;
tmp = nla_get_u8(info->attrs[NL80211_ATTR_P2P_OPPPS]);
- if (tmp > 1)
- return -EINVAL;
params.p2p_opp_ps = tmp;
if (params.p2p_opp_ps != 0 &&
!(rdev->wiphy.features & NL80211_FEATURE_P2P_GO_OPPPS))
@@ -5360,17 +5374,11 @@ static int nl80211_set_station(struct sk_buff *skb, struct genl_info *info)
else
params.listen_interval = -1;
- if (info->attrs[NL80211_ATTR_STA_SUPPORT_P2P_PS]) {
- u8 tmp;
-
- tmp = nla_get_u8(info->attrs[NL80211_ATTR_STA_SUPPORT_P2P_PS]);
- if (tmp >= NUM_NL80211_P2P_PS_STATUS)
- return -EINVAL;
-
- params.support_p2p_ps = tmp;
- } else {
+ if (info->attrs[NL80211_ATTR_STA_SUPPORT_P2P_PS])
+ params.support_p2p_ps =
+ nla_get_u8(info->attrs[NL80211_ATTR_STA_SUPPORT_P2P_PS]);
+ else
params.support_p2p_ps = -1;
- }
if (!info->attrs[NL80211_ATTR_MAC])
return -EINVAL;
@@ -5400,38 +5408,23 @@ static int nl80211_set_station(struct sk_buff *skb, struct genl_info *info)
if (parse_station_flags(info, dev->ieee80211_ptr->iftype, ¶ms))
return -EINVAL;
- if (info->attrs[NL80211_ATTR_STA_PLINK_ACTION]) {
+ if (info->attrs[NL80211_ATTR_STA_PLINK_ACTION])
params.plink_action =
nla_get_u8(info->attrs[NL80211_ATTR_STA_PLINK_ACTION]);
- if (params.plink_action >= NUM_NL80211_PLINK_ACTIONS)
- return -EINVAL;
- }
if (info->attrs[NL80211_ATTR_STA_PLINK_STATE]) {
params.plink_state =
nla_get_u8(info->attrs[NL80211_ATTR_STA_PLINK_STATE]);
- if (params.plink_state >= NUM_NL80211_PLINK_STATES)
- return -EINVAL;
- if (info->attrs[NL80211_ATTR_MESH_PEER_AID]) {
+ if (info->attrs[NL80211_ATTR_MESH_PEER_AID])
params.peer_aid = nla_get_u16(
info->attrs[NL80211_ATTR_MESH_PEER_AID]);
- if (params.peer_aid > IEEE80211_MAX_AID)
- return -EINVAL;
- }
params.sta_modify_mask |= STATION_PARAM_APPLY_PLINK_STATE;
}
- if (info->attrs[NL80211_ATTR_LOCAL_MESH_POWER_MODE]) {
- enum nl80211_mesh_power_mode pm = nla_get_u32(
+ if (info->attrs[NL80211_ATTR_LOCAL_MESH_POWER_MODE])
+ params.local_pm = nla_get_u32(
info->attrs[NL80211_ATTR_LOCAL_MESH_POWER_MODE]);
- if (pm <= NL80211_MESH_POWER_UNKNOWN ||
- pm > NL80211_MESH_POWER_MAX)
- return -EINVAL;
-
- params.local_pm = pm;
- }
-
if (info->attrs[NL80211_ATTR_OPMODE_NOTIF]) {
params.opmode_notif_used = true;
params.opmode_notif =
@@ -5508,13 +5501,8 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
nla_get_u16(info->attrs[NL80211_ATTR_STA_LISTEN_INTERVAL]);
if (info->attrs[NL80211_ATTR_STA_SUPPORT_P2P_PS]) {
- u8 tmp;
-
- tmp = nla_get_u8(info->attrs[NL80211_ATTR_STA_SUPPORT_P2P_PS]);
- if (tmp >= NUM_NL80211_P2P_PS_STATUS)
- return -EINVAL;
-
- params.support_p2p_ps = tmp;
+ params.support_p2p_ps =
+ nla_get_u8(info->attrs[NL80211_ATTR_STA_SUPPORT_P2P_PS]);
} else {
/*
* if not specified, assume it's supported for P2P GO interface,
@@ -5528,8 +5516,6 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
params.aid = nla_get_u16(info->attrs[NL80211_ATTR_PEER_AID]);
else
params.aid = nla_get_u16(info->attrs[NL80211_ATTR_STA_AID]);
- if (!params.aid || params.aid > IEEE80211_MAX_AID)
- return -EINVAL;
if (info->attrs[NL80211_ATTR_STA_CAPABILITY]) {
params.capability =
@@ -5569,12 +5555,9 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info)
nla_get_u8(info->attrs[NL80211_ATTR_OPMODE_NOTIF]);
}
- if (info->attrs[NL80211_ATTR_STA_PLINK_ACTION]) {
+ if (info->attrs[NL80211_ATTR_STA_PLINK_ACTION])
params.plink_action =
nla_get_u8(info->attrs[NL80211_ATTR_STA_PLINK_ACTION]);
- if (params.plink_action >= NUM_NL80211_PLINK_ACTIONS)
- return -EINVAL;
- }
err = nl80211_parse_sta_channel_info(info, ¶ms);
if (err)
@@ -6084,9 +6067,7 @@ static int nl80211_set_bss(struct sk_buff *skb, struct genl_info *info)
if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO)
return -EINVAL;
params.p2p_ctwindow =
- nla_get_s8(info->attrs[NL80211_ATTR_P2P_CTWINDOW]);
- if (params.p2p_ctwindow < 0)
- return -EINVAL;
+ nla_get_u8(info->attrs[NL80211_ATTR_P2P_CTWINDOW]);
if (params.p2p_ctwindow != 0 &&
!(rdev->wiphy.features & NL80211_FEATURE_P2P_GO_CTWIN))
return -EINVAL;
@@ -6098,8 +6079,6 @@ static int nl80211_set_bss(struct sk_buff *skb, struct genl_info *info)
if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO)
return -EINVAL;
tmp = nla_get_u8(info->attrs[NL80211_ATTR_P2P_OPPPS]);
- if (tmp > 1)
- return -EINVAL;
params.p2p_opp_ps = tmp;
if (params.p2p_opp_ps &&
!(rdev->wiphy.features & NL80211_FEATURE_P2P_GO_OPPPS))
@@ -6278,16 +6257,22 @@ static int nl80211_get_mesh_config(struct sk_buff *skb,
return -ENOBUFS;
}
-static const struct nla_policy nl80211_meshconf_params_policy[NL80211_MESHCONF_ATTR_MAX+1] = {
- [NL80211_MESHCONF_RETRY_TIMEOUT] = { .type = NLA_U16 },
- [NL80211_MESHCONF_CONFIRM_TIMEOUT] = { .type = NLA_U16 },
- [NL80211_MESHCONF_HOLDING_TIMEOUT] = { .type = NLA_U16 },
- [NL80211_MESHCONF_MAX_PEER_LINKS] = { .type = NLA_U16 },
- [NL80211_MESHCONF_MAX_RETRIES] = { .type = NLA_U8 },
- [NL80211_MESHCONF_TTL] = { .type = NLA_U8 },
- [NL80211_MESHCONF_ELEMENT_TTL] = { .type = NLA_U8 },
- [NL80211_MESHCONF_AUTO_OPEN_PLINKS] = { .type = NLA_U8 },
- [NL80211_MESHCONF_SYNC_OFFSET_MAX_NEIGHBOR] = { .type = NLA_U32 },
+static const struct nla_policy
+nl80211_meshconf_params_policy[NL80211_MESHCONF_ATTR_MAX+1] = {
+ [NL80211_MESHCONF_RETRY_TIMEOUT] = { .type = NLA_U16,
+ .min = 1, .max = 255 },
+ [NL80211_MESHCONF_CONFIRM_TIMEOUT] = { .type = NLA_U16,
+ .min = 1, .max = 255 },
+ [NL80211_MESHCONF_HOLDING_TIMEOUT] = { .type = NLA_U16,
+ .min = 1, .max = 255 },
+ [NL80211_MESHCONF_MAX_PEER_LINKS] = { .type = NLA_U16, .max = 255 },
+ [NL80211_MESHCONF_MAX_RETRIES] = { .type = NLA_U8, .max = 16 },
+ [NL80211_MESHCONF_TTL] = { .type = NLA_U8, .min = 1, .max = 255 },
+ [NL80211_MESHCONF_ELEMENT_TTL] = { .type = NLA_U8,
+ .min = 1, .max = 255 },
+ [NL80211_MESHCONF_AUTO_OPEN_PLINKS] = { .type = NLA_U8, .max = 1 },
+ [NL80211_MESHCONF_SYNC_OFFSET_MAX_NEIGHBOR] = { .type = NLA_U32,
+ .min = 1, .max = 255 },
[NL80211_MESHCONF_HWMP_MAX_PREQ_RETRIES] = { .type = NLA_U8 },
[NL80211_MESHCONF_PATH_REFRESH_TIME] = { .type = NLA_U32 },
[NL80211_MESHCONF_MIN_DISCOVERY_TIMEOUT] = { .type = NLA_U16 },
@@ -6295,16 +6280,19 @@ static const struct nla_policy nl80211_meshconf_params_policy[NL80211_MESHCONF_A
[NL80211_MESHCONF_HWMP_PREQ_MIN_INTERVAL] = { .type = NLA_U16 },
[NL80211_MESHCONF_HWMP_PERR_MIN_INTERVAL] = { .type = NLA_U16 },
[NL80211_MESHCONF_HWMP_NET_DIAM_TRVS_TIME] = { .type = NLA_U16 },
- [NL80211_MESHCONF_HWMP_ROOTMODE] = { .type = NLA_U8 },
+ [NL80211_MESHCONF_HWMP_ROOTMODE] = { .type = NLA_U8, .max = 4 },
[NL80211_MESHCONF_HWMP_RANN_INTERVAL] = { .type = NLA_U16 },
- [NL80211_MESHCONF_GATE_ANNOUNCEMENTS] = { .type = NLA_U8 },
- [NL80211_MESHCONF_FORWARDING] = { .type = NLA_U8 },
- [NL80211_MESHCONF_RSSI_THRESHOLD] = { .type = NLA_U32 },
+ [NL80211_MESHCONF_GATE_ANNOUNCEMENTS] = { .type = NLA_U8, .max = 1 },
+ [NL80211_MESHCONF_FORWARDING] = { .type = NLA_U8, .max = 1 },
+ [NL80211_MESHCONF_RSSI_THRESHOLD] = { .type = NLA_U32,
+ .min = -255, .max = 0 },
[NL80211_MESHCONF_HT_OPMODE] = { .type = NLA_U16 },
[NL80211_MESHCONF_HWMP_PATH_TO_ROOT_TIMEOUT] = { .type = NLA_U32 },
[NL80211_MESHCONF_HWMP_ROOT_INTERVAL] = { .type = NLA_U16 },
[NL80211_MESHCONF_HWMP_CONFIRMATION_INTERVAL] = { .type = NLA_U16 },
- [NL80211_MESHCONF_POWER_MODE] = { .type = NLA_U32 },
+ [NL80211_MESHCONF_POWER_MODE] = { .type = NLA_U32,
+ .min = NL80211_MESH_POWER_ACTIVE,
+ .max = NL80211_MESH_POWER_MAX },
[NL80211_MESHCONF_AWAKE_WINDOW] = { .type = NLA_U16 },
[NL80211_MESHCONF_PLINK_TIMEOUT] = { .type = NLA_U32 },
};
@@ -6322,63 +6310,6 @@ static const struct nla_policy
[NL80211_MESH_SETUP_USERSPACE_AMPE] = { .type = NLA_FLAG },
};
-static int nl80211_check_bool(const struct nlattr *nla, u8 min, u8 max, bool *out)
-{
- u8 val = nla_get_u8(nla);
- if (val < min || val > max)
- return -EINVAL;
- *out = val;
- return 0;
-}
-
-static int nl80211_check_u8(const struct nlattr *nla, u8 min, u8 max, u8 *out)
-{
- u8 val = nla_get_u8(nla);
- if (val < min || val > max)
- return -EINVAL;
- *out = val;
- return 0;
-}
-
-static int nl80211_check_u16(const struct nlattr *nla, u16 min, u16 max, u16 *out)
-{
- u16 val = nla_get_u16(nla);
- if (val < min || val > max)
- return -EINVAL;
- *out = val;
- return 0;
-}
-
-static int nl80211_check_u32(const struct nlattr *nla, u32 min, u32 max, u32 *out)
-{
- u32 val = nla_get_u32(nla);
- if (val < min || val > max)
- return -EINVAL;
- *out = val;
- return 0;
-}
-
-static int nl80211_check_s32(const struct nlattr *nla, s32 min, s32 max, s32 *out)
-{
- s32 val = nla_get_s32(nla);
- if (val < min || val > max)
- return -EINVAL;
- *out = val;
- return 0;
-}
-
-static int nl80211_check_power_mode(const struct nlattr *nla,
- enum nl80211_mesh_power_mode min,
- enum nl80211_mesh_power_mode max,
- enum nl80211_mesh_power_mode *out)
-{
- u32 val = nla_get_u32(nla);
- if (val < min || val > max)
- return -EINVAL;
- *out = val;
- return 0;
-}
-
static int nl80211_parse_mesh_config(struct genl_info *info,
struct mesh_config *cfg,
u32 *mask_out)
@@ -6387,13 +6318,12 @@ static int nl80211_parse_mesh_config(struct genl_info *info,
u32 mask = 0;
u16 ht_opmode;
-#define FILL_IN_MESH_PARAM_IF_SET(tb, cfg, param, min, max, mask, attr, fn) \
-do { \
- if (tb[attr]) { \
- if (fn(tb[attr], min, max, &cfg->param)) \
- return -EINVAL; \
- mask |= (1 << (attr - 1)); \
- } \
+#define FILL_IN_MESH_PARAM_IF_SET(tb, cfg, param, mask, attr, fn) \
+do { \
+ if (tb[attr]) { \
+ cfg->param = fn(tb[attr]); \
+ mask |= BIT((attr) - 1); \
+ } \
} while (0)
if (!info->attrs[NL80211_ATTR_MESH_CONFIG])
@@ -6408,75 +6338,88 @@ do { \
BUILD_BUG_ON(NL80211_MESHCONF_ATTR_MAX > 32);
/* Fill in the params struct */
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshRetryTimeout, 1, 255,
- mask, NL80211_MESHCONF_RETRY_TIMEOUT,
- nl80211_check_u16);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshConfirmTimeout, 1, 255,
- mask, NL80211_MESHCONF_CONFIRM_TIMEOUT,
- nl80211_check_u16);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshHoldingTimeout, 1, 255,
- mask, NL80211_MESHCONF_HOLDING_TIMEOUT,
- nl80211_check_u16);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshMaxPeerLinks, 0, 255,
- mask, NL80211_MESHCONF_MAX_PEER_LINKS,
- nl80211_check_u16);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshMaxRetries, 0, 16,
- mask, NL80211_MESHCONF_MAX_RETRIES,
- nl80211_check_u8);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshTTL, 1, 255,
- mask, NL80211_MESHCONF_TTL, nl80211_check_u8);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, element_ttl, 1, 255,
- mask, NL80211_MESHCONF_ELEMENT_TTL,
- nl80211_check_u8);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, auto_open_plinks, 0, 1,
- mask, NL80211_MESHCONF_AUTO_OPEN_PLINKS,
- nl80211_check_bool);
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshRetryTimeout, mask,
+ NL80211_MESHCONF_RETRY_TIMEOUT, nla_get_u16);
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshConfirmTimeout, mask,
+ NL80211_MESHCONF_CONFIRM_TIMEOUT,
+ nla_get_u16);
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshHoldingTimeout, mask,
+ NL80211_MESHCONF_HOLDING_TIMEOUT,
+ nla_get_u16);
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshMaxPeerLinks, mask,
+ NL80211_MESHCONF_MAX_PEER_LINKS,
+ nla_get_u16);
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshMaxRetries, mask,
+ NL80211_MESHCONF_MAX_RETRIES, nla_get_u8);
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshTTL, mask,
+ NL80211_MESHCONF_TTL, nla_get_u8);
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, element_ttl, mask,
+ NL80211_MESHCONF_ELEMENT_TTL, nla_get_u8);
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, auto_open_plinks, mask,
+ NL80211_MESHCONF_AUTO_OPEN_PLINKS,
+ nla_get_u8);
FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshNbrOffsetMaxNeighbor,
- 1, 255, mask,
+ mask,
NL80211_MESHCONF_SYNC_OFFSET_MAX_NEIGHBOR,
- nl80211_check_u32);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshHWMPmaxPREQretries, 0, 255,
- mask, NL80211_MESHCONF_HWMP_MAX_PREQ_RETRIES,
- nl80211_check_u8);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, path_refresh_time, 1, 65535,
- mask, NL80211_MESHCONF_PATH_REFRESH_TIME,
- nl80211_check_u32);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, min_discovery_timeout, 1, 65535,
- mask, NL80211_MESHCONF_MIN_DISCOVERY_TIMEOUT,
- nl80211_check_u16);
+ nla_get_u32);
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshHWMPmaxPREQretries, mask,
+ NL80211_MESHCONF_HWMP_MAX_PREQ_RETRIES,
+ nla_get_u8);
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, path_refresh_time, mask,
+ NL80211_MESHCONF_PATH_REFRESH_TIME,
+ nla_get_u32);
+ if (mask & BIT(NL80211_MESHCONF_PATH_REFRESH_TIME) &&
+ (cfg->path_refresh_time < 1 || cfg->path_refresh_time > 65535))
+ return -EINVAL;
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, min_discovery_timeout, mask,
+ NL80211_MESHCONF_MIN_DISCOVERY_TIMEOUT,
+ nla_get_u16);
+ if (mask & BIT(NL80211_MESHCONF_MIN_DISCOVERY_TIMEOUT) &&
+ cfg->min_discovery_timeout < 1)
+ return -EINVAL;
FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshHWMPactivePathTimeout,
- 1, 65535, mask,
+ mask,
NL80211_MESHCONF_HWMP_ACTIVE_PATH_TIMEOUT,
- nl80211_check_u32);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshHWMPpreqMinInterval,
- 1, 65535, mask,
+ nla_get_u32);
+ if (mask & BIT(NL80211_MESHCONF_HWMP_ACTIVE_PATH_TIMEOUT) &&
+ (cfg->dot11MeshHWMPactivePathTimeout < 1 ||
+ cfg->dot11MeshHWMPactivePathTimeout > 65535))
+ return -EINVAL;
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshHWMPpreqMinInterval, mask,
NL80211_MESHCONF_HWMP_PREQ_MIN_INTERVAL,
- nl80211_check_u16);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshHWMPperrMinInterval,
- 1, 65535, mask,
+ nla_get_u16);
+ if (mask & BIT(NL80211_MESHCONF_HWMP_PREQ_MIN_INTERVAL) &&
+ cfg->dot11MeshHWMPpreqMinInterval < 1)
+ return -EINVAL;
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshHWMPperrMinInterval, mask,
NL80211_MESHCONF_HWMP_PERR_MIN_INTERVAL,
- nl80211_check_u16);
+ nla_get_u16);
+ if (mask & BIT(NL80211_MESHCONF_HWMP_PERR_MIN_INTERVAL) &&
+ cfg->dot11MeshHWMPperrMinInterval < 1)
+ return -EINVAL;
FILL_IN_MESH_PARAM_IF_SET(tb, cfg,
- dot11MeshHWMPnetDiameterTraversalTime,
- 1, 65535, mask,
+ dot11MeshHWMPnetDiameterTraversalTime, mask,
NL80211_MESHCONF_HWMP_NET_DIAM_TRVS_TIME,
- nl80211_check_u16);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshHWMPRootMode, 0, 4,
- mask, NL80211_MESHCONF_HWMP_ROOTMODE,
- nl80211_check_u8);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshHWMPRannInterval, 1, 65535,
- mask, NL80211_MESHCONF_HWMP_RANN_INTERVAL,
- nl80211_check_u16);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg,
- dot11MeshGateAnnouncementProtocol, 0, 1,
+ nla_get_u16);
+ if (mask & BIT(NL80211_MESHCONF_HWMP_NET_DIAM_TRVS_TIME) &&
+ cfg->dot11MeshHWMPnetDiameterTraversalTime < 1)
+ return -EINVAL;
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshHWMPRootMode, mask,
+ NL80211_MESHCONF_HWMP_ROOTMODE, nla_get_u8);
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshHWMPRannInterval, mask,
+ NL80211_MESHCONF_HWMP_RANN_INTERVAL,
+ nla_get_u16);
+ if (mask & BIT(NL80211_MESHCONF_HWMP_RANN_INTERVAL) &&
+ cfg->dot11MeshHWMPRannInterval < 1)
+ return -EINVAL;
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshGateAnnouncementProtocol,
mask, NL80211_MESHCONF_GATE_ANNOUNCEMENTS,
- nl80211_check_bool);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshForwarding, 0, 1,
- mask, NL80211_MESHCONF_FORWARDING,
- nl80211_check_bool);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, rssi_threshold, -255, 0,
- mask, NL80211_MESHCONF_RSSI_THRESHOLD,
- nl80211_check_s32);
+ nla_get_u8);
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshForwarding, mask,
+ NL80211_MESHCONF_FORWARDING, nla_get_u8);
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, rssi_threshold, mask,
+ NL80211_MESHCONF_RSSI_THRESHOLD,
+ nla_get_s32);
/*
* Check HT operation mode based on
* IEEE 802.11-2016 9.4.2.57 HT Operation element.
@@ -6495,29 +6438,33 @@ do { \
cfg->ht_opmode = ht_opmode;
mask |= (1 << (NL80211_MESHCONF_HT_OPMODE - 1));
}
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshHWMPactivePathToRootTimeout,
- 1, 65535, mask,
- NL80211_MESHCONF_HWMP_PATH_TO_ROOT_TIMEOUT,
- nl80211_check_u32);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshHWMProotInterval, 1, 65535,
- mask, NL80211_MESHCONF_HWMP_ROOT_INTERVAL,
- nl80211_check_u16);
FILL_IN_MESH_PARAM_IF_SET(tb, cfg,
- dot11MeshHWMPconfirmationInterval,
- 1, 65535, mask,
+ dot11MeshHWMPactivePathToRootTimeout, mask,
+ NL80211_MESHCONF_HWMP_PATH_TO_ROOT_TIMEOUT,
+ nla_get_u32);
+ if (mask & BIT(NL80211_MESHCONF_HWMP_PATH_TO_ROOT_TIMEOUT) &&
+ (cfg->dot11MeshHWMPactivePathToRootTimeout < 1 ||
+ cfg->dot11MeshHWMPactivePathToRootTimeout > 65535))
+ return -EINVAL;
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshHWMProotInterval, mask,
+ NL80211_MESHCONF_HWMP_ROOT_INTERVAL,
+ nla_get_u16);
+ if (mask & BIT(NL80211_MESHCONF_HWMP_ROOT_INTERVAL) &&
+ cfg->dot11MeshHWMProotInterval < 1)
+ return -EINVAL;
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshHWMPconfirmationInterval,
+ mask,
NL80211_MESHCONF_HWMP_CONFIRMATION_INTERVAL,
- nl80211_check_u16);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, power_mode,
- NL80211_MESH_POWER_ACTIVE,
- NL80211_MESH_POWER_MAX,
- mask, NL80211_MESHCONF_POWER_MODE,
- nl80211_check_power_mode);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshAwakeWindowDuration,
- 0, 65535, mask,
- NL80211_MESHCONF_AWAKE_WINDOW, nl80211_check_u16);
- FILL_IN_MESH_PARAM_IF_SET(tb, cfg, plink_timeout, 0, 0xffffffff,
- mask, NL80211_MESHCONF_PLINK_TIMEOUT,
- nl80211_check_u32);
+ nla_get_u16);
+ if (mask & BIT(NL80211_MESHCONF_HWMP_CONFIRMATION_INTERVAL) &&
+ cfg->dot11MeshHWMPconfirmationInterval < 1)
+ return -EINVAL;
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, power_mode, mask,
+ NL80211_MESHCONF_POWER_MODE, nla_get_u32);
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, dot11MeshAwakeWindowDuration, mask,
+ NL80211_MESHCONF_AWAKE_WINDOW, nla_get_u16);
+ FILL_IN_MESH_PARAM_IF_SET(tb, cfg, plink_timeout, mask,
+ NL80211_MESHCONF_PLINK_TIMEOUT, nla_get_u32);
if (mask_out)
*mask_out = mask;
@@ -9516,11 +9463,6 @@ static int nl80211_connect(struct sk_buff *skb, struct genl_info *info)
!wiphy_ext_feature_isset(&rdev->wiphy,
NL80211_EXT_FEATURE_MFP_OPTIONAL))
return -EOPNOTSUPP;
-
- if (connect.mfp != NL80211_MFP_REQUIRED &&
- connect.mfp != NL80211_MFP_NO &&
- connect.mfp != NL80211_MFP_OPTIONAL)
- return -EINVAL;
} else {
connect.mfp = NL80211_MFP_NO;
}
@@ -10279,9 +10221,6 @@ static int nl80211_set_power_save(struct sk_buff *skb, struct genl_info *info)
ps_state = nla_get_u32(info->attrs[NL80211_ATTR_PS_STATE]);
- if (ps_state != NL80211_PS_DISABLED && ps_state != NL80211_PS_ENABLED)
- return -EINVAL;
-
wdev = dev->ieee80211_ptr;
if (!rdev->ops->set_power_mgmt)
@@ -11434,9 +11373,6 @@ static int nl80211_parse_coalesce_rule(struct cfg80211_registered_device *rdev,
if (tb[NL80211_ATTR_COALESCE_RULE_CONDITION])
new_rule->condition =
nla_get_u32(tb[NL80211_ATTR_COALESCE_RULE_CONDITION]);
- if (new_rule->condition != NL80211_COALESCE_CONDITION_MATCH &&
- new_rule->condition != NL80211_COALESCE_CONDITION_NO_MATCH)
- return -EINVAL;
if (!tb[NL80211_ATTR_COALESCE_RULE_PKT_PATTERN])
return -EINVAL;
@@ -11789,8 +11725,6 @@ static int nl80211_start_nan(struct sk_buff *skb, struct genl_info *info)
conf.master_pref =
nla_get_u8(info->attrs[NL80211_ATTR_NAN_MASTER_PREF]);
- if (!conf.master_pref)
- return -EINVAL;
if (info->attrs[NL80211_ATTR_BANDS]) {
u32 bands = nla_get_u32(info->attrs[NL80211_ATTR_BANDS]);
@@ -12775,12 +12709,7 @@ static int nl80211_add_tx_ts(struct sk_buff *skb, struct genl_info *info)
return -EINVAL;
tsid = nla_get_u8(info->attrs[NL80211_ATTR_TSID]);
- if (tsid >= IEEE80211_NUM_TIDS)
- return -EINVAL;
-
up = nla_get_u8(info->attrs[NL80211_ATTR_USER_PRIO]);
- if (up >= IEEE80211_NUM_UPS)
- return -EINVAL;
/* WMM uses TIDs 0-7 even for TSPEC */
if (tsid >= IEEE80211_FIRST_TSPEC_TSID) {
--
2.14.4
On Wed, 2018-09-26 at 22:07 +0200, Johannes Berg wrote:
> From: Johannes Berg <[email protected]>
>
> Many range checks can be done in the policy, move them
> there. A few in mesh are added in the code (taken out of
> the macros) because they don't fit into the s16 range in
> the policy validation.
Forgot to say - this saves 550 bytes of .text on x86-64 for me.
johannes
On Wed, 2018-09-26 at 22:06 +0200, Johannes Berg wrote:
> From: Johannes Berg <[email protected]>
>
> Without further bloating the policy structs, we can overload
> the `validation_data' pointer with a struct of s16 min, max
> and use those to validate ranges in NLA_{U,S}{8,16,32,64}
> attributes.
>
> It may sound strange to validate NLA_U32 with a s16 max, but
> in many cases NLA_U32 is used for enums etc. since there's no
> size benefit in using a smaller attribute width anyway, due
> to netlink attribute alignment; in cases like that it's still
> useful, particularly when the attribute really transports an
> enum value.
That said, I did find a few places where we could benefit from a larger
type here - e.g. having a NLA_U16 that must be non-zero cannot be
represented in the policy as is, since you can't set max to 65535.
However, I don't think we want to push the policy struct to 12 bytes on
32-bit platforms? It's currently 16 bytes on 64-bit due to the pointer
(and alignment), but only 8 bytes on 32-bit.
Keeping the few places that needed this validation is unlikely to be a
larger win than the policy size increase due to the larger type.
johannes
Another thing to note:
Given the union of validation_data pointer and min/max, we'll always get
into this:
> + /* validate range */
> + if (pt->min || pt->max) {
if validation_data is set, but of course end up taking the default case,
where nothing further happens:
> + default:
> + /* no further validation */
> + validate = false;
> + break;
> + }
> +
> + if (validate && (value < pt->min || value > pt->max)) {
> + NL_SET_ERR_MSG_ATTR(extack, nla,
> + "integer out of range");
> + return -ERANGE;
> + }
> + }
> +
> return 0;
I'm not *entirely* happy with this, but I haven't been able to come up
with a way that doesn't do this, doesn't duplicate the nla types list
(NLA_{U,S}{8,16,32,64}) in the code, and also loads the attribute value
only if validation is needed.
johannes
On Wed, 2018-09-26 at 22:17 +0200, Johannes Berg wrote:
> On Wed, 2018-09-26 at 22:06 +0200, Johannes Berg wrote:
> > From: Johannes Berg <[email protected]>
> >
> > Without further bloating the policy structs, we can overload
> > the `validation_data' pointer with a struct of s16 min, max
> > and use those to validate ranges in NLA_{U,S}{8,16,32,64}
> > attributes.
> >
> > It may sound strange to validate NLA_U32 with a s16 max, but
> > in many cases NLA_U32 is used for enums etc. since there's no
> > size benefit in using a smaller attribute width anyway, due
> > to netlink attribute alignment; in cases like that it's still
> > useful, particularly when the attribute really transports an
> > enum value.
>
> That said, I did find a few places where we could benefit from a larger
> type here - e.g. having a NLA_U16 that must be non-zero cannot be
> represented in the policy as is, since you can't set max to 65535.
We could also fix that, btw, by taking two bits out of the "type" field,
and letting those indicate "check_min" and "check_max". That would also
fix the other thing I noted regarding the union, I suppose.
I didn't really like that too much because it makes the whole thing far
more complex, but perhaps if we hide it behind macros like
#define NLA_POLICY_RANGE(tp, _min, _max) {
.type = tp,
.min = _min, .check_min = 1,
.max = _max, .check_max = 1,
}
#define NLA_POLICY_MIN(tp, _min) {
.type = tp,
.min = _min, .check_min = 1,
}
#define NLA_POLICY_MAX(tp, _max) {
.type = tp,
.max = _max, .check_max = 1,
}
it becomes more palatable?
johannes
On Wed, Sep 26, 2018 at 10:35:27PM +0200, Johannes Berg wrote:
> On Wed, 2018-09-26 at 22:17 +0200, Johannes Berg wrote:
> > On Wed, 2018-09-26 at 22:06 +0200, Johannes Berg wrote:
> > > From: Johannes Berg <[email protected]>
> > >
> > > Without further bloating the policy structs, we can overload
> > > the `validation_data' pointer with a struct of s16 min, max
> > > and use those to validate ranges in NLA_{U,S}{8,16,32,64}
> > > attributes.
> > >
> > > It may sound strange to validate NLA_U32 with a s16 max, but
> > > in many cases NLA_U32 is used for enums etc. since there's no
> > > size benefit in using a smaller attribute width anyway, due
> > > to netlink attribute alignment; in cases like that it's still
> > > useful, particularly when the attribute really transports an
> > > enum value.
> >
> > That said, I did find a few places where we could benefit from a larger
> > type here - e.g. having a NLA_U16 that must be non-zero cannot be
> > represented in the policy as is, since you can't set max to 65535.
>
> We could also fix that, btw, by taking two bits out of the "type" field,
> and letting those indicate "check_min" and "check_max". That would also
> fix the other thing I noted regarding the union, I suppose.
>
> I didn't really like that too much because it makes the whole thing far
> more complex, but perhaps if we hide it behind macros like
>
> #define NLA_POLICY_RANGE(tp, _min, _max) {
> .type = tp,
> .min = _min, .check_min = 1,
> .max = _max, .check_max = 1,
> }
>
> #define NLA_POLICY_MIN(tp, _min) {
> .type = tp,
> .min = _min, .check_min = 1,
> }
>
> #define NLA_POLICY_MAX(tp, _max) {
> .type = tp,
> .max = _max, .check_max = 1,
> }
>
> it becomes more palatable?
The overloading still feels a bit complicated. Perhaps we could rather
use validation_data in the natural way, i.e. as a pointer to validation
data. That would be a struct (maybe array) of two values of the
corresponding type. It would mean a bit more data and a bit more writing
but it would be IMHO more straightforward.
Michal Kubecek
On Thu, 2018-09-27 at 09:16 +0200, Michal Kubecek wrote:
> The overloading still feels a bit complicated. Perhaps we could rather
> use validation_data in the natural way, i.e. as a pointer to validation
> data. That would be a struct (maybe array) of two values of the
> corresponding type. It would mean a bit more data and a bit more writing
> but it would be IMHO more straightforward.
I considered that, but I didn't really like it either. The memory
wasting isn't *that* bad (even if we go to s64 that'd only be ~20x16
bytes for nl80211, eating up 320 out of the 550 saved, but still); I'm
more worried about making this really hard to actually *do*.
Consider
policy[] = {
...
[NL80211_ATTR_WIPHY_RETRY_SHORT] =
NLA_POLICY_RANGE(NLA_U8, 1, 255),
...
};
vs.
static const struct netlink_policy_range retry_range = {
.min = 1,
.max = 255,
};
policy[] = {
...
[NL80211_ATTR_WIPHY_RETRY_SHORT] = {
.type = NLA_U8,
.validation_data = &retry_range,
},
...
};
That's significantly more to type, to the point where I'd seriously
consider doing this only for attributes that are used and checked in
many places - it doesn't feel like a big win over manual range-checking.
But I want it to be a win over manual range-checking so it gets used
more because it's more efficient, less prone to getting messed up if
multiple places use the same attribute and validates attributes even if
they're ignored by an operation.
I'd also say that we're certainly no strangers to union/overloading, so
I don't feel like this is a big argument. One doesn't even really have
to be *aware* of it for the most part: if it were a struct instead of a
union, it'd actually have the same effect since the .type field
indicates which part gets used. That it's overloaded in a union is
basically just a space saving measure, I don't think it makes the
reasoning much more complex?
That said, given that I also later sent that RFC patch for a further
validation function pointer (which is useful e.g. for ensuring certain
binary attributes are well-formed), I think it might in fact be better
to split .type field (it really only needs to be u8, we have ~20) and
adding a "validation" enum to the policy:
enum netlink_policy_validation {
/* default */
NLA_VALIDATE_NONE,
/* valid for NLA_{U,S}{8,16,32,64} */
NLA_VALIDATE_MIN,
NLA_VALIDATE_MAX,
NLA_VALIDATE_RANGE,
/* valid for any type other than NLA_BITFIELD32/NLA_REJECT */
NLA_VALIDATE_FUNCTION,
};
Combining that with macros like the ones I wrote in my previous message
in this thread:
#define __NLA_ENSURE(condition) (sizeof(char[1 - 2*!(condition)]) - 1)
#define NLA_ENSURE_INT_TYPE(tp) \
(__NLA_ENSURE(tp == NLA_S8 || tp == NLA_U8 || \
tp == NLA_S16 || tp == NLA_U16 || \
tp == NLA_S32 || tp == NLA_U32 || \
tp == NLA_S64 || tp == NLA_U64) + tp)
#define NLA_ENSURE_NO_VALIDATION_PTR(tp) \
(__NLA_ENSURE(tp != NLA_BITFIELD32 && \
tp != NLA_REJECT && \
tp != NLA_NESTED && \
tp != NLA_NESTED_ARRAY) + tp)
#define NLA_POLICY_RANGE(tp, _min, _max) {
.type = NLA_ENSURE_INT_TYPE(tp),
.validate_type = NLA_VALIDATE_RANGE,
.min = _min,
.max = _max,
}
#define NLA_POLICY_MIN(tp, _min) {
.type = NLA_ENSURE_INT_TYPE(tp),
.validate_type = NLA_VALIDATE_MIN,
.min = _min,
}
#define NLA_POLICY_MAX(tp, _max) {
.type = NLA_ENSURE_INT_TYPE(tp),
.validate_type = NLA_VALIDATE_MAX,
.max = _max,
}
#define NLA_POLICY_FN(tp, fn) {
.type = NLA_ENSURE_NO_VALIDATION_PTR(tp),
.validate_type = NLA_VALIDATE_FUNCTION,
.validate = fn,
}
This would even give us the flexibility to extend the validation type
further in the future, to actually have what you suggested where the
validation_data is a pointer and the valid range is given in a struct
pointed to, to allow larger ranges than s16.
johannes
On Thu, Sep 27, 2018 at 10:12:09AM +0200, Johannes Berg wrote:
> On Thu, 2018-09-27 at 09:16 +0200, Michal Kubecek wrote:
>
> > The overloading still feels a bit complicated. Perhaps we could rather
> > use validation_data in the natural way, i.e. as a pointer to validation
> > data. That would be a struct (maybe array) of two values of the
> > corresponding type. It would mean a bit more data and a bit more writing
> > but it would be IMHO more straightforward.
>
> I considered that, but I didn't really like it either. The memory
> wasting isn't *that* bad (even if we go to s64 that'd only be ~20x16
> bytes for nl80211, eating up 320 out of the 550 saved, but still); I'm
> more worried about making this really hard to actually *do*.
>
> Consider
>
> policy[] = {
> ...
> [NL80211_ATTR_WIPHY_RETRY_SHORT] =
> NLA_POLICY_RANGE(NLA_U8, 1, 255),
> ...
> };
>
> vs.
>
> static const struct netlink_policy_range retry_range = {
> .min = 1,
> .max = 255,
> };
We could still use helper macros so this part could become
DEFINE_NLA_U8_RANGE(retry_range, 1, 255);
or
DEFINE_NLA_RANGE(retry_range, u8, 1, 255);
>
> policy[] = {
> ...
> [NL80211_ATTR_WIPHY_RETRY_SHORT] = {
> .type = NLA_U8,
> .validation_data = &retry_range,
> },
> ...
> };
And this could be also shortened using a macro.
It would still be longer but not that much.
> That's significantly more to type, to the point where I'd seriously
> consider doing this only for attributes that are used and checked in
> many places - it doesn't feel like a big win over manual range-checking.
>
> But I want it to be a win over manual range-checking so it gets used
> more because it's more efficient, less prone to getting messed up if
> multiple places use the same attribute and validates attributes even if
> they're ignored by an operation.
>
>
> I'd also say that we're certainly no strangers to union/overloading, so
> I don't feel like this is a big argument. One doesn't even really have
> to be *aware* of it for the most part: if it were a struct instead of a
> union, it'd actually have the same effect since the .type field
> indicates which part gets used. That it's overloaded in a union is
> basically just a space saving measure, I don't think it makes the
> reasoning much more complex?
I didn't mean it as a serious objection, rather a note that the gain may
not be worth the additional complexity. But if you want to follow in the
direction you indicated later (in particular, allowing different
interpretations of validation_data for the same type), overloading does
indeed make more sense.
Michal Kubecek
On Thu, 2018-09-27 at 10:48 +0200, Michal Kubecek wrote:
> We could still use helper macros so this part could become
>
> DEFINE_NLA_U8_RANGE(retry_range, 1, 255);
>
> or
>
> DEFINE_NLA_RANGE(retry_range, u8, 1, 255);
True.
> > policy[] = {
> > ...
> > [NL80211_ATTR_WIPHY_RETRY_SHORT] = {
> > .type = NLA_U8,
> > .validation_data = &retry_range,
> > },
> > ...
> > };
>
> And this could be also shortened using a macro.
>
> It would still be longer but not that much.
Right. You'd still have to name it, and then we'd probably eventually
want to share some common range definitions, but indeed it would work.
> I didn't mean it as a serious objection, rather a note that the gain may
> not be worth the additional complexity.
Sure, and suggestions are very welcome. I was just trying to explain why
I chose this direction, more than anything else.
> But if you want to follow in the
> direction you indicated later (in particular, allowing different
> interpretations of validation_data for the same type), overloading does
> indeed make more sense.
I'm just working on the patches - give me a few more minutes.
johannes