| Message ID | 1510120780-22760-1-git-send-email-dsahern@gmail.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v2,net-next] net: netlink: Update attr validation to require exact length for some types | expand |
From: David Ahern <dsahern@gmail.com> Date: Tue, 7 Nov 2017 21:59:40 -0800 > Attributes using NLA_U* and NLA_S* (where * is 8, 16,32 and 64) are > expected to be an exact length. Split these data types from > nla_attr_minlen into nla_attr_len and update validate_nla to require > the attribute to have exact length for them. > > Signed-off-by: David Ahern <dsahern@gmail.com> > --- > v2 > - fix check in nla_policy_len - reported by kernel test robot Applied, thanks David.
Well.. This did not go well with gmail defaults and the mailing list.. Sending this with something safer and plaintext only version to get this on the mailing list as well: On Wed, Nov 8, 2017 at 7:59 AM, David Ahern <dsahern@gmail.com> wrote: > Attributes using NLA_U* and NLA_S* (where * is 8, 16,32 and 64) are > expected to be an exact length. Split these data types from > nla_attr_minlen into nla_attr_len and update validate_nla to require > the attribute to have exact length for them. While I understand and support this change in general, I have to note that this resulted in some unfortunate user space regressions that came apparent when testing Wi-Fi with Linux 4.15-rc1. When a new nl80211 attribute was added for controlling SMPS modes in 2014 the kernel contribution added this with NLA_U8 policy while the user space contribution to hostapd used NLA_PUT_U32. This has apparently been unnoticed until now since the first byte contained the appropriate value on little endian devices (no one testing this on big endian hosts?).. I'll obviously fix the encoding of this attribute in hostapd, but it should be noted that Linux 4.15 will result in significant functionality issues if the kernel is updated without a user space fix going in first.
diff --git a/lib/nlattr.c b/lib/nlattr.c index 3d8295c85505..8bf78b4b78f0 100644 --- a/lib/nlattr.c +++ b/lib/nlattr.c @@ -15,19 +15,23 @@ #include <linux/types.h> #include <net/netlink.h> -static const u8 nla_attr_minlen[NLA_TYPE_MAX+1] = { +/* for these data types attribute length must be exactly given size */ +static const u8 nla_attr_len[NLA_TYPE_MAX+1] = { [NLA_U8] = sizeof(u8), [NLA_U16] = sizeof(u16), [NLA_U32] = sizeof(u32), [NLA_U64] = sizeof(u64), - [NLA_MSECS] = sizeof(u64), - [NLA_NESTED] = NLA_HDRLEN, [NLA_S8] = sizeof(s8), [NLA_S16] = sizeof(s16), [NLA_S32] = sizeof(s32), [NLA_S64] = sizeof(s64), }; +static const u8 nla_attr_minlen[NLA_TYPE_MAX+1] = { + [NLA_MSECS] = sizeof(u64), + [NLA_NESTED] = NLA_HDRLEN, +}; + static int validate_nla_bitfield32(const struct nlattr *nla, u32 *valid_flags_allowed) { @@ -65,6 +69,13 @@ static int validate_nla(const struct nlattr *nla, int maxtype, BUG_ON(pt->type > NLA_TYPE_MAX); + /* for data types NLA_U* and NLA_S* require exact length */ + if (nla_attr_len[pt->type]) { + if (attrlen != nla_attr_len[pt->type]) + return -ERANGE; + return 0; + } + switch (pt->type) { case NLA_FLAG: if (attrlen > 0) @@ -191,6 +202,8 @@ nla_policy_len(const struct nla_policy *p, int n) for (i = 0; i < n; i++, p++) { if (p->len) len += nla_total_size(p->len); + else if (nla_attr_len[p->type]) + len += nla_total_size(nla_attr_len[p->type]); else if (nla_attr_minlen[p->type]) len += nla_total_size(nla_attr_minlen[p->type]); }
Attributes using NLA_U* and NLA_S* (where * is 8, 16,32 and 64) are expected to be an exact length. Split these data types from nla_attr_minlen into nla_attr_len and update validate_nla to require the attribute to have exact length for them. Signed-off-by: David Ahern <dsahern@gmail.com> --- v2 - fix check in nla_policy_len - reported by kernel test robot lib/nlattr.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)