netdev - Re: [PATCH net] rtnetlink: Fail dump if target netnsid is invalid

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20181002120431.0817f48e@redhat.com>
Date:   Tue, 2 Oct 2018 12:04:31 +0200
From:   Jiri Benc <jbenc@...hat.com>
To:     David Ahern <dsahern@...nel.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net,
        David Ahern <dsahern@...il.com>
Subject: Re: [PATCH net] rtnetlink: Fail dump if target netnsid is invalid

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.

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).

Thanks!

 Jiri