In commit 6a53b7593233 ("xfrm: check id proto in validate_tmpl()") I introduced a check for xfrm protocol, but unfortunately xfrm_id_proto_match() could still miss IPPROTO_ROUTING which causes entries left in net->xfrm.state_all.
This patch extracts the check from validate_tmpl() to xfrm_id_proto_valid() and uses it in all other places, including the missing one in parse_ipsecrequest(). Fixes: 6a53b7593233 ("xfrm: check id proto in validate_tmpl()") Reported-by: syzbot+0bf0519d6e0de1591...@syzkaller.appspotmail.com Cc: Steffen Klassert <steffen.klass...@secunet.com> Cc: Herbert Xu <herb...@gondor.apana.org.au> Signed-off-by: Cong Wang <xiyou.wangc...@gmail.com> --- include/net/xfrm.h | 21 ++++++++++++++++++--- net/key/af_key.c | 4 +++- net/xfrm/xfrm_user.c | 14 +------------- 3 files changed, 22 insertions(+), 17 deletions(-) diff --git a/include/net/xfrm.h b/include/net/xfrm.h index 85386becbaea..22ef7617087d 100644 --- a/include/net/xfrm.h +++ b/include/net/xfrm.h @@ -1404,12 +1404,27 @@ 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 + case IPSEC_PROTO_ANY: + return true; + default: + return false; + } +} + static inline int xfrm_id_proto_match(u8 proto, u8 userproto) { return (!userproto || proto == userproto || - (userproto == IPSEC_PROTO_ANY && (proto == IPPROTO_AH || - proto == IPPROTO_ESP || - proto == IPPROTO_COMP))); + (userproto == IPSEC_PROTO_ANY && xfrm_id_proto_valid(proto))); } /* diff --git a/net/key/af_key.c b/net/key/af_key.c index 5651c29cb5bd..4af1e1d60b9f 100644 --- a/net/key/af_key.c +++ b/net/key/af_key.c @@ -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; diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c index a131f9ff979e..067ac9ed2918 100644 --- a/net/xfrm/xfrm_user.c +++ b/net/xfrm/xfrm_user.c @@ -1513,20 +1513,8 @@ 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; -- 2.20.1