[PATCH 1/1] net-next: do not store needed_headroom in ip_tunnel_xmit

[PATCH 1/1] net-next: do not store needed_headroom in ip_tunnel_xmit

[Francesco Ruggeri]

Misconfigurations can result in local tunnel loops being created.
__dev_queue_xmit catches packets caught in a loop and drops them,
but the affected tunnels' needed_headroom can be corrupted in the
process as it is recursively updated.
The script below can be used to create a loop between two tunnels
and to send packets.

ip link add dummy1 type dummy
ip addr add 1.1.1.1/32 dev dummy1
ip link set dummy1 up

ip link add dummy3 type dummy
ip addr add 3.3.3.3/32 dev dummy3
ip link set dummy3 up

ip tunnel add t1 mode gre local 1.1.1.1 remote 2.2.2.2
ip link set t1 up

ip tunnel add t3 mode gre local 3.3.3.3 remote 4.4.4.4
ip link set t3 up

ip route add 2.2.2.2 dev t3
ip route add 4.4.4.4 dev t1

ping -c 5 2.2.2.2

Signed-off-by: Francesco Ruggeri <fruggeri@arista.com>
---
 net/ipv4/ip_tunnel.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index 4569da7..2eddbe3 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -601,6 +601,7 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 	__be16 df;
 	struct rtable *rt;		/* Route to the other host */
 	unsigned int max_headroom;	/* The extra header space needed */
+	unsigned int needed_headroom;
 	__be32 dst;
 	bool connected;
 
@@ -731,10 +732,11 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
 
 	max_headroom = LL_RESERVED_SPACE(rt->dst.dev) + sizeof(struct iphdr)
 			+ rt->dst.header_len + ip_encap_hlen(&tunnel->encap);
-	if (max_headroom > dev->needed_headroom)
-		dev->needed_headroom = max_headroom;
+	needed_headroom = dev->needed_headroom;
+	if (max_headroom > needed_headroom)
+		needed_headroom = max_headroom;
 
-	if (skb_cow_head(skb, dev->needed_headroom)) {
+	if (skb_cow_head(skb, needed_headroom)) {
 		ip_rt_put(rt);
 		dev->stats.tx_dropped++;
 		kfree_skb(skb);
-- 
1.8.1.4

Re: [PATCH 1/1] net-next: do not store needed_headroom in ip_tunnel_xmit

[Francesco Ruggeri]

On Fri, Feb 19, 2016 at 8:26 PM, Francesco Ruggeri <fruggeri@arista.com> wrote:
> Misconfigurations can result in local tunnel loops being created.
> __dev_queue_xmit catches packets caught in a loop and drops them,
> but the affected tunnels' needed_headroom can be corrupted in the
> process as it is recursively updated.

The problem with the patch is that it might force a pskb_expand_head
in the most common cases.
On the other hand if a loop is created then it may not be enough to
undo the configuration and the tunnel device itself should be
reinitialized.
Anybody have a better suggestion?

Thanks,
Francesco