From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Ahern Subject: Re: [PATCH v2 net-next] net/ipv6: Do not allow route add with a device that is down Date: Wed, 24 Jan 2018 10:37:28 -0700 Message-ID: <1df0cbbb-57c5-de89-7cb4-f62db3a6eed0@gmail.com> References: <20180124162924.6984-1-dsahern@gmail.com> <20180124090944.217c14b7@xeon-e3> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, yoshfuji@linux-ipv6.org, idosch@mellanox.com, roopa@cumulusnetworks.com To: Stephen Hemminger Return-path: Received: from mail-pf0-f194.google.com ([209.85.192.194]:44480 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964840AbeAXRha (ORCPT ); Wed, 24 Jan 2018 12:37:30 -0500 Received: by mail-pf0-f194.google.com with SMTP id m26so3621545pfj.11 for ; Wed, 24 Jan 2018 09:37:30 -0800 (PST) In-Reply-To: <20180124090944.217c14b7@xeon-e3> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 1/24/18 10:09 AM, Stephen Hemminger wrote: > On Wed, 24 Jan 2018 08:29:24 -0800 > David Ahern wrote: > >> IPv6 allows routes to be installed when the device is not up (admin up). >> Worse, it does not mark it as LINKDOWN. IPv4 does not allow it and really >> there is no reason for IPv6 to allow it, so check the flags and deny if >> device is admin down. >> >> Signed-off-by: David Ahern >> --- >> v2 >> - missed setting err to -ENETDOWN (thanks for catching that Roopa) >> >> net/ipv6/route.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/net/ipv6/route.c b/net/ipv6/route.c >> index f85da2f1e729..4e8fab766018 100644 >> --- a/net/ipv6/route.c >> +++ b/net/ipv6/route.c >> @@ -2734,6 +2734,12 @@ static struct rt6_info *ip6_route_info_create(struct fib6_config *cfg, >> if (!dev) >> goto out; >> >> + err = -ENETDOWN; >> + if (!(dev->flags & IFF_UP)) { >> + NL_SET_ERR_MSG(extack, "Nexthop device is not up"); >> + goto out; >> + } >> + >> if (!ipv6_addr_any(&cfg->fc_prefsrc)) { >> if (!ipv6_chk_addr(net, &cfg->fc_prefsrc, dev, 0)) { >> NL_SET_ERR_MSG(extack, "Invalid source address"); > > This looks like a good idea. > > There are two equal ways to check for admin up. Either the dev flags or > look at link state via netif_running(). Maybe the latter would > be better. > I used dev->flags for consistency with IPv4. Looking at use of netif_running vs dev->flags in IPv4 and IPv6 code the flag is much more prevalent.