Skip to content

Commit 418a730

Browse files
mbizonfreeboxkuba-moo
authored andcommitted
net: dst: fix missing initialization of rt_uncached
xfrm_alloc_dst() followed by xfrm4_dst_destroy(), without a xfrm4_fill_dst() call in between, causes the following BUG: BUG: spinlock bad magic on CPU#0, fbxhostapd/732 lock: 0x890b7668, .magic: 890b7668, .owner: <none>/-1, .owner_cpu: 0 CPU: 0 PID: 732 Comm: fbxhostapd Not tainted 6.3.0-rc6-next-20230414-00613-ge8de66369925-dirty #9 Hardware name: Marvell Kirkwood (Flattened Device Tree) unwind_backtrace from show_stack+0x10/0x14 show_stack from dump_stack_lvl+0x28/0x30 dump_stack_lvl from do_raw_spin_lock+0x20/0x80 do_raw_spin_lock from rt_del_uncached_list+0x30/0x64 rt_del_uncached_list from xfrm4_dst_destroy+0x3c/0xbc xfrm4_dst_destroy from dst_destroy+0x5c/0xb0 dst_destroy from rcu_process_callbacks+0xc4/0xec rcu_process_callbacks from __do_softirq+0xb4/0x22c __do_softirq from call_with_stack+0x1c/0x24 call_with_stack from do_softirq+0x60/0x6c do_softirq from __local_bh_enable_ip+0xa0/0xcc Patch "net: dst: Prevent false sharing vs. dst_entry:: __refcnt" moved rt_uncached and rt_uncached_list fields from rtable struct to dst struct, so they are more zeroed by memset_after(xdst, 0, u.dst) in xfrm_alloc_dst(). Note that rt_uncached (list_head) was never properly initialized at alloc time, but xfrm[46]_dst_destroy() is written in such a way that it was not an issue thanks to the memset: if (xdst->u.rt.dst.rt_uncached_list) rt_del_uncached_list(&xdst->u.rt); The route code does it the other way around: rt_uncached_list is assumed to be valid IIF rt_uncached list_head is not empty: void rt_del_uncached_list(struct rtable *rt) { if (!list_empty(&rt->dst.rt_uncached)) { struct uncached_list *ul = rt->dst.rt_uncached_list; spin_lock_bh(&ul->lock); list_del_init(&rt->dst.rt_uncached); spin_unlock_bh(&ul->lock); } } This patch adds mandatory rt_uncached list_head initialization in generic dst_init(), and adapt xfrm[46]_dst_destroy logic to match the rest of the code. Fixes: d288a16 ("net: dst: Prevent false sharing vs. dst_entry:: __refcnt") Reported-by: kernel test robot <[email protected]> Link: https://lore.kernel.org/oe-lkp/[email protected] Reviewed-by: David Ahern <[email protected]> Reviewed-by: Eric Dumazet <[email protected]> CC: Leon Romanovsky <[email protected]> Signed-off-by: Maxime Bizon <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 33c1af8 commit 418a730

File tree

5 files changed

+3
-11
lines changed

5 files changed

+3
-11
lines changed

net/core/dst.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ void dst_init(struct dst_entry *dst, struct dst_ops *ops,
6767
#endif
6868
dst->lwtstate = NULL;
6969
rcuref_init(&dst->__rcuref, initial_ref);
70+
INIT_LIST_HEAD(&dst->rt_uncached);
7071
dst->__use = 0;
7172
dst->lastuse = jiffies;
7273
dst->flags = flags;

net/ipv4/route.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1644,7 +1644,6 @@ struct rtable *rt_dst_alloc(struct net_device *dev,
16441644
rt->rt_uses_gateway = 0;
16451645
rt->rt_gw_family = 0;
16461646
rt->rt_gw4 = 0;
1647-
INIT_LIST_HEAD(&rt->dst.rt_uncached);
16481647

16491648
rt->dst.output = ip_output;
16501649
if (flags & RTCF_LOCAL)
@@ -1675,7 +1674,6 @@ struct rtable *rt_dst_clone(struct net_device *dev, struct rtable *rt)
16751674
new_rt->rt_gw4 = rt->rt_gw4;
16761675
else if (rt->rt_gw_family == AF_INET6)
16771676
new_rt->rt_gw6 = rt->rt_gw6;
1678-
INIT_LIST_HEAD(&new_rt->dst.rt_uncached);
16791677

16801678
new_rt->dst.input = rt->dst.input;
16811679
new_rt->dst.output = rt->dst.output;
@@ -2858,8 +2856,6 @@ struct dst_entry *ipv4_blackhole_route(struct net *net, struct dst_entry *dst_or
28582856
rt->rt_gw4 = ort->rt_gw4;
28592857
else if (rt->rt_gw_family == AF_INET6)
28602858
rt->rt_gw6 = ort->rt_gw6;
2861-
2862-
INIT_LIST_HEAD(&rt->dst.rt_uncached);
28632859
}
28642860

28652861
dst_release(dst_orig);

net/ipv4/xfrm4_policy.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
9191
xdst->u.rt.rt_gw6 = rt->rt_gw6;
9292
xdst->u.rt.rt_pmtu = rt->rt_pmtu;
9393
xdst->u.rt.rt_mtu_locked = rt->rt_mtu_locked;
94-
INIT_LIST_HEAD(&xdst->u.rt.dst.rt_uncached);
9594
rt_add_uncached_list(&xdst->u.rt);
9695

9796
return 0;
@@ -121,8 +120,7 @@ static void xfrm4_dst_destroy(struct dst_entry *dst)
121120
struct xfrm_dst *xdst = (struct xfrm_dst *)dst;
122121

123122
dst_destroy_metrics_generic(dst);
124-
if (xdst->u.rt.dst.rt_uncached_list)
125-
rt_del_uncached_list(&xdst->u.rt);
123+
rt_del_uncached_list(&xdst->u.rt);
126124
xfrm_dst_destroy(xdst);
127125
}
128126

net/ipv6/route.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,7 +334,6 @@ static const struct rt6_info ip6_blk_hole_entry_template = {
334334
static void rt6_info_init(struct rt6_info *rt)
335335
{
336336
memset_after(rt, 0, dst);
337-
INIT_LIST_HEAD(&rt->dst.rt_uncached);
338337
}
339338

340339
/* allocate dst with ip6_dst_ops */

net/ipv6/xfrm6_policy.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,6 @@ static int xfrm6_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
8989
xdst->u.rt6.rt6i_gateway = rt->rt6i_gateway;
9090
xdst->u.rt6.rt6i_dst = rt->rt6i_dst;
9191
xdst->u.rt6.rt6i_src = rt->rt6i_src;
92-
INIT_LIST_HEAD(&xdst->u.rt6.dst.rt_uncached);
9392
rt6_uncached_list_add(&xdst->u.rt6);
9493

9594
return 0;
@@ -121,8 +120,7 @@ static void xfrm6_dst_destroy(struct dst_entry *dst)
121120
if (likely(xdst->u.rt6.rt6i_idev))
122121
in6_dev_put(xdst->u.rt6.rt6i_idev);
123122
dst_destroy_metrics_generic(dst);
124-
if (xdst->u.rt6.dst.rt_uncached_list)
125-
rt6_uncached_list_del(&xdst->u.rt6);
123+
rt6_uncached_list_del(&xdst->u.rt6);
126124
xfrm_dst_destroy(xdst);
127125
}
128126

0 commit comments

Comments
 (0)