Skip to content

Commit 951f788

Browse files
edumazetdavem330
authored andcommitted
ipv6: fix a BUG in rt6_get_pcpu_route()
Ido reported following splat and provided a patch. [ 122.221814] BUG: using smp_processor_id() in preemptible [00000000] code: sshd/2672 [ 122.221845] caller is debug_smp_processor_id+0x17/0x20 [ 122.221866] CPU: 0 PID: 2672 Comm: sshd Not tainted 4.14.0-rc3-idosch-next-custom #639 [ 122.221880] Hardware name: Mellanox Technologies Ltd. MSN2100-CB2FO/SA001017, BIOS 5.6.5 06/07/2016 [ 122.221893] Call Trace: [ 122.221919] dump_stack+0xb1/0x10c [ 122.221946] ? _atomic_dec_and_lock+0x124/0x124 [ 122.221974] ? ___ratelimit+0xfe/0x240 [ 122.222020] check_preemption_disabled+0x173/0x1b0 [ 122.222060] debug_smp_processor_id+0x17/0x20 [ 122.222083] ip6_pol_route+0x1482/0x24a0 ... I believe we can simplify this code path a bit, since we no longer hold a read_lock and need to release it to avoid a dead lock. By disabling BH, we make sure we'll prevent code re-entry and rt6_get_pcpu_route()/rt6_make_pcpu_route() run on the same cpu. Fixes: 66f5d6c ("ipv6: replace rwlock with rcu and spinlock in fib6_table") Reported-by: Ido Schimmel <[email protected]> Signed-off-by: Eric Dumazet <[email protected]> Tested-by: Ido Schimmel <[email protected]> Signed-off-by: David S. Miller <[email protected]>
1 parent 51a0c00 commit 951f788

File tree

1 file changed

+6
-20
lines changed

1 file changed

+6
-20
lines changed

net/ipv6/route.c

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1136,15 +1136,7 @@ static struct rt6_info *rt6_make_pcpu_route(struct rt6_info *rt)
11361136
dst_hold(&pcpu_rt->dst);
11371137
p = this_cpu_ptr(rt->rt6i_pcpu);
11381138
prev = cmpxchg(p, NULL, pcpu_rt);
1139-
if (prev) {
1140-
/* If someone did it before us, return prev instead */
1141-
/* release refcnt taken by ip6_rt_pcpu_alloc() */
1142-
dst_release_immediate(&pcpu_rt->dst);
1143-
/* release refcnt taken by above dst_hold() */
1144-
dst_release_immediate(&pcpu_rt->dst);
1145-
dst_hold(&prev->dst);
1146-
pcpu_rt = prev;
1147-
}
1139+
BUG_ON(prev);
11481140

11491141
rt6_dst_from_metrics_check(pcpu_rt);
11501142
return pcpu_rt;
@@ -1739,31 +1731,25 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
17391731
struct rt6_info *pcpu_rt;
17401732

17411733
dst_use_noref(&rt->dst, jiffies);
1734+
local_bh_disable();
17421735
pcpu_rt = rt6_get_pcpu_route(rt);
17431736

1744-
if (pcpu_rt) {
1745-
rcu_read_unlock();
1746-
} else {
1737+
if (!pcpu_rt) {
17471738
/* atomic_inc_not_zero() is needed when using rcu */
17481739
if (atomic_inc_not_zero(&rt->rt6i_ref)) {
1749-
/* We have to do the read_unlock first
1750-
* because rt6_make_pcpu_route() may trigger
1751-
* ip6_dst_gc() which will take the write_lock.
1752-
*
1753-
* No dst_hold() on rt is needed because grabbing
1740+
/* No dst_hold() on rt is needed because grabbing
17541741
* rt->rt6i_ref makes sure rt can't be released.
17551742
*/
1756-
rcu_read_unlock();
17571743
pcpu_rt = rt6_make_pcpu_route(rt);
17581744
rt6_release(rt);
17591745
} else {
17601746
/* rt is already removed from tree */
1761-
rcu_read_unlock();
17621747
pcpu_rt = net->ipv6.ip6_null_entry;
17631748
dst_hold(&pcpu_rt->dst);
17641749
}
17651750
}
1766-
1751+
local_bh_enable();
1752+
rcu_read_unlock();
17671753
trace_fib6_table_lookup(net, pcpu_rt, table->tb6_id, fl6);
17681754
return pcpu_rt;
17691755
}

0 commit comments

Comments
 (0)