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

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

[David Ahern]

From: David Ahern <dsahern@gmail.com>

Link dumps can return results from a target namespace. If the namespace id
is invalid, then the dump request should fail if get_target_net fails
rather than continuing with a dump of the current namespace.

Fixes: 79e1ad148c844 ("rtnetlink: use netnsid to query interface")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/core/rtnetlink.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 63ce2283a456..7f37fe9c65a5 100644
--- 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])
-- 
2.11.0

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

[David Miller]

From: David Ahern <dsahern@kernel.org>
Date: Fri, 28 Sep 2018 12:28:41 -0700

> From: David Ahern <dsahern@gmail.com>
> 
> Link dumps can return results from a target namespace. If the namespace id
> is invalid, then the dump request should fail if get_target_net fails
> rather than continuing with a dump of the current namespace.
> 
> Fixes: 79e1ad148c844 ("rtnetlink: use netnsid to query interface")
> Signed-off-by: David Ahern <dsahern@gmail.com>

Applied and queued up for -stable, thanks.

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

[Jiri Benc]

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

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

[David Ahern]

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.