[PATCH] ipv4: Disallow non-namespace aware protocols to register.

[PATCH] ipv4: Disallow non-namespace aware protocols to register.

[David Miller]

All in-tree ipv4 protocol implementations are now namespace
aware.  Therefore all the run-time checks are superfluous.

Reject registry of any non-namespace aware ipv4 protocol.
Eventually we'll remove prot->netns_ok and this registry
time check as well.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/ipv4/af_inet.c  | 19 -------------------
 net/ipv4/ip_input.c |  7 -------
 net/ipv4/protocol.c |  6 ++++++
 3 files changed, 6 insertions(+), 26 deletions(-)

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 49ddca3..1aec92b 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -263,21 +263,6 @@ void build_ehash_secret(void)
 }
 EXPORT_SYMBOL(build_ehash_secret);
 
-static inline int inet_netns_ok(struct net *net, __u8 protocol)
-{
-	const struct net_protocol *ipprot;
-
-	if (net_eq(net, &init_net))
-		return 1;
-
-	ipprot = rcu_dereference(inet_protos[protocol]);
-	if (ipprot == NULL) {
-		/* raw IP is OK */
-		return 1;
-	}
-	return ipprot->netns_ok;
-}
-
 /*
  *	Create an inet socket.
  */
@@ -350,10 +335,6 @@ lookup_protocol:
 	    !ns_capable(net->user_ns, CAP_NET_RAW))
 		goto out_rcu_unlock;
 
-	err = -EAFNOSUPPORT;
-	if (!inet_netns_ok(net, protocol))
-		goto out_rcu_unlock;
-
 	sock->ops = answer->ops;
 	answer_prot = answer->prot;
 	answer_no_check = answer->no_check;
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index f1395a6..87abd3e 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -208,13 +208,6 @@ static int ip_local_deliver_finish(struct sk_buff *skb)
 		if (ipprot != NULL) {
 			int ret;
 
-			if (!net_eq(net, &init_net) && !ipprot->netns_ok) {
-				net_info_ratelimited("%s: proto %d isn't netns-ready\n",
-						     __func__, protocol);
-				kfree_skb(skb);
-				goto out;
-			}
-
 			if (!ipprot->no_policy) {
 				if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb)) {
 					kfree_skb(skb);
diff --git a/net/ipv4/protocol.c b/net/ipv4/protocol.c
index 0f9d09f..ce84846 100644
--- a/net/ipv4/protocol.c
+++ b/net/ipv4/protocol.c
@@ -37,6 +37,12 @@ const struct net_offload __rcu *inet_offloads[MAX_INET_PROTOS] __read_mostly;
 
 int inet_add_protocol(const struct net_protocol *prot, unsigned char protocol)
 {
+	if (!prot->netns_ok) {
+		pr_err("Protocol %u is not namespace aware, cannot register.\n",
+			protocol);
+		return -EINVAL;
+	}
+
 	return !cmpxchg((const struct net_protocol **)&inet_protos[protocol],
 			NULL, prot) ? 0 : -1;
 }
-- 
1.7.11.7

Re: [PATCH] ipv4: Disallow non-namespace aware protocols to register.

[Eric W. Biederman]

David Miller <davem@davemloft.net> writes:

> All in-tree ipv4 protocol implementations are now namespace
> aware.  Therefore all the run-time checks are superfluous.
>
> Reject registry of any non-namespace aware ipv4 protocol.
> Eventually we'll remove prot->netns_ok and this registry
> time check as well.

It has been a long time coming but this is very cool to see we have
finally made all of ipv4 network namespace aware.

Eric

Re: [PATCH] ipv4: Disallow non-namespace aware protocols to register.

[David Miller]

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Thu, 14 Feb 2013 22:25:26 -0800

> David Miller <davem@davemloft.net> writes:
> 
>> All in-tree ipv4 protocol implementations are now namespace
>> aware.  Therefore all the run-time checks are superfluous.
>>
>> Reject registry of any non-namespace aware ipv4 protocol.
>> Eventually we'll remove prot->netns_ok and this registry
>> time check as well.
> 
> It has been a long time coming but this is very cool to see we have
> finally made all of ipv4 network namespace aware.

BTW, I took a look at ipv6 and unlike ipv4 there seems to be no sanity
checks or per-protocol booleans indicating proper netns support.

Is my interpretation right that ipv6 just assumes all registered
protocols are netns aware at this point?

If so that was definitely a bug, because things like l2tp have an
ipv6 component and were not fully netns aware until very recently.

Re: [PATCH] ipv4: Disallow non-namespace aware protocols to register.

[Eric W. Biederman]

David Miller <davem@davemloft.net> writes:

> From: ebiederm@xmission.com (Eric W. Biederman)
> Date: Thu, 14 Feb 2013 22:25:26 -0800
>
>> David Miller <davem@davemloft.net> writes:
>> 
>>> All in-tree ipv4 protocol implementations are now namespace
>>> aware.  Therefore all the run-time checks are superfluous.
>>>
>>> Reject registry of any non-namespace aware ipv4 protocol.
>>> Eventually we'll remove prot->netns_ok and this registry
>>> time check as well.
>> 
>> It has been a long time coming but this is very cool to see we have
>> finally made all of ipv4 network namespace aware.
>
> BTW, I took a look at ipv6 and unlike ipv4 there seems to be no sanity
> checks or per-protocol booleans indicating proper netns support.
>
> Is my interpretation right that ipv6 just assumes all registered
> protocols are netns aware at this point?

It looks like when the ipv6 network namespace work was done work that
check was not added to the ipv6 code :( I skimmed through the history
and I don't see any signs that anything was every done with struct
inet6_protocol.  Nor when I looked at the addition of netns support to
the ipv6 udp code were there any switches flipped.

> If so that was definitely a bug, because things like l2tp have an
> ipv6 component and were not fully netns aware until very recently.

Agreed it was a bug.

I have just read through all of the handlers registered with
inet6_add_protocol in my 3.8 development tree and it appears that
everything except l2tp has network namespace support.  And l2tp is fixed
in net-next so we appear to be good now.

Eric

Re: [PATCH] ipv4: Disallow non-namespace aware protocols to register.

[David Miller]

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Fri, 15 Feb 2013 12:05:18 -0800

> David Miller <davem@davemloft.net> writes:
> 
>> From: ebiederm@xmission.com (Eric W. Biederman)
>> Date: Thu, 14 Feb 2013 22:25:26 -0800
>>
>>> David Miller <davem@davemloft.net> writes:
>>> 
>>>> All in-tree ipv4 protocol implementations are now namespace
>>>> aware.  Therefore all the run-time checks are superfluous.
>>>>
>>>> Reject registry of any non-namespace aware ipv4 protocol.
>>>> Eventually we'll remove prot->netns_ok and this registry
>>>> time check as well.
>>> 
>>> It has been a long time coming but this is very cool to see we have
>>> finally made all of ipv4 network namespace aware.
>>
>> BTW, I took a look at ipv6 and unlike ipv4 there seems to be no sanity
>> checks or per-protocol booleans indicating proper netns support.
>>
>> Is my interpretation right that ipv6 just assumes all registered
>> protocols are netns aware at this point?
> 
> It looks like when the ipv6 network namespace work was done work that
> check was not added to the ipv6 code :( I skimmed through the history
> and I don't see any signs that anything was every done with struct
> inet6_protocol.  Nor when I looked at the addition of netns support to
> the ipv6 udp code were there any switches flipped.
> 
>> If so that was definitely a bug, because things like l2tp have an
>> ipv6 component and were not fully netns aware until very recently.
> 
> Agreed it was a bug.
> 
> I have just read through all of the handlers registered with
> inet6_add_protocol in my 3.8 development tree and it appears that
> everything except l2tp has network namespace support.  And l2tp is fixed
> in net-next so we appear to be good now.

Thanks for confirming my analysis.