stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Zubin Mithra <zsm@chromium.org>
To: stable@vger.kernel.org
Cc: gregkh@linuxfoundation.org, groeck@chromium.org,
	xiyou.wangcong@gmail.com, steffen.klassert@secunet.com,
	herbert@gondor.apana.org.au
Subject: [v4.14.y, v4.9.y] xfrm: clean up xfrm protocol checks
Date: Mon,  9 Sep 2019 12:54:57 -0700	[thread overview]
Message-ID: <20190909195457.56783-1-zsm@chromium.org> (raw)

From: Cong Wang <xiyou.wangcong@gmail.com>

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>
---
Notes:
* Syzkaller triggered a WARNING with the following stacktrace:
 __dump_stack lib/dump_stack.c:17 [inline]
 dump_stack+0x114/0x1cf lib/dump_stack.c:53
 panic+0x1bb/0x3a0 kernel/panic.c:181
 __warn.cold.9+0x149/0x186 kernel/panic.c:542
 report_bug+0x1f7/0x272 lib/bug.c:186
 fixup_bug arch/x86/kernel/traps.c:177 [inline]
 do_error_trap+0x1c1/0x430 arch/x86/kernel/traps.c:295
 do_invalid_op+0x20/0x30 arch/x86/kernel/traps.c:314
 invalid_op+0x1b/0x40 arch/x86/entry/entry_64.S:944
 xfrm_net_exit+0x2a/0x30 net/xfrm/xfrm_policy.c:2971
 ops_exit_list.isra.11+0xb5/0x160 net/core/net_namespace.c:142
 cleanup_net+0x575/0xab0 net/core/net_namespace.c:483
 process_one_work+0xb5c/0x1950 kernel/workqueue.c:2114
 worker_thread+0x224/0x18c0 kernel/workqueue.c:2248
 kthread+0x359/0x430 kernel/kthread.c:231
 ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:402

* The commit is present in linux-4.19.y. A backport for linux-4.4.y will
be sent separately.

* The patch resolves the following conflicts:
- conflicts in content of xfrm6_tunnel_net_exit()
- changes in the prototype of xfrm_state_flush()
- ordering of xfrm_state_flush(...) and flush_work(&xfrm_state_gc_work)
  inside xfrm_state_fini.

* Tests run: Chrome OS tryjobs, syzkaller reproducer

 include/net/xfrm.h    | 17 +++++++++++++++++
 net/key/af_key.c      |  4 +++-
 net/xfrm/xfrm_state.c |  2 +-
 net/xfrm/xfrm_user.c  | 14 +-------------
 4 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index bdf185ae93db..57b8b11cf7d4 100644
--- 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 ac38b47e9f86..f8f7065f7b62 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_state.c b/net/xfrm/xfrm_state.c
index 7c093de68780..bd16e6882017 100644
--- 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 150c58dc8a7b..339a070da597 100644
--- 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;
-- 
2.23.0.162.g0b9fbb3734-goog


             reply	other threads:[~2019-09-09 19:55 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-09 19:54 Zubin Mithra [this message]
2019-09-10 14:25 ` [v4.14.y, v4.9.y] xfrm: clean up xfrm protocol checks Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190909195457.56783-1-zsm@chromium.org \
    --to=zsm@chromium.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=groeck@chromium.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=stable@vger.kernel.org \
    --cc=steffen.klassert@secunet.com \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).