Commit 418a7307 authored by Maxime Bizon's avatar Maxime Bizon Committed by Jakub Kicinski

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: d288a162 ("net: dst: Prevent false sharing vs. dst_entry:: __refcnt")
Reported-by: default avatarkernel test robot <oliver.sang@intel.com>
Link: https://lore.kernel.org/oe-lkp/202304162125.18b7bcdd-oliver.sang@intel.comReviewed-by: default avatarDavid Ahern <dsahern@kernel.org>
Reviewed-by: default avatarEric Dumazet <edumazet@google.com>
CC: Leon Romanovsky <leon@kernel.org>
Signed-off-by: default avatarMaxime Bizon <mbizon@freebox.fr>
Link: https://lore.kernel.org/r/20230420182508.2417582-1-mbizon@freebox.frSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 33c1af8e
...@@ -67,6 +67,7 @@ void dst_init(struct dst_entry *dst, struct dst_ops *ops, ...@@ -67,6 +67,7 @@ void dst_init(struct dst_entry *dst, struct dst_ops *ops,
#endif #endif
dst->lwtstate = NULL; dst->lwtstate = NULL;
rcuref_init(&dst->__rcuref, initial_ref); rcuref_init(&dst->__rcuref, initial_ref);
INIT_LIST_HEAD(&dst->rt_uncached);
dst->__use = 0; dst->__use = 0;
dst->lastuse = jiffies; dst->lastuse = jiffies;
dst->flags = flags; dst->flags = flags;
......
...@@ -1644,7 +1644,6 @@ struct rtable *rt_dst_alloc(struct net_device *dev, ...@@ -1644,7 +1644,6 @@ struct rtable *rt_dst_alloc(struct net_device *dev,
rt->rt_uses_gateway = 0; rt->rt_uses_gateway = 0;
rt->rt_gw_family = 0; rt->rt_gw_family = 0;
rt->rt_gw4 = 0; rt->rt_gw4 = 0;
INIT_LIST_HEAD(&rt->dst.rt_uncached);
rt->dst.output = ip_output; rt->dst.output = ip_output;
if (flags & RTCF_LOCAL) if (flags & RTCF_LOCAL)
...@@ -1675,7 +1674,6 @@ struct rtable *rt_dst_clone(struct net_device *dev, struct rtable *rt) ...@@ -1675,7 +1674,6 @@ struct rtable *rt_dst_clone(struct net_device *dev, struct rtable *rt)
new_rt->rt_gw4 = rt->rt_gw4; new_rt->rt_gw4 = rt->rt_gw4;
else if (rt->rt_gw_family == AF_INET6) else if (rt->rt_gw_family == AF_INET6)
new_rt->rt_gw6 = rt->rt_gw6; new_rt->rt_gw6 = rt->rt_gw6;
INIT_LIST_HEAD(&new_rt->dst.rt_uncached);
new_rt->dst.input = rt->dst.input; new_rt->dst.input = rt->dst.input;
new_rt->dst.output = rt->dst.output; 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 ...@@ -2858,8 +2856,6 @@ struct dst_entry *ipv4_blackhole_route(struct net *net, struct dst_entry *dst_or
rt->rt_gw4 = ort->rt_gw4; rt->rt_gw4 = ort->rt_gw4;
else if (rt->rt_gw_family == AF_INET6) else if (rt->rt_gw_family == AF_INET6)
rt->rt_gw6 = ort->rt_gw6; rt->rt_gw6 = ort->rt_gw6;
INIT_LIST_HEAD(&rt->dst.rt_uncached);
} }
dst_release(dst_orig); dst_release(dst_orig);
......
...@@ -91,7 +91,6 @@ static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev, ...@@ -91,7 +91,6 @@ static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
xdst->u.rt.rt_gw6 = rt->rt_gw6; xdst->u.rt.rt_gw6 = rt->rt_gw6;
xdst->u.rt.rt_pmtu = rt->rt_pmtu; xdst->u.rt.rt_pmtu = rt->rt_pmtu;
xdst->u.rt.rt_mtu_locked = rt->rt_mtu_locked; xdst->u.rt.rt_mtu_locked = rt->rt_mtu_locked;
INIT_LIST_HEAD(&xdst->u.rt.dst.rt_uncached);
rt_add_uncached_list(&xdst->u.rt); rt_add_uncached_list(&xdst->u.rt);
return 0; return 0;
...@@ -121,8 +120,7 @@ static void xfrm4_dst_destroy(struct dst_entry *dst) ...@@ -121,8 +120,7 @@ static void xfrm4_dst_destroy(struct dst_entry *dst)
struct xfrm_dst *xdst = (struct xfrm_dst *)dst; struct xfrm_dst *xdst = (struct xfrm_dst *)dst;
dst_destroy_metrics_generic(dst); dst_destroy_metrics_generic(dst);
if (xdst->u.rt.dst.rt_uncached_list) rt_del_uncached_list(&xdst->u.rt);
rt_del_uncached_list(&xdst->u.rt);
xfrm_dst_destroy(xdst); xfrm_dst_destroy(xdst);
} }
......
...@@ -334,7 +334,6 @@ static const struct rt6_info ip6_blk_hole_entry_template = { ...@@ -334,7 +334,6 @@ static const struct rt6_info ip6_blk_hole_entry_template = {
static void rt6_info_init(struct rt6_info *rt) static void rt6_info_init(struct rt6_info *rt)
{ {
memset_after(rt, 0, dst); memset_after(rt, 0, dst);
INIT_LIST_HEAD(&rt->dst.rt_uncached);
} }
/* allocate dst with ip6_dst_ops */ /* allocate dst with ip6_dst_ops */
......
...@@ -89,7 +89,6 @@ static int xfrm6_fill_dst(struct xfrm_dst *xdst, struct net_device *dev, ...@@ -89,7 +89,6 @@ static int xfrm6_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
xdst->u.rt6.rt6i_gateway = rt->rt6i_gateway; xdst->u.rt6.rt6i_gateway = rt->rt6i_gateway;
xdst->u.rt6.rt6i_dst = rt->rt6i_dst; xdst->u.rt6.rt6i_dst = rt->rt6i_dst;
xdst->u.rt6.rt6i_src = rt->rt6i_src; xdst->u.rt6.rt6i_src = rt->rt6i_src;
INIT_LIST_HEAD(&xdst->u.rt6.dst.rt_uncached);
rt6_uncached_list_add(&xdst->u.rt6); rt6_uncached_list_add(&xdst->u.rt6);
return 0; return 0;
...@@ -121,8 +120,7 @@ static void xfrm6_dst_destroy(struct dst_entry *dst) ...@@ -121,8 +120,7 @@ static void xfrm6_dst_destroy(struct dst_entry *dst)
if (likely(xdst->u.rt6.rt6i_idev)) if (likely(xdst->u.rt6.rt6i_idev))
in6_dev_put(xdst->u.rt6.rt6i_idev); in6_dev_put(xdst->u.rt6.rt6i_idev);
dst_destroy_metrics_generic(dst); dst_destroy_metrics_generic(dst);
if (xdst->u.rt6.dst.rt_uncached_list) rt6_uncached_list_del(&xdst->u.rt6);
rt6_uncached_list_del(&xdst->u.rt6);
xfrm_dst_destroy(xdst); xfrm_dst_destroy(xdst);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment