Skip to content

Commit

Permalink
xfrm: Fix RCU vs hash_resize_mutex lock inversion
Browse files Browse the repository at this point in the history 
xfrm_bydst_resize() calls synchronize_rcu() while holding
hash_resize_mutex. But then on PREEMPT_RT configurations,
xfrm_policy_lookup_bytype() may acquire that mutex while running in an
RCU read side critical section. This results in a deadlock.

In fact the scope of hash_resize_mutex is way beyond the purpose of
xfrm_policy_lookup_bytype() to just fetch a coherent and stable policy
for a given destination/direction, along with other details.

The lower level net->xfrm.xfrm_policy_lock, which among other things
protects per destination/direction references to policy entries, is
enough to serialize and benefit from priority inheritance against the
write side. As a bonus, it makes it officially a per network namespace
synchronization business where a policy table resize on namespace A
shouldn't block a policy lookup on namespace B.

Fixes: 77cc278 (xfrm: policy: Use sequence counters with associated lock)
Cc: [email protected]
Cc: Ahmed S. Darwish <[email protected]>
Cc: Peter Zijlstra (Intel) <[email protected]>
Cc: Varad Gautam <[email protected]>
Cc: Steffen Klassert <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: David S. Miller <[email protected]>
Signed-off-by: Frederic Weisbecker <[email protected]>
Signed-off-by: Steffen Klassert <[email protected]>
  • Loading branch information
@klassert
Frederic Weisbecker authored and klassert committed Jul 2, 2021
1 parent eaf2282 commit 2580d3f
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 9 deletions.
1 change: 1 addition & 0 deletions include/net/netns/xfrm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ struct netns_xfrm {
#endif
spinlock_t xfrm_state_lock;
seqcount_spinlock_t xfrm_state_hash_generation;
seqcount_spinlock_t xfrm_policy_hash_generation;

spinlock_t xfrm_policy_lock;
struct mutex xfrm_cfg_mutex;
Expand Down
17 changes: 8 additions & 9 deletions net/xfrm/xfrm_policy.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,6 @@ static struct xfrm_policy_afinfo const __rcu *xfrm_policy_afinfo[AF_INET6 + 1]
__read_mostly;

static struct kmem_cache *xfrm_dst_cache __ro_after_init;
static __read_mostly seqcount_mutex_t xfrm_policy_hash_generation;

static struct rhashtable xfrm_policy_inexact_table;
static const struct rhashtable_params xfrm_pol_inexact_params;
Expand Down Expand Up @@ -585,7 +584,7 @@ static void xfrm_bydst_resize(struct net *net, int dir)
return;

spin_lock_bh(&net->xfrm.xfrm_policy_lock);
write_seqcount_begin(&xfrm_policy_hash_generation);
write_seqcount_begin(&net->xfrm.xfrm_policy_hash_generation);

odst = rcu_dereference_protected(net->xfrm.policy_bydst[dir].table,
lockdep_is_held(&net->xfrm.xfrm_policy_lock));
Expand All @@ -596,7 +595,7 @@ static void xfrm_bydst_resize(struct net *net, int dir)
rcu_assign_pointer(net->xfrm.policy_bydst[dir].table, ndst);
net->xfrm.policy_bydst[dir].hmask = nhashmask;

write_seqcount_end(&xfrm_policy_hash_generation);
write_seqcount_end(&net->xfrm.xfrm_policy_hash_generation);
spin_unlock_bh(&net->xfrm.xfrm_policy_lock);

synchronize_rcu();
Expand Down Expand Up @@ -1245,7 +1244,7 @@ static void xfrm_hash_rebuild(struct work_struct *work)
} while (read_seqretry(&net->xfrm.policy_hthresh.lock, seq));

spin_lock_bh(&net->xfrm.xfrm_policy_lock);
write_seqcount_begin(&xfrm_policy_hash_generation);
write_seqcount_begin(&net->xfrm.xfrm_policy_hash_generation);

/* make sure that we can insert the indirect policies again before
* we start with destructive action.
Expand Down Expand Up @@ -1354,7 +1353,7 @@ static void xfrm_hash_rebuild(struct work_struct *work)

out_unlock:
__xfrm_policy_inexact_flush(net);
write_seqcount_end(&xfrm_policy_hash_generation);
write_seqcount_end(&net->xfrm.xfrm_policy_hash_generation);
spin_unlock_bh(&net->xfrm.xfrm_policy_lock);

mutex_unlock(&hash_resize_mutex);
Expand Down Expand Up @@ -2095,9 +2094,9 @@ static struct xfrm_policy *xfrm_policy_lookup_bytype(struct net *net, u8 type,
rcu_read_lock();
retry:
do {
sequence = read_seqcount_begin(&xfrm_policy_hash_generation);
sequence = read_seqcount_begin(&net->xfrm.xfrm_policy_hash_generation);
chain = policy_hash_direct(net, daddr, saddr, family, dir);
} while (read_seqcount_retry(&xfrm_policy_hash_generation, sequence));
} while (read_seqcount_retry(&net->xfrm.xfrm_policy_hash_generation, sequence));

ret = NULL;
hlist_for_each_entry_rcu(pol, chain, bydst) {
Expand Down Expand Up @@ -2128,7 +2127,7 @@ static struct xfrm_policy *xfrm_policy_lookup_bytype(struct net *net, u8 type,
}

skip_inexact:
if (read_seqcount_retry(&xfrm_policy_hash_generation, sequence))
if (read_seqcount_retry(&net->xfrm.xfrm_policy_hash_generation, sequence))
goto retry;

if (ret && !xfrm_pol_hold_rcu(ret))
Expand Down Expand Up @@ -4084,6 +4083,7 @@ static int __net_init xfrm_net_init(struct net *net)
/* Initialize the per-net locks here */
spin_lock_init(&net->xfrm.xfrm_state_lock);
spin_lock_init(&net->xfrm.xfrm_policy_lock);
seqcount_spinlock_init(&net->xfrm.xfrm_policy_hash_generation, &net->xfrm.xfrm_policy_lock);
mutex_init(&net->xfrm.xfrm_cfg_mutex);

rv = xfrm_statistics_init(net);
Expand Down Expand Up @@ -4128,7 +4128,6 @@ void __init xfrm_init(void)
{
register_pernet_subsys(&xfrm_net_ops);
xfrm_dev_init();
seqcount_mutex_init(&xfrm_policy_hash_generation, &hash_resize_mutex);
xfrm_input_init();

#ifdef CONFIG_XFRM_ESPINTCP
Expand Down

0 comments on commit 2580d3f

Please sign in to comment.