From 05d7f547bea1872e711ee97bd46aace6cf61c42b Mon Sep 17 00:00:00 2001 From: Michal Kubecek Date: Thu, 2 May 2019 16:15:10 +0200 Subject: genetlink: do not validate dump requests if there is no policy Unlike do requests, dump genetlink requests now perform strict validation by default even if the genetlink family does not set policy and maxtype because it does validation and parsing on its own (e.g. because it wants to allow different message format for different commands). While the null policy will be ignored, maxtype (which would be zero) is still checked so that any attribute will fail validation. The solution is to only call __nla_validate() from genl_family_rcv_msg() if family->maxtype is set. Fixes: ef6243acb478 ("genetlink: optionally validate strictly/dumps") Signed-off-by: Michal Kubecek Reviewed-by: Johannes Berg Signed-off-by: David S. Miller --- net/netlink/genetlink.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c index 79cfa031dc7d..efccd1ac9a66 100644 --- a/net/netlink/genetlink.c +++ b/net/netlink/genetlink.c @@ -537,21 +537,25 @@ static int genl_family_rcv_msg(const struct genl_family *family, return -EOPNOTSUPP; if (!(ops->validate & GENL_DONT_VALIDATE_DUMP)) { - unsigned int validate = NL_VALIDATE_STRICT; int hdrlen = GENL_HDRLEN + family->hdrsize; - if (ops->validate & GENL_DONT_VALIDATE_DUMP_STRICT) - validate = NL_VALIDATE_LIBERAL; - if (nlh->nlmsg_len < nlmsg_msg_size(hdrlen)) return -EINVAL; - rc = __nla_validate(nlmsg_attrdata(nlh, hdrlen), - nlmsg_attrlen(nlh, hdrlen), - family->maxattr, family->policy, - validate, extack); - if (rc) - return rc; + if (family->maxattr) { + unsigned int validate = NL_VALIDATE_STRICT; + + if (ops->validate & + GENL_DONT_VALIDATE_DUMP_STRICT) + validate = NL_VALIDATE_LIBERAL; + rc = __nla_validate(nlmsg_attrdata(nlh, hdrlen), + nlmsg_attrlen(nlh, hdrlen), + family->maxattr, + family->policy, + validate, extack); + if (rc) + return rc; + } } if (!family->parallel_ops) { -- cgit v1.2.3 From d54a16b20157ce300298eb4a1169bf9acfda3d08 Mon Sep 17 00:00:00 2001 From: Michal Kubecek Date: Thu, 2 May 2019 16:15:10 +0200 Subject: netlink: set bad attribute also on maxtype check The check that attribute type is within 0...maxtype range in __nla_validate_parse() sets only error message but not bad_attr in extack. Set also bad_attr to tell userspace which attribute failed validation. Signed-off-by: Michal Kubecek Reviewed-by: Johannes Berg Reviewed-by: David Ahern Signed-off-by: David S. Miller --- lib/nlattr.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/nlattr.c b/lib/nlattr.c index 29f6336e2422..adc919b32bf9 100644 --- a/lib/nlattr.c +++ b/lib/nlattr.c @@ -356,7 +356,8 @@ static int __nla_validate_parse(const struct nlattr *head, int len, int maxtype, if (type == 0 || type > maxtype) { if (validate & NL_VALIDATE_MAXTYPE) { - NL_SET_ERR_MSG(extack, "Unknown attribute type"); + NL_SET_ERR_MSG_ATTR(extack, nla, + "Unknown attribute type"); return -EINVAL; } continue; -- cgit v1.2.3 From b424e432e770d6dd572765459d5b6a96a19c5286 Mon Sep 17 00:00:00 2001 From: Michal Kubecek Date: Thu, 2 May 2019 16:15:10 +0200 Subject: netlink: add validation of NLA_F_NESTED flag Add new validation flag NL_VALIDATE_NESTED which adds three consistency checks of NLA_F_NESTED_FLAG: - the flag is set on attributes with NLA_NESTED{,_ARRAY} policy - the flag is not set on attributes with other policies except NLA_UNSPEC - the flag is set on attribute passed to nla_parse_nested() Signed-off-by: Michal Kubecek v2: change error messages to mention NLA_F_NESTED explicitly Reviewed-by: Johannes Berg Reviewed-by: David Ahern Signed-off-by: David S. Miller --- include/net/netlink.h | 11 ++++++++++- lib/nlattr.c | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/include/net/netlink.h b/include/net/netlink.h index 679f649748d4..395b4406f4b0 100644 --- a/include/net/netlink.h +++ b/include/net/netlink.h @@ -401,6 +401,8 @@ struct nl_info { * are enforced going forward. * @NL_VALIDATE_STRICT_ATTRS: strict attribute policy parsing (e.g. * U8, U16, U32 must have exact size, etc.) + * @NL_VALIDATE_NESTED: Check that NLA_F_NESTED is set for NLA_NESTED(_ARRAY) + * and unset for other policies. */ enum netlink_validation { NL_VALIDATE_LIBERAL = 0, @@ -408,6 +410,7 @@ enum netlink_validation { NL_VALIDATE_MAXTYPE = BIT(1), NL_VALIDATE_UNSPEC = BIT(2), NL_VALIDATE_STRICT_ATTRS = BIT(3), + NL_VALIDATE_NESTED = BIT(4), }; #define NL_VALIDATE_DEPRECATED_STRICT (NL_VALIDATE_TRAILING |\ @@ -415,7 +418,8 @@ enum netlink_validation { #define NL_VALIDATE_STRICT (NL_VALIDATE_TRAILING |\ NL_VALIDATE_MAXTYPE |\ NL_VALIDATE_UNSPEC |\ - NL_VALIDATE_STRICT_ATTRS) + NL_VALIDATE_STRICT_ATTRS |\ + NL_VALIDATE_NESTED) int netlink_rcv_skb(struct sk_buff *skb, int (*cb)(struct sk_buff *, struct nlmsghdr *, @@ -1132,6 +1136,11 @@ static inline int nla_parse_nested(struct nlattr *tb[], int maxtype, const struct nla_policy *policy, struct netlink_ext_ack *extack) { + if (!(nla->nla_type & NLA_F_NESTED)) { + NL_SET_ERR_MSG_ATTR(extack, nla, "NLA_F_NESTED is missing"); + return -EINVAL; + } + return __nla_parse(tb, maxtype, nla_data(nla), nla_len(nla), policy, NL_VALIDATE_STRICT, extack); } diff --git a/lib/nlattr.c b/lib/nlattr.c index adc919b32bf9..cace9b307781 100644 --- a/lib/nlattr.c +++ b/lib/nlattr.c @@ -184,6 +184,21 @@ static int validate_nla(const struct nlattr *nla, int maxtype, } } + if (validate & NL_VALIDATE_NESTED) { + if ((pt->type == NLA_NESTED || pt->type == NLA_NESTED_ARRAY) && + !(nla->nla_type & NLA_F_NESTED)) { + NL_SET_ERR_MSG_ATTR(extack, nla, + "NLA_F_NESTED is missing"); + return -EINVAL; + } + if (pt->type != NLA_NESTED && pt->type != NLA_NESTED_ARRAY && + pt->type != NLA_UNSPEC && (nla->nla_type & NLA_F_NESTED)) { + NL_SET_ERR_MSG_ATTR(extack, nla, + "NLA_F_NESTED not expected"); + return -EINVAL; + } + } + switch (pt->type) { case NLA_EXACT_LEN: if (attrlen != pt->len) -- cgit v1.2.3