netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 net 0/2] ipv6: fix ip6_tnl_parse_tlv_enc_lim() issues
@ 2017-01-24  0:43 Eric Dumazet
  2017-01-24  0:43 ` [PATCH v2 net 1/2] ip6_tunnel: must reload ipv6h in ip6ip6_tnl_xmit() Eric Dumazet
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Eric Dumazet @ 2017-01-24  0:43 UTC (permalink / raw)
  To: David S . Miller
  Cc: netdev, Eric Dumazet, Willem de Bruijn, Alexei Starovoitov,
	Eric Dumazet

First patch fixes ip6_tnl_parse_tlv_enc_lim() callers,
bug added in linux-3.7

Second patch fixes ip6_tnl_parse_tlv_enc_lim() itself,
bug predates linux-2.6.12

Based on a report from Dmitry Vyukov, thanks to KASAN.


Eric Dumazet (2):
  ip6_tunnel: must reload ipv6h in ip6ip6_tnl_xmit()
  ipv6: fix ip6_tnl_parse_tlv_enc_lim()

 net/ipv6/ip6_gre.c    |  3 +++
 net/ipv6/ip6_tunnel.c | 36 ++++++++++++++++++++++++------------
 2 files changed, 27 insertions(+), 12 deletions(-)

-- 
2.11.0.483.g087da7b7c-goog

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v2 net 1/2] ip6_tunnel: must reload ipv6h in ip6ip6_tnl_xmit()
  2017-01-24  0:43 [PATCH v2 net 0/2] ipv6: fix ip6_tnl_parse_tlv_enc_lim() issues Eric Dumazet
@ 2017-01-24  0:43 ` Eric Dumazet
  2017-01-24  0:43 ` [PATCH v2 net 2/2] ipv6: fix ip6_tnl_parse_tlv_enc_lim() Eric Dumazet
  2017-01-24 19:54 ` [PATCH v2 net 0/2] ipv6: fix ip6_tnl_parse_tlv_enc_lim() issues David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2017-01-24  0:43 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 v2 net 2/2] ipv6: fix ip6_tnl_parse_tlv_enc_lim()
  2017-01-24  0:43 [PATCH v2 net 0/2] ipv6: fix ip6_tnl_parse_tlv_enc_lim() issues Eric Dumazet
  2017-01-24  0:43 ` [PATCH v2 net 1/2] ip6_tunnel: must reload ipv6h in ip6ip6_tnl_xmit() Eric Dumazet
@ 2017-01-24  0:43 ` Eric Dumazet
  2017-01-24 19:54 ` [PATCH v2 net 0/2] ipv6: fix ip6_tnl_parse_tlv_enc_lim() issues David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: Eric Dumazet @ 2017-01-24  0:43 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 | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
index 02923f956ac8ba5f3270e8d5282fd9637c2d..ff8ee06491c335d209e86bb15f2526ab1915 100644
--- a/net/ipv6/ip6_tunnel.c
+++ b/net/ipv6/ip6_tunnel.c
@@ -400,18 +400,19 @@ 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;
+	unsigned int off = nhoff + 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, off + sizeof(*hdr)))
 			break;
 
-		hdr = (struct ipv6_opt_hdr *) (raw + off);
+		hdr = (struct ipv6_opt_hdr *)(skb->data + off);
 		if (nexthdr == NEXTHDR_FRAGMENT) {
 			struct frag_hdr *frag_hdr = (struct frag_hdr *) hdr;
 			if (frag_hdr->frag_off)
@@ -422,20 +423,29 @@ __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, off + optlen))
+				break;
+
 			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];
+				tel = (struct ipv6_tlv_tnl_enc_lim *) skb->data + off + i;
 				/* return index of option if found and valid */
 				if (tel->type == IPV6_TLV_TNL_ENCAP_LIMIT &&
 				    tel->length == 1)
-					return i;
+					return i + off - nhoff;
 				/* else jump to next option */
 				if (tel->type)
 					i += tel->length + 2;
@@ -443,7 +453,7 @@ __u16 ip6_tnl_parse_tlv_enc_lim(struct sk_buff *skb, __u8 *raw)
 					i++;
 			}
 		}
-		nexthdr = hdr->nexthdr;
+		nexthdr = next;
 		off += optlen;
 	}
 	return 0;
-- 
2.11.0.483.g087da7b7c-goog

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2 net 0/2] ipv6: fix ip6_tnl_parse_tlv_enc_lim() issues
  2017-01-24  0:43 [PATCH v2 net 0/2] ipv6: fix ip6_tnl_parse_tlv_enc_lim() issues Eric Dumazet
  2017-01-24  0:43 ` [PATCH v2 net 1/2] ip6_tunnel: must reload ipv6h in ip6ip6_tnl_xmit() Eric Dumazet
  2017-01-24  0:43 ` [PATCH v2 net 2/2] ipv6: fix ip6_tnl_parse_tlv_enc_lim() Eric Dumazet
@ 2017-01-24 19:54 ` David Miller
  2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2017-01-24 19:54 UTC (permalink / raw)
  To: edumazet; +Cc: netdev, willemb, ast, eric.dumazet

From: Eric Dumazet <edumazet@google.com>
Date: Mon, 23 Jan 2017 16:43:04 -0800

> First patch fixes ip6_tnl_parse_tlv_enc_lim() callers,
> bug added in linux-3.7
> 
> Second patch fixes ip6_tnl_parse_tlv_enc_lim() itself,
> bug predates linux-2.6.12
> 
> Based on a report from Dmitry Vyukov, thanks to KASAN.

Series applied and queued up for -stable, thanks Eric.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-01-24 19:54 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-24  0:43 [PATCH v2 net 0/2] ipv6: fix ip6_tnl_parse_tlv_enc_lim() issues Eric Dumazet
2017-01-24  0:43 ` [PATCH v2 net 1/2] ip6_tunnel: must reload ipv6h in ip6ip6_tnl_xmit() Eric Dumazet
2017-01-24  0:43 ` [PATCH v2 net 2/2] ipv6: fix ip6_tnl_parse_tlv_enc_lim() Eric Dumazet
2017-01-24 19:54 ` [PATCH v2 net 0/2] ipv6: fix ip6_tnl_parse_tlv_enc_lim() issues David Miller

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).