Skip to content

Commit 7eb11c0

Browse files
qsnklassert
authored andcommitted
xfrm: state: use a consistent pcpu_id in xfrm_state_find
If we get preempted during xfrm_state_find, we could run xfrm_state_look_at using a different pcpu_id than the one xfrm_state_find saw. This could lead to ignoring states that should have matched, and triggering acquires on a CPU that already has a pcpu state. xfrm_state_find starts on CPU1 pcpu_id = 1 lookup starts <preemption, we're now on CPU2> xfrm_state_look_at pcpu_id = 2 finds a state found: best->pcpu_num != pcpu_id (2 != 1) if (!x && !error && !acquire_in_progress) { ... xfrm_state_alloc xfrm_init_tempstate ... This can be avoided by passing the original pcpu_id down to all xfrm_state_look_at() calls. Also switch to raw_smp_processor_id, disabling preempting just to re-enable it immediately doesn't really make sense. Fixes: 1ddf991 ("xfrm: Add support for per cpu xfrm state handling.") Signed-off-by: Sabrina Dubroca <[email protected]> Reviewed-by: Florian Westphal <[email protected]> Signed-off-by: Steffen Klassert <[email protected]>
1 parent 94d077c commit 7eb11c0

File tree

1 file changed

+6
-13
lines changed

1 file changed

+6
-13
lines changed

net/xfrm/xfrm_state.c

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1307,14 +1307,8 @@ static void xfrm_hash_grow_check(struct net *net, int have_hash_collision)
13071307
static void xfrm_state_look_at(struct xfrm_policy *pol, struct xfrm_state *x,
13081308
const struct flowi *fl, unsigned short family,
13091309
struct xfrm_state **best, int *acq_in_progress,
1310-
int *error)
1310+
int *error, unsigned int pcpu_id)
13111311
{
1312-
/* We need the cpu id just as a lookup key,
1313-
* we don't require it to be stable.
1314-
*/
1315-
unsigned int pcpu_id = get_cpu();
1316-
put_cpu();
1317-
13181312
/* Resolution logic:
13191313
* 1. There is a valid state with matching selector. Done.
13201314
* 2. Valid state with inappropriate selector. Skip.
@@ -1381,8 +1375,7 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
13811375
/* We need the cpu id just as a lookup key,
13821376
* we don't require it to be stable.
13831377
*/
1384-
pcpu_id = get_cpu();
1385-
put_cpu();
1378+
pcpu_id = raw_smp_processor_id();
13861379

13871380
to_put = NULL;
13881381

@@ -1402,7 +1395,7 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
14021395
tmpl->id.proto == x->id.proto &&
14031396
(tmpl->id.spi == x->id.spi || !tmpl->id.spi))
14041397
xfrm_state_look_at(pol, x, fl, encap_family,
1405-
&best, &acquire_in_progress, &error);
1398+
&best, &acquire_in_progress, &error, pcpu_id);
14061399
}
14071400

14081401
if (best)
@@ -1419,7 +1412,7 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
14191412
tmpl->id.proto == x->id.proto &&
14201413
(tmpl->id.spi == x->id.spi || !tmpl->id.spi))
14211414
xfrm_state_look_at(pol, x, fl, family,
1422-
&best, &acquire_in_progress, &error);
1415+
&best, &acquire_in_progress, &error, pcpu_id);
14231416
}
14241417

14251418
cached:
@@ -1460,7 +1453,7 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
14601453
tmpl->id.proto == x->id.proto &&
14611454
(tmpl->id.spi == x->id.spi || !tmpl->id.spi))
14621455
xfrm_state_look_at(pol, x, fl, family,
1463-
&best, &acquire_in_progress, &error);
1456+
&best, &acquire_in_progress, &error, pcpu_id);
14641457
}
14651458
if (best || acquire_in_progress)
14661459
goto found;
@@ -1495,7 +1488,7 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr,
14951488
tmpl->id.proto == x->id.proto &&
14961489
(tmpl->id.spi == x->id.spi || !tmpl->id.spi))
14971490
xfrm_state_look_at(pol, x, fl, family,
1498-
&best, &acquire_in_progress, &error);
1491+
&best, &acquire_in_progress, &error, pcpu_id);
14991492
}
15001493

15011494
found:

0 commit comments

Comments
 (0)