Commit 766f6a78 authored by Steffen Klassert's avatar Steffen Klassert
Browse files

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: default avatarSteffen Klassert <steffen.klassert@secunet.com>
parents b56bbaf8 7eb11c0a

net/xfrm/xfrm_state.c

+8 −15
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,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 @@ xfrm_state_find(const xfrm_address_t *daddr, const xfrm_address_t *saddr, 
	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 @@ 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: