From mboxrd@z Thu Jan 1 00:00:00 1970 From: David Ahern Subject: Re: [PATCH net] rtnetlink: Fail dump if target netnsid is invalid Date: Tue, 2 Oct 2018 08:41:48 -0600 Message-ID: References: <20180928192841.20410-1-dsahern@kernel.org> <20181002120431.0817f48e@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: netdev@vger.kernel.org, davem@davemloft.net To: Jiri Benc , David Ahern Return-path: Received: from mail-pl1-f195.google.com ([209.85.214.195]:33997 "EHLO mail-pl1-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727562AbeJBVZf (ORCPT ); Tue, 2 Oct 2018 17:25:35 -0400 Received: by mail-pl1-f195.google.com with SMTP id f18-v6so1759178plr.1 for ; Tue, 02 Oct 2018 07:41:52 -0700 (PDT) In-Reply-To: <20181002120431.0817f48e@redhat.com> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 10/2/18 4:04 AM, Jiri Benc wrote: > On Fri, 28 Sep 2018 12:28:41 -0700, David Ahern wrote: >> --- a/net/core/rtnetlink.c >> +++ b/net/core/rtnetlink.c >> @@ -1898,10 +1898,8 @@ static int rtnl_dump_ifinfo(struct sk_buff *skb, struct netlink_callback *cb) >> if (tb[IFLA_IF_NETNSID]) { >> netnsid = nla_get_s32(tb[IFLA_IF_NETNSID]); >> tgt_net = get_target_net(skb->sk, netnsid); >> - if (IS_ERR(tgt_net)) { >> - tgt_net = net; >> - netnsid = -1; >> - } >> + if (IS_ERR(tgt_net)) >> + return PTR_ERR(tgt_net); >> } >> >> if (tb[IFLA_EXT_MASK]) > > Sorry for the late review, I see it has been applied. > > I intentionally chose the behavior to preserve the behavior of the > older kernels: that attribute was silently ignored. Note that the > IFLA_IF_NETNSID is not returned in such case, thus it's easy to > distinguish that it was not applied. And the user space has to do such > check anyway to support old kernels. First, rtnl_dellink, rtnl_newlink and rtnl_getlink all fail if net namespace id is invalid. Second, the user is requesting data from a target namespace and the dump happily continued with the current namespace which is not what the user requested. Hence, it makes no sense for a dump to continue which is why I sent the patch. > > But you're right that there was no way to distinguish "the kernel does > not support IFLA_IF_NETNSID" from "wrong IFLA_IF_NETNSID provided". I'm > okay with the patch, I just don't think the "Fixes" tag is justified but > whatever, can't be unapplied :-) (and it's my fault for not reviewing > the patches timely). The id was the commit that added the code to ignore the error.