diff mbox series

[net] ip_tunnel: fix use-after-free in ip_tunnel_lookup()

Message ID 20200615150613.21698-1-ap420073@gmail.com
State New
Headers show
Series [net] ip_tunnel: fix use-after-free in ip_tunnel_lookup() | expand

Commit Message

Taehee Yoo June 15, 2020, 3:06 p.m. UTC
In the datapath, the ip_tunnel_lookup() is used and it internally uses
fallback tunnel device pointer, which is fb_tunnel_dev.
This pointer is protected by RTNL. It's not enough to be used
in the datapath.
So, this pointer would be used after an interface is deleted.
It eventually results in the use-after-free problem.

In order to avoid the problem, the new tunnel pointer variable is added,
which indicates a fallback tunnel device's tunnel pointer.
This is protected by both RTNL and RCU.
So, it's safe to be used in the datapath.

Test commands:
    ip netns add A
    ip netns add B
    ip link add eth0 type veth peer name eth1
    ip link set eth0 netns A
    ip link set eth1 netns B

    ip netns exec A ip link set lo up
    ip netns exec A ip link set eth0 up
    ip netns exec A ip link add gre1 type gre local 10.0.0.1 \
	    remote 10.0.0.2
    ip netns exec A ip link set gre1 up
    ip netns exec A ip a a 10.0.100.1/24 dev gre1
    ip netns exec A ip a a 10.0.0.1/24 dev eth0

    ip netns exec B ip link set lo up
    ip netns exec B ip link set eth1 up
    ip netns exec B ip link add gre1 type gre local 10.0.0.2 \
	    remote 10.0.0.1
    ip netns exec B ip link set gre1 up
    ip netns exec B ip a a 10.0.100.2/24 dev gre1
    ip netns exec B ip a a 10.0.0.2/24 dev eth1
    ip netns exec A hping3 10.0.100.2 -2 --flood -d 60000 &
    ip netns del B

Splat looks like:
[  133.319668][    C3] BUG: KASAN: use-after-free in ip_tunnel_lookup+0x9d6/0xde0
[  133.343852][    C3] Read of size 4 at addr ffff8880b1701c84 by task hping3/1222
[  133.344724][    C3]
[  133.345002][    C3] CPU: 3 PID: 1222 Comm: hping3 Not tainted 5.7.0+ #591
[  133.345814][    C3] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
[  133.373336][    C3] Call Trace:
[  133.374792][    C3]  <IRQ>
[  133.375205][    C3]  dump_stack+0x96/0xdb
[  133.375789][    C3]  print_address_description.constprop.6+0x2cc/0x450
[  133.376720][    C3]  ? ip_tunnel_lookup+0x9d6/0xde0
[  133.377431][    C3]  ? ip_tunnel_lookup+0x9d6/0xde0
[  133.378130][    C3]  ? ip_tunnel_lookup+0x9d6/0xde0
[  133.378851][    C3]  kasan_report+0x154/0x190
[  133.379494][    C3]  ? ip_tunnel_lookup+0x9d6/0xde0
[  133.380200][    C3]  ip_tunnel_lookup+0x9d6/0xde0
[  133.380894][    C3]  __ipgre_rcv+0x1ab/0xaa0 [ip_gre]
[  133.381630][    C3]  ? rcu_read_lock_sched_held+0xc0/0xc0
[  133.382429][    C3]  gre_rcv+0x304/0x1910 [ip_gre]
[ ... ]

Fixes: c54419321455 ("GRE: Refactor GRE tunneling code.")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
---
 include/net/ip_tunnels.h |  1 +
 net/ipv4/ip_tunnel.c     | 11 ++++++++---
 2 files changed, 9 insertions(+), 3 deletions(-)
diff mbox series

Patch

diff --git a/include/net/ip_tunnels.h b/include/net/ip_tunnels.h
index 076e5d7db7d3..7442c517bb75 100644
--- a/include/net/ip_tunnels.h
+++ b/include/net/ip_tunnels.h
@@ -164,6 +164,7 @@  struct ip_tunnel_net {
 	struct rtnl_link_ops *rtnl_link_ops;
 	struct hlist_head tunnels[IP_TNL_HASH_SIZE];
 	struct ip_tunnel __rcu *collect_md_tun;
+	struct ip_tunnel __rcu *fb_tun;
 	int type;
 };
 
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index f4f1d11eab50..285b863e2fcc 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -162,8 +162,9 @@  struct ip_tunnel *ip_tunnel_lookup(struct ip_tunnel_net *itn,
 	if (t && t->dev->flags & IFF_UP)
 		return t;
 
-	if (itn->fb_tunnel_dev && itn->fb_tunnel_dev->flags & IFF_UP)
-		return netdev_priv(itn->fb_tunnel_dev);
+	t = rcu_dereference(itn->fb_tun);
+	if (t && t->dev->flags & IFF_UP)
+		return t;
 
 	return NULL;
 }
@@ -1059,6 +1060,7 @@  int ip_tunnel_init_net(struct net *net, unsigned int ip_tnl_net_id,
 		it_init_net = net_generic(&init_net, ip_tnl_net_id);
 		itn->type = it_init_net->type;
 		itn->fb_tunnel_dev = NULL;
+		RCU_INIT_POINTER(itn->fb_tun, NULL);
 		return 0;
 	}
 
@@ -1074,8 +1076,9 @@  int ip_tunnel_init_net(struct net *net, unsigned int ip_tnl_net_id,
 	if (!IS_ERR(itn->fb_tunnel_dev)) {
 		itn->fb_tunnel_dev->features |= NETIF_F_NETNS_LOCAL;
 		itn->fb_tunnel_dev->mtu = ip_tunnel_bind_dev(itn->fb_tunnel_dev);
-		ip_tunnel_add(itn, netdev_priv(itn->fb_tunnel_dev));
 		itn->type = itn->fb_tunnel_dev->type;
+		rcu_assign_pointer(itn->fb_tun,
+				   netdev_priv(itn->fb_tunnel_dev));
 	}
 	rtnl_unlock();
 
@@ -1262,6 +1265,8 @@  void ip_tunnel_uninit(struct net_device *dev)
 	/* fb_tunnel_dev will be unregisted in net-exit call. */
 	if (itn->fb_tunnel_dev != dev)
 		ip_tunnel_del(itn, netdev_priv(dev));
+	else
+		rcu_assign_pointer(itn->fb_tun, NULL);
 
 	dst_cache_reset(&tunnel->dst_cache);
 }