Skip to content

Commit

Permalink
net: xfrm: Localize sequence counter per network namespace
Browse files Browse the repository at this point in the history
[ Upstream commit e88add1 ]

A sequence counter write section must be serialized or its internal
state can get corrupted. The "xfrm_state_hash_generation" seqcount is
global, but its write serialization lock (net->xfrm.xfrm_state_lock) is
instantiated per network namespace. The write protection is thus
insufficient.

To provide full protection, localize the sequence counter per network
namespace instead. This should be safe as both the seqcount read and
write sections access data exclusively within the network namespace. It
also lays the foundation for transforming "xfrm_state_hash_generation"
data type from seqcount_t to seqcount_LOCKNAME_t in further commits.

Fixes: b65e3d7 ("xfrm: state: add sequence count to detect hash resizes")
Signed-off-by: Ahmed S. Darwish <a.darwish@linutronix.de>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
  • Loading branch information
a-darwish authored and gregkh committed Apr 14, 2021
1 parent 8ac165d commit fbc15e4
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 6 deletions.
4 changes: 3 additions & 1 deletion include/net/netns/xfrm.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ struct netns_xfrm {
#if IS_ENABLED(CONFIG_IPV6)
struct dst_ops xfrm6_dst_ops;
#endif
spinlock_t xfrm_state_lock;
spinlock_t xfrm_state_lock;
seqcount_t xfrm_state_hash_generation;

spinlock_t xfrm_policy_lock;
struct mutex xfrm_cfg_mutex;
};
Expand Down
10 changes: 5 additions & 5 deletions net/xfrm/xfrm_state.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ static void xfrm_state_gc_task(struct work_struct *work);
*/

static unsigned int xfrm_state_hashmax __read_mostly = 1 * 1024 * 1024;
static __read_mostly seqcount_t xfrm_state_hash_generation = SEQCNT_ZERO(xfrm_state_hash_generation);
static struct kmem_cache *xfrm_state_cache __ro_after_init;

static DECLARE_WORK(xfrm_state_gc_work, xfrm_state_gc_task);
Expand Down Expand Up @@ -140,7 +139,7 @@ static void xfrm_hash_resize(struct work_struct *work)
}

spin_lock_bh(&net->xfrm.xfrm_state_lock);
write_seqcount_begin(&xfrm_state_hash_generation);
write_seqcount_begin(&net->xfrm.xfrm_state_hash_generation);

nhashmask = (nsize / sizeof(struct hlist_head)) - 1U;
odst = xfrm_state_deref_prot(net->xfrm.state_bydst, net);
Expand All @@ -156,7 +155,7 @@ static void xfrm_hash_resize(struct work_struct *work)
rcu_assign_pointer(net->xfrm.state_byspi, nspi);
net->xfrm.state_hmask = nhashmask;

write_seqcount_end(&xfrm_state_hash_generation);
write_seqcount_end(&net->xfrm.xfrm_state_hash_generation);
spin_unlock_bh(&net->xfrm.xfrm_state_lock);

osize = (ohashmask + 1) * sizeof(struct hlist_head);
Expand Down Expand Up @@ -1063,7 +1062,7 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,

to_put = NULL;

sequence = read_seqcount_begin(&xfrm_state_hash_generation);
sequence = read_seqcount_begin(&net->xfrm.xfrm_state_hash_generation);

rcu_read_lock();
h = xfrm_dst_hash(net, daddr, saddr, tmpl->reqid, encap_family);
Expand Down Expand Up @@ -1176,7 +1175,7 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
if (to_put)
xfrm_state_put(to_put);

if (read_seqcount_retry(&xfrm_state_hash_generation, sequence)) {
if (read_seqcount_retry(&net->xfrm.xfrm_state_hash_generation, sequence)) {
*err = -EAGAIN;
if (x) {
xfrm_state_put(x);
Expand Down Expand Up @@ -2666,6 +2665,7 @@ int __net_init xfrm_state_init(struct net *net)
net->xfrm.state_num = 0;
INIT_WORK(&net->xfrm.state_hash_work, xfrm_hash_resize);
spin_lock_init(&net->xfrm.xfrm_state_lock);
seqcount_init(&net->xfrm.xfrm_state_hash_generation);
return 0;

out_byspi:
Expand Down

0 comments on commit fbc15e4

Please sign in to comment.