diff mbox

[Trusty] tcp: take care of truncations done by sk_filter()

Message ID 20170428205851.15429-1-dan.streetman@canonical.com
State New
Headers show

Commit Message

Dan Streetman April 28, 2017, 8:58 p.m. UTC
From: Eric Dumazet <edumazet@google.com>

BugLink: http://bugs.launchpad.net/bugs/1687107

With syzkaller help, Marco Grassi found a bug in TCP stack,
crashing in tcp_collapse()

Root cause is that sk_filter() can truncate the incoming skb,
but TCP stack was not really expecting this to happen.
It probably was expecting a simple DROP or ACCEPT behavior.

We first need to make sure no part of TCP header could be removed.
Then we need to adjust TCP_SKB_CB(skb)->end_seq

Many thanks to syzkaller team and Marco for giving us a reproducer.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Marco Grassi <marco.gra@gmail.com>
Reported-by: Vladis Dronov <vdronov@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(backported from commit ac6e780070e30e4c35bd395acfe9191e6268bdd3)
Signed-off-by: Dan Streetman <dan.streetman@canonical.com>

---
 include/net/tcp.h   |  1 +
 net/ipv4/tcp_ipv4.c | 19 ++++++++++++++++++-
 net/ipv6/tcp_ipv6.c |  6 ++++--
 3 files changed, 23 insertions(+), 3 deletions(-)

Comments

Dan Streetman April 28, 2017, 9:02 p.m. UTC | #1
Sorry please ignore this, the tcp_filter() ffunction needs updating to
work with the 3.13 kernel.

On Fri, Apr 28, 2017 at 4:58 PM, Dan Streetman
<dan.streetman@canonical.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> BugLink: http://bugs.launchpad.net/bugs/1687107
>
> With syzkaller help, Marco Grassi found a bug in TCP stack,
> crashing in tcp_collapse()
>
> Root cause is that sk_filter() can truncate the incoming skb,
> but TCP stack was not really expecting this to happen.
> It probably was expecting a simple DROP or ACCEPT behavior.
>
> We first need to make sure no part of TCP header could be removed.
> Then we need to adjust TCP_SKB_CB(skb)->end_seq
>
> Many thanks to syzkaller team and Marco for giving us a reproducer.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Marco Grassi <marco.gra@gmail.com>
> Reported-by: Vladis Dronov <vdronov@redhat.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> (backported from commit ac6e780070e30e4c35bd395acfe9191e6268bdd3)
> Signed-off-by: Dan Streetman <dan.streetman@canonical.com>
>
> ---
>  include/net/tcp.h   |  1 +
>  net/ipv4/tcp_ipv4.c | 19 ++++++++++++++++++-
>  net/ipv6/tcp_ipv6.c |  6 ++++--
>  3 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index b7e2f6e22527..ddd58d0886df 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -1037,6 +1037,7 @@ static inline void tcp_prequeue_init(struct tcp_sock *tp)
>  }
>
>  bool tcp_prequeue(struct sock *sk, struct sk_buff *skb);
> +int tcp_filter(struct sock *sk, struct sk_buff *skb);
>
>  #undef STATE_TRACE
>
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index 98f3eb8462b7..e4082a19dc76 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1621,6 +1621,21 @@ drop:
>  EXPORT_SYMBOL(tcp_v4_conn_request);
>
>
> +int tcp_filter(struct sock *sk, struct sk_buff *skb)
> +{
> +       struct tcphdr *th = (struct tcphdr *)skb->data;
> +       unsigned int eaten = skb->len;
> +       int err;
> +
> +       err = sk_filter_trim_cap(sk, skb, th->doff * 4);
> +       if (!err) {
> +               eaten -= skb->len;
> +               TCP_SKB_CB(skb)->end_seq -= eaten;
> +       }
> +       return err;
> +}
> +EXPORT_SYMBOL(tcp_filter);
> +
>  /*
>   * The three way handshake has completed - we got a valid synack -
>   * now create the new socket.
> @@ -1997,8 +2012,10 @@ process:
>                 goto discard_and_relse;
>         nf_reset(skb);
>
> -       if (sk_filter(sk, skb))
> +       if (tcp_filter(sk, skb))
>                 goto discard_and_relse;
> +       th = (const struct tcphdr *)skb->data;
> +       iph = ip_hdr(skb);
>
>         sk_mark_napi_id(sk, skb);
>         skb->dev = NULL;
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 0006e37e5c82..69ee7982d09f 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1329,7 +1329,7 @@ static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
>                 goto discard;
>  #endif
>
> -       if (sk_filter(sk, skb))
> +       if (tcp_filter(sk, skb))
>                 goto discard;
>
>         /*
> @@ -1499,8 +1499,10 @@ process:
>         if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
>                 goto discard_and_relse;
>
> -       if (sk_filter(sk, skb))
> +       if (tcp_filter(sk, skb))
>                 goto discard_and_relse;
> +       th = (const struct tcphdr *)skb->data;
> +       hdr = ipv6_hdr(skb);
>
>         sk_mark_napi_id(sk, skb);
>         skb->dev = NULL;
> --
> 2.11.0
>
>
> --
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff mbox

Patch

diff --git a/include/net/tcp.h b/include/net/tcp.h
index b7e2f6e22527..ddd58d0886df 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1037,6 +1037,7 @@  static inline void tcp_prequeue_init(struct tcp_sock *tp)
 }
 
 bool tcp_prequeue(struct sock *sk, struct sk_buff *skb);
+int tcp_filter(struct sock *sk, struct sk_buff *skb);
 
 #undef STATE_TRACE
 
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 98f3eb8462b7..e4082a19dc76 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1621,6 +1621,21 @@  drop:
 EXPORT_SYMBOL(tcp_v4_conn_request);
 
 
+int tcp_filter(struct sock *sk, struct sk_buff *skb)
+{
+	struct tcphdr *th = (struct tcphdr *)skb->data;
+	unsigned int eaten = skb->len;
+	int err;
+
+	err = sk_filter_trim_cap(sk, skb, th->doff * 4);
+	if (!err) {
+		eaten -= skb->len;
+		TCP_SKB_CB(skb)->end_seq -= eaten;
+	}
+	return err;
+}
+EXPORT_SYMBOL(tcp_filter);
+
 /*
  * The three way handshake has completed - we got a valid synack -
  * now create the new socket.
@@ -1997,8 +2012,10 @@  process:
 		goto discard_and_relse;
 	nf_reset(skb);
 
-	if (sk_filter(sk, skb))
+	if (tcp_filter(sk, skb))
 		goto discard_and_relse;
+	th = (const struct tcphdr *)skb->data;
+	iph = ip_hdr(skb);
 
 	sk_mark_napi_id(sk, skb);
 	skb->dev = NULL;
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 0006e37e5c82..69ee7982d09f 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1329,7 +1329,7 @@  static int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 		goto discard;
 #endif
 
-	if (sk_filter(sk, skb))
+	if (tcp_filter(sk, skb))
 		goto discard;
 
 	/*
@@ -1499,8 +1499,10 @@  process:
 	if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
 		goto discard_and_relse;
 
-	if (sk_filter(sk, skb))
+	if (tcp_filter(sk, skb))
 		goto discard_and_relse;
+	th = (const struct tcphdr *)skb->data;
+	hdr = ipv6_hdr(skb);
 
 	sk_mark_napi_id(sk, skb);
 	skb->dev = NULL;