Skip to content

Commit

Permalink
ip_tunnel: fix use-after-free in ip_tunnel_lookup()
Browse files Browse the repository at this point in the history
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+ torvalds#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: c544193 ("GRE: Refactor GRE tunneling code.")
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
  • Loading branch information
TaeheeYoo authored and intel-lab-lkp committed Jun 15, 2020
1 parent 96144c5 commit acfa08d
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 3 deletions.
1 change: 1 addition & 0 deletions include/net/ip_tunnels.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down
11 changes: 8 additions & 3 deletions net/ipv4/ip_tunnel.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}

Expand All @@ -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();

Expand Down Expand Up @@ -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);
}
Expand Down

0 comments on commit acfa08d

Please sign in to comment.