xfrm: clean up xfrm protocol checks

commit dbb2483b2a46fbaf833cfb5deb5ed9cace9c7399 upstream.

In commit 6a53b7593233 ("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: 6a53b7593233 ("xfrm: check id proto in validate_tmpl()")
Reported-by: syzbot+0bf0519d6e0de15914fe@syzkaller.appspotmail.com
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com>
Signed-off-by: Zubin Mithra <zsm@chromium.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index bdf185ae93dbe6a0dd780433a486960e6adb9133..57b8b11cf7d422baad720f87b67137f3854004ef 100644 (file)
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -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 ||

diff --git a/net/key/af_key.c b/net/key/af_key.c
index ac38b47e9f86b50db49d9ffdea5fbcab30c1cce2..f8f7065f7b6270dc368af2e759490809ff1b0972 100644 (file)
--- 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_state.c b/net/xfrm/xfrm_state.c
index 7c093de6878097f385050d79af98ab06a972486a..bd16e6882017e23e731aa1d7829ba72e91ddde28 100644 (file)
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -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));

diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 150c58dc8a7b3fcff3cc4fc7b0fcd5faa5a0e700..339a070da597879e33dc041f9b71e9fcd16336b6 100644 (file)
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1489,20 +1489,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;