diff options
| author | Steffen Klassert <steffen.klassert@secunet.com> | 2025-06-10 09:51:54 +0200 |
|---|---|---|
| committer | Steffen Klassert <steffen.klassert@secunet.com> | 2025-06-10 09:51:54 +0200 |
| commit | 766f6a784bdfd0747c66e40102e53c48bb89e135 (patch) | |
| tree | 1b2bbdde98c7f1f4ca6f253be5edf1dace39c150 | |
| parent | b56bbaf8c9ffe02468f6ba8757668e95dda7e62c (diff) | |
| parent | 7eb11c0ab70777b9e5145a5ba1c0a2312c3980b2 (diff) | |
| download | ipsec-766f6a784bdfd0747c66e40102e53c48bb89e135.tar.gz | |
Merge branch 'xfrm: fixes for xfrm_state_find under preemption'
Sabrina Dubroca says:
====================
While looking at the pcpu_id changes, I found two issues that can
happen if we get preempted and the cpu_id changes. The second patch
takes care of both problems. The first patch also makes sure we don't
use state_ptrs uninitialized, which could currently happen. syzbot
seems to have hit this issue [1].
[1] https://syzkaller.appspot.com/bug?extid=7ed9d47e15e88581dc5b
====================
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
| -rw-r--r-- | net/xfrm/xfrm_state.c | 23 |
1 files changed, 8 insertions, 15 deletions
diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c index 203b585c2ae28f..7e34fc94f668b2 100644 --- a/net/xfrm/xfrm_state.c +++ b/net/xfrm/xfrm_state.c @@ -1307,14 +1307,8 @@ static void xfrm_hash_grow_check(struct net *net, int have_hash_collision) static void xfrm_state_look_at(struct xfrm_policy *pol, struct xfrm_state *x, const struct flowi *fl, unsigned short family, struct xfrm_state **best, int *acq_in_progress, - int *error) + int *error, unsigned int pcpu_id) { - /* We need the cpu id just as a lookup key, - * we don't require it to be stable. - */ - unsigned int pcpu_id = get_cpu(); - put_cpu(); - /* Resolution logic: * 1. There is a valid state with matching selector. Done. * 2. Valid state with inappropriate selector. Skip. @@ -1381,14 +1375,15 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr, /* We need the cpu id just as a lookup key, * we don't require it to be stable. */ - pcpu_id = get_cpu(); - put_cpu(); + pcpu_id = raw_smp_processor_id(); to_put = NULL; sequence = read_seqcount_begin(&net->xfrm.xfrm_state_hash_generation); rcu_read_lock(); + xfrm_hash_ptrs_get(net, &state_ptrs); + hlist_for_each_entry_rcu(x, &pol->state_cache_list, state_cache) { if (x->props.family == encap_family && x->props.reqid == tmpl->reqid && @@ -1400,7 +1395,7 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr, tmpl->id.proto == x->id.proto && (tmpl->id.spi == x->id.spi || !tmpl->id.spi)) xfrm_state_look_at(pol, x, fl, encap_family, - &best, &acquire_in_progress, &error); + &best, &acquire_in_progress, &error, pcpu_id); } if (best) @@ -1417,7 +1412,7 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr, tmpl->id.proto == x->id.proto && (tmpl->id.spi == x->id.spi || !tmpl->id.spi)) xfrm_state_look_at(pol, x, fl, family, - &best, &acquire_in_progress, &error); + &best, &acquire_in_progress, &error, pcpu_id); } cached: @@ -1429,8 +1424,6 @@ cached: else if (acquire_in_progress) /* XXX: acquire_in_progress should not happen */ WARN_ON(1); - xfrm_hash_ptrs_get(net, &state_ptrs); - h = __xfrm_dst_hash(daddr, saddr, tmpl->reqid, encap_family, state_ptrs.hmask); hlist_for_each_entry_rcu(x, state_ptrs.bydst + h, bydst) { #ifdef CONFIG_XFRM_OFFLOAD @@ -1460,7 +1453,7 @@ cached: tmpl->id.proto == x->id.proto && (tmpl->id.spi == x->id.spi || !tmpl->id.spi)) xfrm_state_look_at(pol, x, fl, family, - &best, &acquire_in_progress, &error); + &best, &acquire_in_progress, &error, pcpu_id); } if (best || acquire_in_progress) goto found; @@ -1495,7 +1488,7 @@ cached: tmpl->id.proto == x->id.proto && (tmpl->id.spi == x->id.spi || !tmpl->id.spi)) xfrm_state_look_at(pol, x, fl, family, - &best, &acquire_in_progress, &error); + &best, &acquire_in_progress, &error, pcpu_id); } found: |