* [Patch net v2] xfrm: clean up xfrm protocol checks
@ 2019-03-22 23:26 Cong Wang
2019-03-25 10:42 ` Herbert Xu
0 siblings, 1 reply; 3+ messages in thread
From: Cong Wang @ 2019-03-22 23:26 UTC (permalink / raw)
To: netdev
Cc: Cong Wang, syzbot+0bf0519d6e0de15914fe, Steffen Klassert, Herbert Xu
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>
---
include/net/xfrm.h | 17 +++++++++++++++++
net/ipv6/xfrm6_tunnel.c | 2 +-
net/key/af_key.c | 4 +++-
net/xfrm/xfrm_state.c | 2 +-
net/xfrm/xfrm_user.c | 14 +-------------
5 files changed, 23 insertions(+), 16 deletions(-)
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 85386becbaea..902437dfbce7 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -1404,6 +1404,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/ipv6/xfrm6_tunnel.c b/net/ipv6/xfrm6_tunnel.c
index bc65db782bfb..eb2b844b9796 100644
--- a/net/ipv6/xfrm6_tunnel.c
+++ b/net/ipv6/xfrm6_tunnel.c
@@ -345,7 +345,7 @@ static void __net_exit xfrm6_tunnel_net_exit(struct net *net)
unsigned int i;
xfrm_flush_gc();
- xfrm_state_flush(net, IPSEC_PROTO_ANY, false, true);
+ xfrm_state_flush(net, 0, false, true);
for (i = 0; i < XFRM6_TUNNEL_SPI_BYADDR_HSIZE; i++)
WARN_ON_ONCE(!hlist_empty(&xfrm6_tn->spi_byaddr[i]));
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_state.c b/net/xfrm/xfrm_state.c
index 1bb971f46fc6..178baaa037e5 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -2384,7 +2384,7 @@ void xfrm_state_fini(struct net *net)
flush_work(&net->xfrm.state_hash_work);
flush_work(&xfrm_state_gc_work);
- xfrm_state_flush(net, IPSEC_PROTO_ANY, false, true);
+ xfrm_state_flush(net, 0, false, true);
WARN_ON(!list_empty(&net->xfrm.state_all));
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Patch net v2] xfrm: clean up xfrm protocol checks
2019-03-22 23:26 [Patch net v2] xfrm: clean up xfrm protocol checks Cong Wang
@ 2019-03-25 10:42 ` Herbert Xu
2019-03-27 8:23 ` Steffen Klassert
0 siblings, 1 reply; 3+ messages in thread
From: Herbert Xu @ 2019-03-25 10:42 UTC (permalink / raw)
To: Cong Wang; +Cc: netdev, syzbot+0bf0519d6e0de15914fe, Steffen Klassert
On Fri, Mar 22, 2019 at 04:26:19PM -0700, Cong Wang wrote:
> 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>
> ---
> include/net/xfrm.h | 17 +++++++++++++++++
> net/ipv6/xfrm6_tunnel.c | 2 +-
> net/key/af_key.c | 4 +++-
> net/xfrm/xfrm_state.c | 2 +-
> net/xfrm/xfrm_user.c | 14 +-------------
> 5 files changed, 23 insertions(+), 16 deletions(-)
Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
Thanks,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Patch net v2] xfrm: clean up xfrm protocol checks
2019-03-25 10:42 ` Herbert Xu
@ 2019-03-27 8:23 ` Steffen Klassert
0 siblings, 0 replies; 3+ messages in thread
From: Steffen Klassert @ 2019-03-27 8:23 UTC (permalink / raw)
To: Herbert Xu; +Cc: Cong Wang, netdev, syzbot+0bf0519d6e0de15914fe
On Mon, Mar 25, 2019 at 06:42:58PM +0800, Herbert Xu wrote:
> On Fri, Mar 22, 2019 at 04:26:19PM -0700, Cong Wang wrote:
> > 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>
> > ---
> > include/net/xfrm.h | 17 +++++++++++++++++
> > net/ipv6/xfrm6_tunnel.c | 2 +-
> > net/key/af_key.c | 4 +++-
> > net/xfrm/xfrm_state.c | 2 +-
> > net/xfrm/xfrm_user.c | 14 +-------------
> > 5 files changed, 23 insertions(+), 16 deletions(-)
>
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
>
Patch applied, thanks everyone!
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-03-27 8:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-22 23:26 [Patch net v2] xfrm: clean up xfrm protocol checks Cong Wang
2019-03-25 10:42 ` Herbert Xu
2019-03-27 8:23 ` Steffen Klassert
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.