Message ID | 20161216083059.251368-1-grawity@gmail.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
From: Mantas Mikulėnas <grawity@gmail.com> Date: Fri, 16 Dec 2016 10:30:59 +0200 > The protocol field is checked when deleting IPv4 routes, but ignored for > IPv6, which causes problems with routing daemons accidentally deleting > externally set routes (observed by multiple bird6 users). > > This can be verified using `ip -6 route del <prefix> proto something`. > > Signed-off-by: Mantas Mikulėnas <grawity@gmail.com> Applied, thanks.
On Fri, Dec 16, 2016 at 5:30 PM, Mantas Mikulėnas <grawity@gmail.com> wrote: > The protocol field is checked when deleting IPv4 routes, but ignored for > IPv6, which causes problems with routing daemons accidentally deleting > externally set routes (observed by multiple bird6 users). > > This can be verified using `ip -6 route del <prefix> proto something`. I think this change might have broken userspace deleting routes that were created by RAs. This is because the rtm_protocol returned to userspace is actually synthesized only at route dump time by rt6_fill_node: else if (rt->rt6i_flags & RTF_ADDRCONF) { if (rt->rt6i_flags & (RTF_DEFAULT | RTF_ROUTEINFO)) rtm->rtm_protocol = RTPROT_RA; else rtm->rtm_protocol = RTPROT_KERNEL; } but rt6_add_dflt_router and rt6_add_route_info add the route with a protocol of 0, and 0 is silently upgraded to RTPROT_BOOT by ip6_route_info_create. if (cfg->fc_protocol == RTPROT_UNSPEC) cfg->fc_protocol = RTPROT_BOOT; rt->rt6i_protocol = cfg->fc_protocol; So an app that was previously trying to delete routes looking at rtm_proto, and issuing a delete with whatever rtm_proto was returned by netlink, will result in this check failing because its passed-in protocol (RTPROT_RA or RTPROT_KERNEL) will not match the actual FIB value, which is RTPROT_BOOT. I can't easily test on a vanilla kernel, but on a system running a slightly modified 4.4.63, I see the code fail like this: # ip -6 route show 2001:db8:64::/64 dev nettest100 proto kernel metric 256 expires 291sec fe80::/64 dev nettest100 proto kernel metric 256 default via fe80::6400 dev nettest100 proto ra metric 1024 expires 291sec # ip -6 route flush Failed to send flush request: No such process # ip -6 route show default via fe80::6400 dev nettest100 proto ra metric 1024 expires 286sec If so, it seems unfortunate to have brought this into -stable. For non-stable kernels, it seems that the proper fix would be: 1. Ensure that when an RA creates a route, it properly sets rtm_protocol at time of route creation. 2. When we dump routes to userspace, we don't overwrite the rtm_protocol. I can try to write that up, but I'm not sure why the code doesn't do this already. Perhaps there's a good reason for it. Yoshifuji, Hannes, any thoughts?
On 4/24/17 3:48 AM, Lorenzo Colitti wrote: > For non-stable kernels, it seems that the proper fix would be: > > 1. Ensure that when an RA creates a route, it properly sets > rtm_protocol at time of route creation. > 2. When we dump routes to userspace, we don't overwrite the rtm_protocol. +1
diff --git a/net/ipv6/route.c b/net/ipv6/route.c index 947ed1ded026..ef6de8f9e5a2 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -2163,6 +2163,8 @@ static int ip6_route_del(struct fib6_config *cfg) continue; if (cfg->fc_metric && cfg->fc_metric != rt->rt6i_metric) continue; + if (cfg->fc_protocol && cfg->fc_protocol != rt->rt6i_protocol) + continue; dst_hold(&rt->dst); read_unlock_bh(&table->tb6_lock);
The protocol field is checked when deleting IPv4 routes, but ignored for IPv6, which causes problems with routing daemons accidentally deleting externally set routes (observed by multiple bird6 users). This can be verified using `ip -6 route del <prefix> proto something`. Signed-off-by: Mantas Mikulėnas <grawity@gmail.com> --- net/ipv6/route.c | 2 ++ 1 file changed, 2 insertions(+)