From: Cong Wang <xiyou.wangcong@xxxxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxxxxxx
Cc: Steffen Klassert <steffen.klassert@xxxxxxxxxxx>
Cc: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Cong Wang <xiyou.wangcong@xxxxxxxxx>
Acked-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Steffen Klassert <steffen.klassert@xxxxxxxxxxx>
Signed-off-by: Zubin Mithra <zsm@xxxxxxxxxxxx>
---
Notes:
* Syzkaller triggered a WARNING with the following stacktrace:
__dump_stack lib/dump_stack.c:15 [inline]
dump_stack+0xbf/0x113 lib/dump_stack.c:51
panic+0x1a6/0x361 kernel/panic.c:116
__warn+0x168/0x1b0 kernel/panic.c:470
warn_slowpath_null+0x3c/0x40 kernel/panic.c:514
xfrm_state_fini+0x83/0x28e net/xfrm/xfrm_state.c:2131
xfrm_net_exit+0x32/0x35 net/xfrm/xfrm_policy.c:3044
ops_exit_list+0xfc/0x12f net/core/net_namespace.c:136
cleanup_net+0x328/0x4ce net/core/net_namespace.c:454
process_one_work+0x7df/0xca8 kernel/workqueue.c:2064
worker_thread+0x592/0x765 kernel/workqueue.c:2196
kthread+0x279/0x28d kernel/kthread.c:211
ret_from_fork+0x4e/0x80 arch/x86/entry/entry_64.S:600
* The commit is present in linux-4.19.y. A backport for 4.14.y and 4.9.y
has been sent separately.
* The patch resolves the following conflicts:
- conflicts in content of xfrm6_tunnel_net_exit()
- changes in the prototype of xfrm_state_flush()
- net->xfrm.state_gc_work has been moved to a global xfrm_state_gc_work
- ordering of xfrm_state_flush(...) and flush_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 185fb037b332..631614856afc 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1301,6 +1301,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 36db179d848e..d2ec620319d7 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -1969,8 +1969,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 13f261feb75c..787f2cac18c5 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -2133,7 +2133,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(&net->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 8cc2a9df84fd..4dbe6ebeabf8 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -1448,20 +1448,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
