syzbot reported a null-ptr-deref in __ip6_del_rt_siblings() [0].
The stack trace hinted towards a null dereference of rt->fib6_node when
fn->leaf is accessed in __ip6_del_rt_siblings(). With
RTNL_FLAG_DOIT_UNLOCKED set, inet6_rtm_delroute() operations run
concurrently without acquiring the RTNL lock. In ip6_route_del(), the
route lookup happens under rcu_read_lock() without acquiring
table->tb6_lock.
Between ip6_route_del() looking up the route and __ip6_del_rt_siblings()
acquiring table->tb6_lock, another thread can modify the routing table.
For example, when an ECMP route is replaced via RTM_NEWROUTE with
NLM_F_REPLACE, fib6_add_rt2node() unlinks all old siblings and sets
iter->fib6_node = NULL. A reproducer was found that triggers this [1].
Add a check to ensure rt->fib6_node is non-null before accessing it.
[0]
KASAN: null-ptr-deref in range [0x0000000000000020-0x0000000000000027]
RIP: 0010:__ip6_del_rt_siblings+0x31e/0x7c0 net/ipv6/route.c:4056
Call Trace:
ip6_route_del+0x1054/0x1110 net/ipv6/route.c:4232
inet6_rtm_delroute+0x5d7/0x6d0 net/ipv6/route.c:5669
rtnetlink_rcv_msg+0x802/0xc00 net/core/rtnetlink.c:7132
netlink_rcv_skb+0x226/0x4a0 net/netlink/af_netlink.c:2556
netlink_unicast_kernel net/netlink/af_netlink.c:1319 [inline]
netlink_unicast+0x7f5/0x990 net/netlink/af_netlink.c:1345
netlink_sendmsg+0x813/0xb40 net/netlink/af_netlink.c:1900
sock_sendmsg_nosec+0x13a/0x180 net/socket.c:800
__sock_sendmsg net/socket.c:815 [inline]
____sys_sendmsg+0x565/0x870 net/socket.c:2713
___sys_sendmsg+0x2a5/0x360 net/socket.c:2767
__sys_sendmsg net/socket.c:2799 [inline]
__do_sys_sendmsg net/socket.c:2804 [inline]
__se_sys_sendmsg net/socket.c:2802 [inline]
__x64_sys_sendmsg+0x1b7/0x290 net/socket.c:2802
do_syscall_x64 arch/x86/entry/syscall_64.c:61 [inline]
do_syscall_64+0x166/0x520 arch/x86/entry/syscall_64.c:84
entry_SYSCALL_64_after_hwframe+0x77/0x7f
[1] https://gist.github.com/NamanGulati/0766a1159b6ca61928faaf87425ff899
Fixes: bd11ff421d36 ("ipv6: Get rid of RTNL for SIOCDELRT and RTM_DELROUTE.")
Reported-by: syzbot+a73e5ee0fd534fed75bd@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/netdev/6a9b03f9.04649fcc.10325f.0003.GAE@google.com
Signed-off-by: Naman Gulati
Reviewed-by: Kuniyuki Iwashima
---
net/ipv6/route.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 9658939511e0..08bd68f1b5bb 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4019,6 +4019,7 @@ static int __ip6_del_rt_siblings(struct fib6_info *rt, struct fib6_config *cfg)
struct net *net = info->nl_net;
struct sk_buff *skb = NULL;
struct fib6_table *table;
+ struct fib6_node *fn;
int err = -ENOENT;
if (rt == net->ipv6.fib6_null_entry)
@@ -4026,9 +4027,13 @@ static int __ip6_del_rt_siblings(struct fib6_info *rt, struct fib6_config *cfg)
table = rt->fib6_table;
spin_lock_bh(&table->tb6_lock);
+ fn = rcu_dereference_protected(rt->fib6_node,
+ lockdep_is_held(&table->tb6_lock));
+ if (!fn)
+ goto out_unlock;
+
if (rt->fib6_nsiblings && cfg->fc_delete_all_nh) {
struct fib6_info *sibling, *next_sibling;
- struct fib6_node *fn;
/* prefer to send a single notification with all hops */
skb = nlmsg_new(rt6_nlmsg_size(rt), GFP_ATOMIC);
@@ -4051,8 +4056,6 @@ static int __ip6_del_rt_siblings(struct fib6_info *rt, struct fib6_config *cfg)
* and emit a replace or delete notification, respectively.
*/
info->skip_notify_kernel = 1;
- fn = rcu_dereference_protected(rt->fib6_node,
- lockdep_is_held(&table->tb6_lock));
if (rcu_access_pointer(fn->leaf) == rt) {
struct fib6_info *last_sibling, *replace_rt;
--
2.55.0.979.g7e5102b832-goog