From 901bb9891855164fdcfcfdd9c3d25bcc800d3f5b Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Tue, 28 May 2019 10:56:03 +0200 Subject: [PATCH] nl80211: require and validate vendor command policy Require that each vendor command give a policy of its sub-attributes in NL80211_ATTR_VENDOR_DATA, and then (stricly) check the contents, including the NLA_F_NESTED flag that we couldn't check on the outer layer because there we don't know yet. It is possible to use VENDOR_CMD_RAW_DATA for raw data, but then no nested data can be given (NLA_F_NESTED flag must be clear) and the data is just passed as is to the command. Signed-off-by: Johannes Berg --- drivers/net/wireless/mac80211_hwsim.c | 2 ++ include/net/cfg80211.h | 8 ++++++ include/net/netlink.h | 9 +++++++ net/wireless/core.c | 13 +++++++++ net/wireless/nl80211.c | 39 +++++++++++++++++++++++++-- 5 files changed, 69 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/mac80211_hwsim.c b/drivers/net/wireless/mac80211_hwsim.c index 60ca13e0f15b..b88768c661e2 100644 --- a/drivers/net/wireless/mac80211_hwsim.c +++ b/drivers/net/wireless/mac80211_hwsim.c @@ -457,6 +457,8 @@ static struct wiphy_vendor_command mac80211_hwsim_vendor_commands[] = { .subcmd = QCA_NL80211_SUBCMD_TEST }, .flags = WIPHY_VENDOR_CMD_NEED_NETDEV, .doit = mac80211_hwsim_vendor_cmd_test, + .policy = hwsim_vendor_test_policy, + .maxattr = QCA_WLAN_VENDOR_ATTR_MAX, } }; diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 20613b35afcd..7c4aa868e7a5 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -4170,6 +4170,8 @@ struct sta_opmode_info { u8 rx_nss; }; +#define VENDOR_CMD_RAW_DATA ((const struct nla_policy *)ERR_PTR(-ENODATA)) + /** * struct wiphy_vendor_command - vendor command definition * @info: vendor command identifying information, as used in nl80211 @@ -4180,6 +4182,10 @@ struct sta_opmode_info { * @dumpit: dump callback, for transferring bigger/multiple items. The * @storage points to cb->args[5], ie. is preserved over the multiple * dumpit calls. + * @policy: policy pointer for attributes within %NL80211_ATTR_VENDOR_DATA. + * Set this to %VENDOR_CMD_RAW_DATA if no policy can be given and the + * attribute is just raw data (e.g. a firmware command). + * @maxattr: highest attribute number in policy * It's recommended to not have the same sub command with both @doit and * @dumpit, so that userspace can assume certain ones are get and others * are used with dump requests. @@ -4192,6 +4198,8 @@ struct wiphy_vendor_command { int (*dumpit)(struct wiphy *wiphy, struct wireless_dev *wdev, struct sk_buff *skb, const void *data, int data_len, unsigned long *storage); + const struct nla_policy *policy; + unsigned int maxattr; }; /** diff --git a/include/net/netlink.h b/include/net/netlink.h index 395b4406f4b0..28ece67f5312 100644 --- a/include/net/netlink.h +++ b/include/net/netlink.h @@ -1754,6 +1754,15 @@ static inline int __nla_validate_nested(const struct nlattr *start, int maxtype, validate, extack); } +static inline int +nl80211_validate_nested(const struct nlattr *start, int maxtype, + const struct nla_policy *policy, + struct netlink_ext_ack *extack) +{ + return __nla_validate_nested(start, maxtype, policy, + NL_VALIDATE_STRICT, extack); +} + static inline int nla_validate_nested_deprecated(const struct nlattr *start, int maxtype, const struct nla_policy *policy, diff --git a/net/wireless/core.c b/net/wireless/core.c index 037816163e70..fba0915fbd6f 100644 --- a/net/wireless/core.c +++ b/net/wireless/core.c @@ -859,6 +859,19 @@ int wiphy_register(struct wiphy *wiphy) return -EINVAL; } + for (i = 0; i < rdev->wiphy.n_vendor_commands; i++) { + /* + * Validate we have a policy (can be explicitly set to + * VENDOR_CMD_RAW_DATA which is non-NULL) and also that + * we have at least one of doit/dumpit. + */ + if (WARN_ON(!rdev->wiphy.vendor_commands[i].policy)) + return -EINVAL; + if (WARN_ON(!rdev->wiphy.vendor_commands[i].doit && + !rdev->wiphy.vendor_commands[i].dumpit)) + return -EINVAL; + } + #ifdef CONFIG_PM if (WARN_ON(rdev->wiphy.wowlan && rdev->wiphy.wowlan->n_patterns && (!rdev->wiphy.wowlan->pattern_min_len || diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index 80e514872719..34e86539552e 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -12669,6 +12669,29 @@ static int nl80211_crit_protocol_stop(struct sk_buff *skb, return 0; } +static int nl80211_vendor_check_policy(const struct wiphy_vendor_command *vcmd, + struct nlattr *attr, + struct netlink_ext_ack *extack) +{ + if (vcmd->policy == VENDOR_CMD_RAW_DATA) { + if (attr->nla_type & NLA_F_NESTED) { + NL_SET_ERR_MSG_ATTR(extack, attr, + "unexpected nested data"); + return -EINVAL; + } + + return 0; + } + + if (!(attr->nla_type & NLA_F_NESTED)) { + NL_SET_ERR_MSG_ATTR(extack, attr, "expected nested data"); + return -EINVAL; + } + + return nl80211_validate_nested(attr, vcmd->maxattr, vcmd->policy, + extack); +} + static int nl80211_vendor_cmd(struct sk_buff *skb, struct genl_info *info) { struct cfg80211_registered_device *rdev = info->user_ptr[0]; @@ -12727,11 +12750,16 @@ static int nl80211_vendor_cmd(struct sk_buff *skb, struct genl_info *info) if (info->attrs[NL80211_ATTR_VENDOR_DATA]) { data = nla_data(info->attrs[NL80211_ATTR_VENDOR_DATA]); len = nla_len(info->attrs[NL80211_ATTR_VENDOR_DATA]); + + err = nl80211_vendor_check_policy(vcmd, + info->attrs[NL80211_ATTR_VENDOR_DATA], + info->extack); + if (err) + return err; } rdev->cur_cmd_info = info; - err = rdev->wiphy.vendor_commands[i].doit(&rdev->wiphy, wdev, - data, len); + err = vcmd->doit(&rdev->wiphy, wdev, data, len); rdev->cur_cmd_info = NULL; return err; } @@ -12818,6 +12846,13 @@ static int nl80211_prepare_vendor_dump(struct sk_buff *skb, if (attrbuf[NL80211_ATTR_VENDOR_DATA]) { data = nla_data(attrbuf[NL80211_ATTR_VENDOR_DATA]); data_len = nla_len(attrbuf[NL80211_ATTR_VENDOR_DATA]); + + err = nl80211_vendor_check_policy( + &(*rdev)->wiphy.vendor_commands[vcmd_idx], + attrbuf[NL80211_ATTR_VENDOR_DATA], + cb->extack); + if (err) + return err; } /* 0 is the first index - add 1 to parse only once */ -- 2.45.2