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

Reply via email to