diff options
author | Johannes Berg <johannes.berg@intel.com> | 2019-04-26 14:07:30 +0200 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2019-04-27 17:07:22 -0400 |
commit | 56738f460841761abc70347c919d5c45f6f05a42 (patch) | |
tree | e44f0424a4a2c3e04650ba6251f31437cc2eebac /lib/nlattr.c | |
parent | 3de644035446567017e952f16da2594d6bd195fc (diff) | |
download | linux-56738f460841761abc70347c919d5c45f6f05a42.tar.gz linux-56738f460841761abc70347c919d5c45f6f05a42.tar.bz2 linux-56738f460841761abc70347c919d5c45f6f05a42.zip |
netlink: add strict parsing for future attributes
Unfortunately, we cannot add strict parsing for all attributes, as
that would break existing userspace. We currently warn about it, but
that's about all we can do.
For new attributes, however, the story is better: nobody is using
them, so we can reject bad sizes.
Also, for new attributes, we need not accept them when the policy
doesn't declare their usage.
David Ahern and I went back and forth on how to best encode this, and
the best way we found was to have a "boundary type", from which point
on new attributes have all possible validation applied, and NLA_UNSPEC
is rejected.
As we didn't want to add another argument to all functions that get a
netlink policy, the workaround is to encode that boundary in the first
entry of the policy array (which is for type 0 and thus probably not
really valid anyway). I put it into the validation union for the rare
possibility that somebody is actually using attribute 0, which would
continue to work fine unless they tried to use the extended validation,
which isn't likely. We also didn't find any in-tree users with type 0.
The reason for setting the "start strict here" attribute is that we
never really need to start strict from 0, which is invalid anyway (or
in legacy families where that isn't true, it cannot be set to strict),
so we can thus reserve the value 0 for "don't do this check" and don't
have to add the tag to all policies right now.
Thus, policies can now opt in to this validation, which we should do
for all existing policies, at least when adding new attributes.
Note that entirely *new* policies won't need to set it, as the use
of that should be using nla_parse()/nlmsg_parse() etc. which anyway
do fully strict validation now, regardless of this.
So in effect, this patch only covers the "existing command with new
attribute" case.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'lib/nlattr.c')
-rw-r--r-- | lib/nlattr.c | 4 |
1 files changed, 4 insertions, 0 deletions
diff --git a/lib/nlattr.c b/lib/nlattr.c index af0f8b0309c6..29f6336e2422 100644 --- a/lib/nlattr.c +++ b/lib/nlattr.c @@ -158,10 +158,14 @@ static int validate_nla(const struct nlattr *nla, int maxtype, const struct nla_policy *policy, unsigned int validate, struct netlink_ext_ack *extack) { + u16 strict_start_type = policy[0].strict_start_type; const struct nla_policy *pt; int minlen = 0, attrlen = nla_len(nla), type = nla_type(nla); int err = -ERANGE; + if (strict_start_type && type >= strict_start_type) + validate |= NL_VALIDATE_STRICT; + if (type <= 0 || type > maxtype) return 0; |