* [PATCH net-next 1/2] ip6_tunnel: must reload ipv6h in ip6ip6_tnl_xmit()
2017-01-23 23:58 [PATCH net 0/2] ipv6: fix ip6_tnl_parse_tlv_enc_lim() issues Eric Dumazet
@ 2017-01-23 23:58 ` Eric Dumazet
2017-01-23 23:58 ` [PATCH net-next 2/2] ipv6: fix ip6_tnl_parse_tlv_enc_lim() Eric Dumazet
1 sibling, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2017-01-23 23:58 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Willem de Bruijn, Alexei Starovoitov,
Eric Dumazet, Dmitry Kozlov
Since ip6_tnl_parse_tlv_enc_lim() can call pskb_may_pull(),
we must reload any pointer that was related to skb->head
(or skb->data), or risk use after free.
Fixes: c12b395a4664 ("gre: Support GRE over IPv6")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Dmitry Kozlov <xeb@mail.ru>
---
net/ipv6/ip6_gre.c | 3 +++
net/ipv6/ip6_tunnel.c | 2 ++
2 files changed, 5 insertions(+)
diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
index 75b6108234dd05a54af0ae51c7c11eaf1ca2..558631860d91bbcb1321a4b566b6e92ddcaf 100644
--- a/net/ipv6/ip6_gre.c
+++ b/net/ipv6/ip6_gre.c
@@ -582,6 +582,9 @@ static inline int ip6gre_xmit_ipv6(struct sk_buff *skb, struct net_device *dev)
return -1;
offset = ip6_tnl_parse_tlv_enc_lim(skb, skb_network_header(skb));
+ /* ip6_tnl_parse_tlv_enc_lim() might have reallocated skb->head */
+ ipv6h = ipv6_hdr(skb);
+
if (offset > 0) {
struct ipv6_tlv_tnl_enc_lim *tel;
tel = (struct ipv6_tlv_tnl_enc_lim *)&skb_network_header(skb)[offset];
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 753d6d0860fb14c100ab8b20799782ab8160..02923f956ac8ba5f3270e8d5282fd9637c2d 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -1303,6 +1303,8 @@ ip6ip6_tnl_xmit(struct sk_buff *skb, struct net_device *dev)
fl6.flowlabel = key->label;
} else {
offset = ip6_tnl_parse_tlv_enc_lim(skb, skb_network_header(skb));
+ /* ip6_tnl_parse_tlv_enc_lim() might have reallocated skb->head */
+ ipv6h = ipv6_hdr(skb);
if (offset > 0) {
struct ipv6_tlv_tnl_enc_lim *tel;
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH net-next 2/2] ipv6: fix ip6_tnl_parse_tlv_enc_lim()
2017-01-23 23:58 [PATCH net 0/2] ipv6: fix ip6_tnl_parse_tlv_enc_lim() issues Eric Dumazet
2017-01-23 23:58 ` [PATCH net-next 1/2] ip6_tunnel: must reload ipv6h in ip6ip6_tnl_xmit() Eric Dumazet
@ 2017-01-23 23:58 ` Eric Dumazet
2017-01-24 0:06 ` Eric Dumazet
1 sibling, 1 reply; 4+ messages in thread
From: Eric Dumazet @ 2017-01-23 23:58 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Willem de Bruijn, Alexei Starovoitov,
Eric Dumazet
This function suffers from multiple issues.
First one is that pskb_may_pull() may reallocate skb->head,
so the 'raw' pointer needs either to be reloaded or not used at all.
Second issue is that NEXTHDR_DEST handling does not validate
that the options are present in skb->data, so we might read
garbage or access non existent memory.
With help from Willem de Bruijn.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Cc: Willem de Bruijn <willemb@google.com>
---
net/ipv6/ip6_tunnel.c | 32 +++++++++++++++++++++-----------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 02923f956ac8ba5f3270e8d5282fd9637c2d..f9c43c9043f7e386108bca2f2aeb866cfb76 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -400,18 +400,18 @@ ip6_tnl_dev_uninit(struct net_device *dev)
__u16 ip6_tnl_parse_tlv_enc_lim(struct sk_buff *skb, __u8 *raw)
{
- const struct ipv6hdr *ipv6h = (const struct ipv6hdr *) raw;
- __u8 nexthdr = ipv6h->nexthdr;
- __u16 off = sizeof(*ipv6h);
+ const struct ipv6hdr *ipv6h = (const struct ipv6hdr *)raw;
+ unsigned int nhoff = (raw - skb->data) + sizeof(*ipv6h);
+ u8 next, nexthdr = ipv6h->nexthdr;
while (ipv6_ext_hdr(nexthdr) && nexthdr != NEXTHDR_NONE) {
- __u16 optlen = 0;
struct ipv6_opt_hdr *hdr;
- if (raw + off + sizeof(*hdr) > skb->data &&
- !pskb_may_pull(skb, raw - skb->data + off + sizeof (*hdr)))
+ u16 optlen;
+
+ if (!pskb_may_pull(skb, nhoff + sizeof(*hdr)))
break;
- hdr = (struct ipv6_opt_hdr *) (raw + off);
+ hdr = (struct ipv6_opt_hdr *)(skb->data + nhoff);
if (nexthdr == NEXTHDR_FRAGMENT) {
struct frag_hdr *frag_hdr = (struct frag_hdr *) hdr;
if (frag_hdr->frag_off)
@@ -422,13 +422,23 @@ __u16 ip6_tnl_parse_tlv_enc_lim(struct sk_buff *skb, __u8 *raw)
} else {
optlen = ipv6_optlen(hdr);
}
+ /* cache hdr->nexthdr, since pskb_may_pull() might
+ * invalidate hdr
+ */
+ next = hdr->nexthdr;
if (nexthdr == NEXTHDR_DEST) {
- __u16 i = off + 2;
+ u16 i = 2;
+
+ /* Remember : hdr is no longer valid at this point. */
+ if (!pskb_may_pull(skb, nhoff + optlen))
+ break;
+
+ raw = skb->data + nhoff;
while (1) {
struct ipv6_tlv_tnl_enc_lim *tel;
/* No more room for encapsulation limit */
- if (i + sizeof (*tel) > off + optlen)
+ if (i + sizeof(*tel) > optlen)
break;
tel = (struct ipv6_tlv_tnl_enc_lim *) &raw[i];
@@ -443,8 +453,8 @@ __u16 ip6_tnl_parse_tlv_enc_lim(struct sk_buff *skb, __u8 *raw)
i++;
}
}
- nexthdr = hdr->nexthdr;
- off += optlen;
+ nexthdr = next;
+ nhoff += optlen;
}
return 0;
}
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread