]>
Commit | Line | Data |
---|---|---|
e6832405 SL |
1 | From fa696c9245ffc3997ef065f9f42c528a96ec0654 Mon Sep 17 00:00:00 2001 |
2 | From: Cong Wang <xiyou.wangcong@gmail.com> | |
3 | Date: Fri, 22 Mar 2019 16:26:19 -0700 | |
4 | Subject: xfrm: clean up xfrm protocol checks | |
5 | ||
6 | [ Upstream commit dbb2483b2a46fbaf833cfb5deb5ed9cace9c7399 ] | |
7 | ||
8 | In commit 6a53b7593233 ("xfrm: check id proto in validate_tmpl()") | |
9 | I introduced a check for xfrm protocol, but according to Herbert | |
10 | IPSEC_PROTO_ANY should only be used as a wildcard for lookup, so | |
11 | it should be removed from validate_tmpl(). | |
12 | ||
13 | And, IPSEC_PROTO_ANY is expected to only match 3 IPSec-specific | |
14 | protocols, this is why xfrm_state_flush() could still miss | |
15 | IPPROTO_ROUTING, which leads that those entries are left in | |
16 | net->xfrm.state_all before exit net. Fix this by replacing | |
17 | IPSEC_PROTO_ANY with zero. | |
18 | ||
19 | This patch also extracts the check from validate_tmpl() to | |
20 | xfrm_id_proto_valid() and uses it in parse_ipsecrequest(). | |
21 | With this, no other protocols should be added into xfrm. | |
22 | ||
23 | Fixes: 6a53b7593233 ("xfrm: check id proto in validate_tmpl()") | |
24 | Reported-by: syzbot+0bf0519d6e0de15914fe@syzkaller.appspotmail.com | |
25 | Cc: Steffen Klassert <steffen.klassert@secunet.com> | |
26 | Cc: Herbert Xu <herbert@gondor.apana.org.au> | |
27 | Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> | |
28 | Acked-by: Herbert Xu <herbert@gondor.apana.org.au> | |
29 | Signed-off-by: Steffen Klassert <steffen.klassert@secunet.com> | |
30 | Signed-off-by: Sasha Levin <sashal@kernel.org> | |
31 | --- | |
32 | include/net/xfrm.h | 17 +++++++++++++++++ | |
33 | net/ipv6/xfrm6_tunnel.c | 2 +- | |
34 | net/key/af_key.c | 4 +++- | |
35 | net/xfrm/xfrm_state.c | 2 +- | |
36 | net/xfrm/xfrm_user.c | 14 +------------- | |
37 | 5 files changed, 23 insertions(+), 16 deletions(-) | |
38 | ||
39 | diff --git a/include/net/xfrm.h b/include/net/xfrm.h | |
40 | index 85386becbaea2..902437dfbce77 100644 | |
41 | --- a/include/net/xfrm.h | |
42 | +++ b/include/net/xfrm.h | |
43 | @@ -1404,6 +1404,23 @@ static inline int xfrm_state_kern(const struct xfrm_state *x) | |
44 | return atomic_read(&x->tunnel_users); | |
45 | } | |
46 | ||
47 | +static inline bool xfrm_id_proto_valid(u8 proto) | |
48 | +{ | |
49 | + switch (proto) { | |
50 | + case IPPROTO_AH: | |
51 | + case IPPROTO_ESP: | |
52 | + case IPPROTO_COMP: | |
53 | +#if IS_ENABLED(CONFIG_IPV6) | |
54 | + case IPPROTO_ROUTING: | |
55 | + case IPPROTO_DSTOPTS: | |
56 | +#endif | |
57 | + return true; | |
58 | + default: | |
59 | + return false; | |
60 | + } | |
61 | +} | |
62 | + | |
63 | +/* IPSEC_PROTO_ANY only matches 3 IPsec protocols, 0 could match all. */ | |
64 | static inline int xfrm_id_proto_match(u8 proto, u8 userproto) | |
65 | { | |
66 | return (!userproto || proto == userproto || | |
67 | diff --git a/net/ipv6/xfrm6_tunnel.c b/net/ipv6/xfrm6_tunnel.c | |
68 | index 12cb3aa990af4..d9e5f6808811a 100644 | |
69 | --- a/net/ipv6/xfrm6_tunnel.c | |
70 | +++ b/net/ipv6/xfrm6_tunnel.c | |
71 | @@ -345,7 +345,7 @@ static void __net_exit xfrm6_tunnel_net_exit(struct net *net) | |
72 | unsigned int i; | |
73 | ||
74 | xfrm_flush_gc(); | |
75 | - xfrm_state_flush(net, IPSEC_PROTO_ANY, false, true); | |
76 | + xfrm_state_flush(net, 0, false, true); | |
77 | ||
78 | for (i = 0; i < XFRM6_TUNNEL_SPI_BYADDR_HSIZE; i++) | |
79 | WARN_ON_ONCE(!hlist_empty(&xfrm6_tn->spi_byaddr[i])); | |
80 | diff --git a/net/key/af_key.c b/net/key/af_key.c | |
81 | index 5651c29cb5bd0..4af1e1d60b9f2 100644 | |
82 | --- a/net/key/af_key.c | |
83 | +++ b/net/key/af_key.c | |
84 | @@ -1951,8 +1951,10 @@ parse_ipsecrequest(struct xfrm_policy *xp, struct sadb_x_ipsecrequest *rq) | |
85 | ||
86 | if (rq->sadb_x_ipsecrequest_mode == 0) | |
87 | return -EINVAL; | |
88 | + if (!xfrm_id_proto_valid(rq->sadb_x_ipsecrequest_proto)) | |
89 | + return -EINVAL; | |
90 | ||
91 | - t->id.proto = rq->sadb_x_ipsecrequest_proto; /* XXX check proto */ | |
92 | + t->id.proto = rq->sadb_x_ipsecrequest_proto; | |
93 | if ((mode = pfkey_mode_to_xfrm(rq->sadb_x_ipsecrequest_mode)) < 0) | |
94 | return -EINVAL; | |
95 | t->mode = mode; | |
96 | diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c | |
97 | index 1bb971f46fc6f..178baaa037e5b 100644 | |
98 | --- a/net/xfrm/xfrm_state.c | |
99 | +++ b/net/xfrm/xfrm_state.c | |
100 | @@ -2384,7 +2384,7 @@ void xfrm_state_fini(struct net *net) | |
101 | ||
102 | flush_work(&net->xfrm.state_hash_work); | |
103 | flush_work(&xfrm_state_gc_work); | |
104 | - xfrm_state_flush(net, IPSEC_PROTO_ANY, false, true); | |
105 | + xfrm_state_flush(net, 0, false, true); | |
106 | ||
107 | WARN_ON(!list_empty(&net->xfrm.state_all)); | |
108 | ||
109 | diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c | |
110 | index 8d4d52fd457b2..6916931b1de1c 100644 | |
111 | --- a/net/xfrm/xfrm_user.c | |
112 | +++ b/net/xfrm/xfrm_user.c | |
113 | @@ -1513,20 +1513,8 @@ static int validate_tmpl(int nr, struct xfrm_user_tmpl *ut, u16 family) | |
114 | return -EINVAL; | |
115 | } | |
116 | ||
117 | - switch (ut[i].id.proto) { | |
118 | - case IPPROTO_AH: | |
119 | - case IPPROTO_ESP: | |
120 | - case IPPROTO_COMP: | |
121 | -#if IS_ENABLED(CONFIG_IPV6) | |
122 | - case IPPROTO_ROUTING: | |
123 | - case IPPROTO_DSTOPTS: | |
124 | -#endif | |
125 | - case IPSEC_PROTO_ANY: | |
126 | - break; | |
127 | - default: | |
128 | + if (!xfrm_id_proto_valid(ut[i].id.proto)) | |
129 | return -EINVAL; | |
130 | - } | |
131 | - | |
132 | } | |
133 | ||
134 | return 0; | |
135 | -- | |
136 | 2.20.1 | |
137 |