Skip to content

Commit fe3d1f0

Browse files
tracywwnjonettboots
tracywwnj
authored and
onettboots
committed
BACKPORT: ipv6: use fib6_info_hold_safe() when necessary
In the code path where only rcu read lock is held, e.g. in the route lookup code path, it is not safe to directly call fib6_info_hold() because the fib6_info may already have been deleted but still exists in the rcu grace period. Holding reference to it could cause double free and crash the kernel. This patch adds a new function fib6_info_hold_safe() and replace fib6_info_hold() in all necessary places. Syzbot reported 3 crash traces because of this. One of them is: 8021q: adding VLAN 0 to HW filter on device team0 IPv6: ADDRCONF(NETDEV_CHANGE): team0: link becomes ready dst_release: dst:(____ptrval____) refcnt:-1 dst_release: dst:(____ptrval____) refcnt:-2 WARNING: CPU: 1 PID: 4845 at include/net/dst.h:239 dst_hold include/net/dst.h:239 [inline] WARNING: CPU: 1 PID: 4845 at include/net/dst.h:239 ip6_setup_cork+0xd66/0x1830 net/ipv6/ip6_output.c:1204 dst_release: dst:(____ptrval____) refcnt:-1 Kernel panic - not syncing: panic_on_warn set ... CPU: 1 PID: 4845 Comm: syz-executor493 Not tainted 4.18.0-rc3+ #10 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113 panic+0x238/0x4e7 kernel/panic.c:184 dst_release: dst:(____ptrval____) refcnt:-2 dst_release: dst:(____ptrval____) refcnt:-3 __warn.cold.8+0x163/0x1ba kernel/panic.c:536 dst_release: dst:(____ptrval____) refcnt:-4 report_bug+0x252/0x2d0 lib/bug.c:186 fixup_bug arch/x86/kernel/traps.c:178 [inline] do_error_trap+0x1fc/0x4d0 arch/x86/kernel/traps.c:296 dst_release: dst:(____ptrval____) refcnt:-5 do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316 invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:992 RIP: 0010:dst_hold include/net/dst.h:239 [inline] RIP: 0010:ip6_setup_cork+0xd66/0x1830 net/ipv6/ip6_output.c:1204 Code: c1 ed 03 89 9d 18 ff ff ff 48 b8 00 00 00 00 00 fc ff df 41 c6 44 05 00 f8 e9 2d 01 00 00 4c 8b a5 c8 fe ff ff e8 1a f6 e6 fa <0f> 0b e9 6a fc ff ff e8 0e f6 e6 fa 48 8b 85 d0 fe ff ff 48 8d 78 RSP: 0018:ffff8801a8fcf178 EFLAGS: 00010293 RAX: ffff8801a8eba5c0 RBX: 0000000000000000 RCX: ffffffff869511e6 RDX: 0000000000000000 RSI: ffffffff869515b6 RDI: 0000000000000005 RBP: ffff8801a8fcf2c8 R08: ffff8801a8eba5c0 R09: ffffed0035ac8338 R10: ffffed0035ac8338 R11: ffff8801ad6419c3 R12: ffff8801a8fcf720 R13: ffff8801a8fcf6a0 R14: ffff8801ad6419c0 R15: ffff8801ad641980 ip6_make_skb+0x2c8/0x600 net/ipv6/ip6_output.c:1768 udpv6_sendmsg+0x2c90/0x35f0 net/ipv6/udp.c:1376 inet_sendmsg+0x1a1/0x690 net/ipv4/af_inet.c:798 sock_sendmsg_nosec net/socket.c:641 [inline] sock_sendmsg+0xd5/0x120 net/socket.c:651 ___sys_sendmsg+0x51d/0x930 net/socket.c:2125 __sys_sendmmsg+0x240/0x6f0 net/socket.c:2220 __do_sys_sendmmsg net/socket.c:2249 [inline] __se_sys_sendmmsg net/socket.c:2246 [inline] __x64_sys_sendmmsg+0x9d/0x100 net/socket.c:2246 do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x446ba9 Code: e8 cc bb 02 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 eb 08 fc ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:00007fb39a469da8 EFLAGS: 00000246 ORIG_RAX: 0000000000000133 RAX: ffffffffffffffda RBX: 00000000006dcc54 RCX: 0000000000446ba9 RDX: 00000000000000b8 RSI: 0000000020001b00 RDI: 0000000000000003 RBP: 00000000006dcc50 R08: 00007fb39a46a700 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000246 R12: 45c828efc7a64843 R13: e6eeb815b9d8a477 R14: 5068caf6f713c6fc R15: 0000000000000001 Dumping ftrace buffer: (ftrace buffer empty) Kernel Offset: disabled Rebooting in 86400 seconds.. Fixes: 93531c6 ("net/ipv6: separate handling of FIB entries from dst based routes") Reported-by: syzbot+902e2a1bcd4f7808cef5@syzkaller.appspotmail.com Reported-by: syzbot+8ae62d67f647abeeceb9@syzkaller.appspotmail.com Reported-by: syzbot+3f08feb14086930677d0@syzkaller.appspotmail.com Change-Id: Iff6c4861cdd0aaf75bb3f1ba4159b8d4c1e0f830 Signed-off-by: Wei Wang <weiwan@google.com> Acked-by: Eric Dumazet <edumazet@google.com> Reviewed-by: David Ahern <dsahern@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net> Signed-off-by: onettboots <blackcocopet@gmail.com>
1 parent 87d8d1c commit fe3d1f0

3 files changed

Lines changed: 36 additions & 9 deletions

File tree

‎include/net/ip6_fib.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,11 @@ static inline void fib6_info_hold(struct fib6_info *f6i)
280280
atomic_inc(&f6i->fib6_ref);
281281
}
282282

283+
static inline bool fib6_info_hold_safe(struct fib6_info *f6i)
284+
{
285+
return atomic_inc_not_zero(&f6i->fib6_ref);
286+
}
287+
283288
static inline void fib6_info_release(struct fib6_info *f6i)
284289
{
285290
if (f6i && atomic_dec_and_test(&f6i->fib6_ref))

‎net/ipv6/addrconf.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2379,7 +2379,8 @@ static struct fib6_info *addrconf_get_prefix_route(const struct in6_addr *pfx,
23792379
continue;
23802380
if ((rt->fib6_flags & noflags) != 0)
23812381
continue;
2382-
fib6_info_hold(rt);
2382+
if (!fib6_info_hold_safe(rt))
2383+
continue;
23832384
break;
23842385
}
23852386
out:

‎net/ipv6/route.c‎

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -969,10 +969,10 @@ static void ip6_rt_init_dst(struct rt6_info *rt, struct fib6_info *ort)
969969
rt->dst.lastuse = jiffies;
970970
}
971971

972+
/* Caller must already hold reference to @from */
972973
static void rt6_set_from(struct rt6_info *rt, struct fib6_info *from)
973974
{
974975
rt->rt6i_flags &= ~RTF_EXPIRES;
975-
fib6_info_hold(from);
976976
rcu_assign_pointer(rt->from, from);
977977
dst_init_metrics(&rt->dst, from->fib6_metrics->metrics, true);
978978
if (from->fib6_metrics != &dst_default_metrics) {
@@ -981,6 +981,7 @@ static void rt6_set_from(struct rt6_info *rt, struct fib6_info *from)
981981
}
982982
}
983983

984+
/* Caller must already hold reference to @ort */
984985
static void ip6_rt_copy_init(struct rt6_info *rt, struct fib6_info *ort)
985986
{
986987
struct net_device *dev = fib6_info_nh_dev(ort);
@@ -1041,9 +1042,14 @@ static struct rt6_info *ip6_create_rt_rcu(struct fib6_info *rt)
10411042
struct net_device *dev = rt->fib6_nh.nh_dev;
10421043
struct rt6_info *nrt;
10431044

1045+
if (!fib6_info_hold_safe(rt))
1046+
return NULL;
1047+
10441048
nrt = ip6_dst_alloc(dev_net(dev), dev, flags);
10451049
if (nrt)
10461050
ip6_rt_copy_init(nrt, rt);
1051+
else
1052+
fib6_info_release(rt);
10471053

10481054
return nrt;
10491055
}
@@ -1175,10 +1181,15 @@ static struct rt6_info *ip6_rt_cache_alloc(struct fib6_info *ort,
11751181
* Clone the route.
11761182
*/
11771183

1184+
if (!fib6_info_hold_safe(ort))
1185+
return NULL;
1186+
11781187
dev = ip6_rt_get_dev_rcu(ort);
11791188
rt = ip6_dst_alloc(dev_net(dev), dev, 0);
1180-
if (!rt)
1189+
if (!rt) {
1190+
fib6_info_release(ort);
11811191
return NULL;
1192+
}
11821193

11831194
ip6_rt_copy_init(rt, ort);
11841195
rt->rt6i_flags |= RTF_CACHE;
@@ -1207,12 +1218,17 @@ static struct rt6_info *ip6_rt_pcpu_alloc(struct fib6_info *rt)
12071218
struct net_device *dev;
12081219
struct rt6_info *pcpu_rt;
12091220

1221+
if (!fib6_info_hold_safe(rt))
1222+
return NULL;
1223+
12101224
rcu_read_lock();
12111225
dev = ip6_rt_get_dev_rcu(rt);
12121226
pcpu_rt = ip6_dst_alloc(dev_net(dev), dev, flags);
12131227
rcu_read_unlock();
1214-
if (!pcpu_rt)
1228+
if (!pcpu_rt) {
1229+
fib6_info_release(rt);
12151230
return NULL;
1231+
}
12161232
ip6_rt_copy_init(pcpu_rt, rt);
12171233
pcpu_rt->rt6i_flags |= RTF_PCPU;
12181234
return pcpu_rt;
@@ -2466,7 +2482,7 @@ static struct rt6_info *__ip6_route_redirect(struct net *net,
24662482

24672483
out:
24682484
if (ret)
2469-
dst_hold(&ret->dst);
2485+
ip6_hold_safe(net, &ret, true);
24702486
else
24712487
ret = ip6_create_rt_rcu(rt);
24722488

@@ -3258,7 +3274,8 @@ static int ip6_route_del(struct fib6_config *cfg,
32583274
continue;
32593275
if (cfg->fc_protocol && cfg->fc_protocol != rt->fib6_protocol)
32603276
continue;
3261-
fib6_info_hold(rt);
3277+
if (!fib6_info_hold_safe(rt))
3278+
continue;
32623279
rcu_read_unlock();
32633280

32643281
/* if gateway was specified only delete the one hop */
@@ -3364,6 +3381,9 @@ static void rt6_do_redirect(struct dst_entry *dst, struct sock *sk, struct sk_bu
33643381

33653382
rcu_read_lock();
33663383
from = rcu_dereference(rt->from);
3384+
/* This fib6_info_hold() is safe here because we hold reference to rt
3385+
* and rt already holds reference to fib6_info.
3386+
*/
33673387
fib6_info_hold(from);
33683388
rcu_read_unlock();
33693389

@@ -3424,7 +3444,8 @@ static struct fib6_info *rt6_get_route_info(struct net *net,
34243444
continue;
34253445
if (!ipv6_addr_equal(&rt->fib6_nh.nh_gw, gwaddr))
34263446
continue;
3427-
fib6_info_hold(rt);
3447+
if (!fib6_info_hold_safe(rt))
3448+
continue;
34283449
break;
34293450
}
34303451
out:
@@ -3484,8 +3505,8 @@ struct fib6_info *rt6_get_dflt_router(struct net *net,
34843505
ipv6_addr_equal(&rt->fib6_nh.nh_gw, addr))
34853506
break;
34863507
}
3487-
if (rt)
3488-
fib6_info_hold(rt);
3508+
if (rt && !fib6_info_hold_safe(rt))
3509+
rt = NULL;
34893510
rcu_read_unlock();
34903511
return rt;
34913512
}

0 commit comments

Comments
 (0)