Skip to content

Commit

Permalink
Merge branch 'netlink-strict-attribute-checking-follow-up'
Browse files Browse the repository at this point in the history
Michal Kubecek says:

====================
netlink: strict attribute checking follow-up

Three follow-up patches for recent strict netlink validation series.

Patch 1 fixes dump handling for genetlink families which validate and parse
messages themselves (e.g. because they need different policies for diferent
commands).

Patch 2 sets bad_attr in extack in one place where this was omitted.

Patch 3 adds new NL_VALIDATE_NESTED flags for strict validation to enable
checking that NLA_F_NESTED value in received messages matches expectations
and includes this flag in NL_VALIDATE_STRICT. This would change userspace
visible behavior but the previous switching to NL_VALIDATE_STRICT for new
code is still only in net-next at the moment.

v2: change error messages to mention NLA_F_NESTED explicitly
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
David S. Miller committed May 4, 2019
2 parents 5eabc27 + b424e43 commit 8cca339
Showing 3 changed files with 41 additions and 12 deletions.
11 changes: 10 additions & 1 deletion include/net/netlink.h
Original file line number Diff line number Diff line change
@@ -401,21 +401,25 @@ 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,
NL_VALIDATE_TRAILING = BIT(0),
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 |\
NL_VALIDATE_MAXTYPE)
#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);
}
18 changes: 17 additions & 1 deletion lib/nlattr.c
Original file line number Diff line number Diff line change
@@ -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)
@@ -356,7 +371,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;
24 changes: 14 additions & 10 deletions net/netlink/genetlink.c
Original file line number Diff line number Diff line change
@@ -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) {

0 comments on commit 8cca339

Please sign in to comment.