| Message ID | 1496832727-15373-1-git-send-email-stefan.bader@canonical.com |
|---|---|
| State | New |
| Headers | show |
On 07/06/17 11:52, Stefan Bader wrote: > Patch applies as cherry-pick to all releases. Build-tested on Trusty > amd64. > > -Stefan > > --- > > From 232cd35d0804cc241eb887bb8d4d9b3b9881c64a Mon Sep 17 00:00:00 2001 > From: Eric Dumazet <edumazet@google.com> > Date: Fri, 19 May 2017 14:17:48 -0700 > Subject: [PATCH] ipv6: fix out of bound writes in __ip6_append_data() > > Andrey Konovalov and idaifish@gmail.com reported crashes caused by > one skb shared_info being overwritten from __ip6_append_data() > > Andrey program lead to following state : > > copy -4200 datalen 2000 fraglen 2040 > maxfraglen 2040 alloclen 2048 transhdrlen 0 offset 0 fraggap 6200 > > The skb_copy_and_csum_bits(skb_prev, maxfraglen, data + transhdrlen, > fraggap, 0); is overwriting skb->head and skb_shared_info > > Since we apparently detect this rare condition too late, move the > code earlier to even avoid allocating skb and risking crashes. > > Once again, many thanks to Andrey and syzkaller team. > > Signed-off-by: Eric Dumazet <edumazet@google.com> > Reported-by: Andrey Konovalov <andreyknvl@google.com> > Tested-by: Andrey Konovalov <andreyknvl@google.com> > Reported-by: <idaifish@gmail.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > > CVE-2017-9242 > > (cherry-picked from 232cd35d0804cc241eb887bb8d4d9b3b9881c64a) > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > --- > net/ipv6/ip6_output.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c > index d4a31be..bf8a58a 100644 > --- a/net/ipv6/ip6_output.c > +++ b/net/ipv6/ip6_output.c > @@ -1466,6 +1466,11 @@ static int __ip6_append_data(struct sock *sk, > */ > alloclen += sizeof(struct frag_hdr); > > + copy = datalen - transhdrlen - fraggap; > + if (copy < 0) { > + err = -EINVAL; > + goto error; > + } > if (transhdrlen) { > skb = sock_alloc_send_skb(sk, > alloclen + hh_len, > @@ -1515,13 +1520,9 @@ static int __ip6_append_data(struct sock *sk, > data += fraggap; > pskb_trim_unique(skb_prev, maxfraglen); > } > - copy = datalen - transhdrlen - fraggap; > - > - if (copy < 0) { > - err = -EINVAL; > - kfree_skb(skb); > - goto error; > - } else if (copy > 0 && getfrag(from, data + transhdrlen, offset, copy, fraggap, skb) < 0) { > + if (copy > 0 && > + getfrag(from, data + transhdrlen, offset, > + copy, fraggap, skb) < 0) { > err = -EFAULT; > kfree_skb(skb); > goto error; > Clean cherry pick, looks good to me. Thanks Stefan Acked-by: Colin Ian King <colin.king@canonical.com>
On Wed, Jun 07, 2017 at 12:52:07PM +0200, Stefan Bader wrote: > Patch applies as cherry-pick to all releases. Build-tested on Trusty > amd64. > > -Stefan > > --- > > From 232cd35d0804cc241eb887bb8d4d9b3b9881c64a Mon Sep 17 00:00:00 2001 > From: Eric Dumazet <edumazet@google.com> > Date: Fri, 19 May 2017 14:17:48 -0700 > Subject: [PATCH] ipv6: fix out of bound writes in __ip6_append_data() > > Andrey Konovalov and idaifish@gmail.com reported crashes caused by > one skb shared_info being overwritten from __ip6_append_data() > > Andrey program lead to following state : > > copy -4200 datalen 2000 fraglen 2040 > maxfraglen 2040 alloclen 2048 transhdrlen 0 offset 0 fraggap 6200 > > The skb_copy_and_csum_bits(skb_prev, maxfraglen, data + transhdrlen, > fraggap, 0); is overwriting skb->head and skb_shared_info > > Since we apparently detect this rare condition too late, move the > code earlier to even avoid allocating skb and risking crashes. > > Once again, many thanks to Andrey and syzkaller team. > > Signed-off-by: Eric Dumazet <edumazet@google.com> > Reported-by: Andrey Konovalov <andreyknvl@google.com> > Tested-by: Andrey Konovalov <andreyknvl@google.com> > Reported-by: <idaifish@gmail.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > > CVE-2017-9242 > > (cherry-picked from 232cd35d0804cc241eb887bb8d4d9b3b9881c64a) > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > --- > net/ipv6/ip6_output.c | 15 ++++++++------- > 1 file changed, 8 insertions(+), 7 deletions(-) > > diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c > index d4a31be..bf8a58a 100644 > --- a/net/ipv6/ip6_output.c > +++ b/net/ipv6/ip6_output.c > @@ -1466,6 +1466,11 @@ static int __ip6_append_data(struct sock *sk, > */ > alloclen += sizeof(struct frag_hdr); > > + copy = datalen - transhdrlen - fraggap; > + if (copy < 0) { > + err = -EINVAL; > + goto error; > + } > if (transhdrlen) { > skb = sock_alloc_send_skb(sk, > alloclen + hh_len, > @@ -1515,13 +1520,9 @@ static int __ip6_append_data(struct sock *sk, > data += fraggap; > pskb_trim_unique(skb_prev, maxfraglen); > } > - copy = datalen - transhdrlen - fraggap; > - > - if (copy < 0) { > - err = -EINVAL; > - kfree_skb(skb); > - goto error; > - } else if (copy > 0 && getfrag(from, data + transhdrlen, offset, copy, fraggap, skb) < 0) { > + if (copy > 0 && > + getfrag(from, data + transhdrlen, offset, > + copy, fraggap, skb) < 0) { > err = -EFAULT; > kfree_skb(skb); > goto error; Looks to do what is claimed. Clean cherry-pick. Acked-by: Andy Whitcroft <apw@canonical.com> -apw
Applied to trusty, xenial, yakkety, zesty master-next branches. Thanks. Cascardo.
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c index d4a31be..bf8a58a 100644 --- a/net/ipv6/ip6_output.c +++ b/net/ipv6/ip6_output.c @@ -1466,6 +1466,11 @@ static int __ip6_append_data(struct sock *sk, */ alloclen += sizeof(struct frag_hdr); + copy = datalen - transhdrlen - fraggap; + if (copy < 0) { + err = -EINVAL; + goto error; + } if (transhdrlen) { skb = sock_alloc_send_skb(sk, alloclen + hh_len, @@ -1515,13 +1520,9 @@ static int __ip6_append_data(struct sock *sk, data += fraggap; pskb_trim_unique(skb_prev, maxfraglen); } - copy = datalen - transhdrlen - fraggap; - - if (copy < 0) { - err = -EINVAL; - kfree_skb(skb); - goto error; - } else if (copy > 0 && getfrag(from, data + transhdrlen, offset, copy, fraggap, skb) < 0) { + if (copy > 0 && + getfrag(from, data + transhdrlen, offset, + copy, fraggap, skb) < 0) { err = -EFAULT; kfree_skb(skb); goto error;