On 4/28/19 1:32 PM, Johannes Berg wrote:
> On Fri, 2019-04-26 at 20:28 -0600, David Ahern wrote:
>>
>> I agree with this set and will help moving forward. As I recall it
>> requires follow up patches for each policy to set strict_start_type
>> opting in to the strict checking. With that in place new userspace on
>> old kernels will get a failure trying to configure a feature the old
>> kernel does not recognize.
>
> Actually, that part you had already handled with nla_parse_strict() (now
> nla_parse_strict_deprecated()) - and I'm not sure we can make this even
> stricter because existing code might be setting future attributes
> already, expecting them to be ignored. This is already fishy of
> applications to expect though, but I'm not sure we really can change
> that? I don't think I'm actually changing something here, but I'm
> certainly open to suggestions - after all, when we actually do get
> around to adding that future attribute it almost certainly will have a
> different type than a (buggy) application would be using now.
>
> However, what I did already do is that adding strict_start_type to
> policies means that all future attributes added to those policies will
> be handled in a strict fashion, i.e. if you add strict_start_type and
> then add a new u32 attribute >= strict_start_type, that new u32
> attribute will not be permitted to have a size other than 4 octets.
>
For routes, an unknown attribute should cause the NEW/DEL command to
fail. There is a rare exception - something like RTA_PROTOCOL which is
just a passthrough, but in general the attribute is an important
modifier for the route.
With this change:
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index b298255f6fdb..7325c0265c5b 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -645,6 +645,7 @@ int ip_rt_ioctl(struct net *net, unsigned int cmd,
struct rtentry *rt)
}
const struct nla_policy rtm_ipv4_policy[RTA_MAX + 1] = {
+ [RTA_UNSPEC] = { .strict_start_type = RTA_DPORT + 1 },
[RTA_DST] = { .type = NLA_U32 },
[RTA_SRC] = { .type = NLA_U32 },
[RTA_IIF] = { .type = NLA_U32 },
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index b18e85cd7587..0760224927d2 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4211,6 +4211,7 @@ void rt6_mtu_change(struct net_device *dev,
unsigned int mtu)
}
static const struct nla_policy rtm_ipv6_policy[RTA_MAX+1] = {
+ [RTA_UNSPEC] = { .strict_start_type = RTA_DPORT + 1 },
[RTA_GATEWAY] = { .len = sizeof(struct in6_addr) },
[RTA_PREFSRC] = { .len = sizeof(struct in6_addr) },
[RTA_OIF] = { .type = NLA_U32 },
I do get that behavior - a new command using a new attribute fails on an
older kernel.
And yes, exact length checking works though with extack we do not need
to spam dmesg:
[ 475.175153] netlink: 'ip': attribute type 30 has an invalid length.
