Commit 7eb11c0a authored by Sabrina Dubroca's avatar Sabrina Dubroca Committed by Steffen Klassert
Browse files

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: 1ddf9916 ("xfrm: Add support for per cpu xfrm state handling.")
Signed-off-by: default avatarSabrina Dubroca <sd@queasysnail.net>
Reviewed-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarSteffen Klassert <steffen.klassert@secunet.com>
parent 94d077c3

net/xfrm/xfrm_state.c

+6 −13
Original line number Diff line number Diff line
@@ -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,8 +1375,7 @@ 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;
@@ -1402,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)
@@ -1419,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:
@@ -1460,7 +1453,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);

	}

	if (best || acquire_in_progress)

		goto found;
@@ -1495,7 +1488,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);

	}



found: