Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit cd393b38 authored by Cong Wang's avatar Cong Wang Committed by Greg Kroah-Hartman
Browse files

xfrm: clean up xfrm protocol checks 



commit dbb2483b2a46fbaf833cfb5deb5ed9cace9c7399 upstream.

In commit 6a53b759 ("xfrm: check id proto in validate_tmpl()")
I introduced a check for xfrm protocol, but according to Herbert
IPSEC_PROTO_ANY should only be used as a wildcard for lookup, so
it should be removed from validate_tmpl().

And, IPSEC_PROTO_ANY is expected to only match 3 IPSec-specific
protocols, this is why xfrm_state_flush() could still miss
IPPROTO_ROUTING, which leads that those entries are left in
net->xfrm.state_all before exit net. Fix this by replacing
IPSEC_PROTO_ANY with zero.

This patch also extracts the check from validate_tmpl() to
xfrm_id_proto_valid() and uses it in parse_ipsecrequest().
With this, no other protocols should be added into xfrm.

Fixes: 6a53b759 ("xfrm: check id proto in validate_tmpl()")
Reported-by: default avatar <syzbot+0bf0519d6e0de15914fe@syzkaller.appspotmail.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: default avatarCong Wang <xiyou.wangcong@gmail.com>
Acked-by: default avatarHerbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: default avatarSteffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: default avatarZubin Mithra <zsm@chromium.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 32b803e8

include/net/xfrm.h

+17 −0
Original line number Diff line number Diff line
@@ -1366,6 +1366,23 @@ static inline int xfrm_state_kern(const struct xfrm_state *x) 
	return atomic_read(&x->tunnel_users);

}



static inline bool xfrm_id_proto_valid(u8 proto)

{

	switch (proto) {

	case IPPROTO_AH:

	case IPPROTO_ESP:

	case IPPROTO_COMP:

#if IS_ENABLED(CONFIG_IPV6)

	case IPPROTO_ROUTING:

	case IPPROTO_DSTOPTS:

#endif

		return true;

	default:

		return false;

	}

}



/* IPSEC_PROTO_ANY only matches 3 IPsec protocols, 0 could match all. */

static inline int xfrm_id_proto_match(u8 proto, u8 userproto)

{

	return (!userproto || proto == userproto ||

net/key/af_key.c

+3 −1
Original line number Diff line number Diff line
@@ -1951,8 +1951,10 @@ parse_ipsecrequest(struct xfrm_policy *xp, struct sadb_x_ipsecrequest *rq) 


	if (rq->sadb_x_ipsecrequest_mode == 0)

		return -EINVAL;

	if (!xfrm_id_proto_valid(rq->sadb_x_ipsecrequest_proto))

		return -EINVAL;



	t->id.proto = rq->sadb_x_ipsecrequest_proto; /* XXX check proto */

	t->id.proto = rq->sadb_x_ipsecrequest_proto;

	if ((mode = pfkey_mode_to_xfrm(rq->sadb_x_ipsecrequest_mode)) < 0)

		return -EINVAL;

	t->mode = mode;

net/xfrm/xfrm_state.c

+1 −1
Original line number Diff line number Diff line
@@ -2330,7 +2330,7 @@ void xfrm_state_fini(struct net *net) 
	unsigned int sz;



	flush_work(&net->xfrm.state_hash_work);

	xfrm_state_flush(net, IPSEC_PROTO_ANY, false);

	xfrm_state_flush(net, 0, false);

	flush_work(&xfrm_state_gc_work);



	WARN_ON(!list_empty(&net->xfrm.state_all));

net/xfrm/xfrm_user.c

+1 −13
Original line number Diff line number Diff line
@@ -1489,22 +1489,10 @@ static int validate_tmpl(int nr, struct xfrm_user_tmpl *ut, u16 family) 
			return -EINVAL;

		}



		switch (ut[i].id.proto) {

		case IPPROTO_AH:

		case IPPROTO_ESP:

		case IPPROTO_COMP:

#if IS_ENABLED(CONFIG_IPV6)

		case IPPROTO_ROUTING:

		case IPPROTO_DSTOPTS:

#endif

		case IPSEC_PROTO_ANY:

			break;

		default:

		if (!xfrm_id_proto_valid(ut[i].id.proto))

			return -EINVAL;

	}



	}



	return 0;

}