Skip to contentSkip to conversation

ip_tunnel fixes

PATCH In review Thomas Graf 3 participants 6 messages Dec 16, 2014

Subsystems (3)
Networking Network drivers IPv4/IPv6
lore
Share & tools
lore

Thomas Graf

author
#0 lore raw

cover ip_tunnel fixes

Thomas Graf (2):
ip_tunnel: Add sanity checks to ip_tunnel_encap_add_ops()
ip_tunnel: Add missing validation of encap type to
ip_tunnel_encap_setup()

 net/ipv4/ip_tunnel.c | 9 +++++++++
 1 file changed, 9 insertions(+)
Signature · 1 line
1.9.3
author
#1 lore raw

PATCH 1/2 ip_tunnel: Add sanity checks to ip_tunnel_encap_add_ops()

The symbols are exported and could be used by external modules.

Fixes a8c5f9 ("ip_tunnel: Ops registration for secondary encap (fou, gue)")
Signed-off-by Thomas Graf <[email protected]>
---
 net/ipv4/ip_tunnel.c | 6 ++++++
 1 file changed, 6 insertions(+)
net/ipv4/ip_tunnel.c +6 -0
index 63e745a..2f498f8 100644
@@ -514,6 +514,9 @@ const struct ip_tunnel_encap_ops __rcu *
 int ip_tunnel_encap_add_ops(const struct ip_tunnel_encap_ops *ops,
 			    unsigned int num)
 {
+	if (num >= MAX_IPTUN_ENCAP_OPS)
+		return -ERANGE;
+
 	return !cmpxchg((const struct ip_tunnel_encap_ops **)
 			&iptun_encaps[num],
 			NULL, ops) ? 0 : -1;

@@ -525,6 +528,9 @@ int ip_tunnel_encap_del_ops(const struct ip_tunnel_encap_ops *ops,
 {
 	int ret;
 
+	if (num >= MAX_IPTUN_ENCAP_OPS)
+		return -ERANGE;
+
 	ret = (cmpxchg((const struct ip_tunnel_encap_ops **)
 		       &iptun_encaps[num],
 		       ops, NULL) == ops) ? 0 : -1;
Signature · 1 line
1.9.3
author
#2 lore raw

PATCH 2/2 ip_tunnel: Add missing validation of encap type to ip_tunnel_encap_setup()

In reply to Thomas Graf

The encap->type comes straight from Netlink. Validate it against max supported encap types just like ip_encap_hlen() already does.

Fixes a8c5f9 ("ip_tunnel: Ops registration for secondary encap (fou, gue)")
Signed-off-by Thomas Graf <[email protected]>
---
 net/ipv4/ip_tunnel.c | 3 +++
 1 file changed, 3 insertions(+)
net/ipv4/ip_tunnel.c +3 -0
index 2f498f8..d3e4479 100644
@@ -573,6 +573,9 @@ int ip_tunnel_encap(struct sk_buff *skb, struct ip_tunnel *t,
 	if (t->encap.type == TUNNEL_ENCAP_NONE)
 		return 0;
 
+	if (t->encap.type >= MAX_IPTUN_ENCAP_OPS)
+		return -EINVAL;
+
 	rcu_read_lock();
 	ops = rcu_dereference(iptun_encaps[t->encap.type]);
 	if (likely(ops && ops->build_header))
Signature · 1 line
1.9.3
#3 lore raw

Re: [PATCH 2/2] ip_tunnel: Add missing validation of encap type to ip_tunnel_encap_setup()

Quoted 14 lines · Thomas Graf

The encap->type comes straight from Netlink. Validate it against max supported encap types just like ip_encap_hlen() already does.

Fixes a8c5f9 ("ip_tunnel: Ops registration for secondary encap (fou, gue)")
Signed-off-by Thomas Graf <[email protected]>
---
 net/ipv4/ip_tunnel.c | 3 +++
 1 file changed, 3 insertions(+)
net/ipv4/ip_tunnel.c +0 -0
index 2f498f8..d3e4479 100644

net/ipv4/ip_tunnel.c int ip_tunnel_encap(struct sk_buff *skb, struct ip_tunnel *t, · line 578

Quoted 6 lines
net/ipv4/ip_tunnel.c +3 -0
@@ -573,6 +573,9 @@ int ip_tunnel_encap(struct sk_buff *skb, struct ip_tunnel *t,
        if (t->encap.type == TUNNEL_ENCAP_NONE)
                return 0;
 
+       if (t->encap.type >= MAX_IPTUN_ENCAP_OPS)
+               return -EINVAL;
+

I don't think this is technically needed, we should have already verified the type when setting up the tunnel (ip_encap_hlen).

Quoted 6 lines
net/ipv4/ip_tunnel.c +0 -1
        rcu_read_lock();
        ops = rcu_dereference(iptun_encaps[t->encap.type]);
        if (likely(ops && ops->build_header))
--

1.9.3

author
#4 lore raw

Re: [PATCH 2/2] ip_tunnel: Add missing validation of encap type to ip_tunnel_encap_setup()

Quoted 18 lines · Tom Herbert
Quoted 17 lines · Thomas Graf

The encap->type comes straight from Netlink. Validate it against max supported encap types just like ip_encap_hlen() already does.

Fixes a8c5f9 ("ip_tunnel: Ops registration for secondary encap (fou, gue)")
Signed-off-by Thomas Graf <[email protected]>
---
 net/ipv4/ip_tunnel.c | 3 +++
 1 file changed, 3 insertions(+)
net/ipv4/ip_tunnel.c +0 -0
index 2f498f8..d3e4479 100644
@@ -573,6 +573,9 @@ int ip_tunnel_encap(struct sk_buff *skb, struct ip_tunnel *t,
        if (t->encap.type == TUNNEL_ENCAP_NONE)
                return 0;
Quoted 6 lines

net/ipv4/ip_tunnel.c int ip_tunnel_encap(struct sk_buff *skb, struct ip_tunnel *t, · line 578

Quoted 3 lines
net/ipv4/ip_tunnel.c +3 -0
+       if (t->encap.type >= MAX_IPTUN_ENCAP_OPS)
+               return -EINVAL;
+

I don't think this is technically needed, we should have already verified the type when setting up the tunnel (ip_encap_hlen).

Right, assuming that every API user always calls ip_tunnel_encap_setup() on changelink. It's currently the case but since this is a exported API I figured we better be safe than sorry, in particular as ip_tunnel_encap() is called before ip_encap_hlen() on xmit.

maintainer
#5 lore raw
In reply to Thomas Graf

From: Thomas Graf <[email protected]>
Date: Tue, 16 Dec 2014 21:05:19 +0100

Quoted 4 lines

Thomas Graf (2):
ip_tunnel: Add sanity checks to ip_tunnel_encap_add_ops()
ip_tunnel: Add missing validation of encap type to
ip_tunnel_encap_setup()

Both applied, thanks Thomas.