netdev - Re: BUG: KASAN: double-free or invalid-free in ip

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89iJaTAHvKesjibA91hP-1cqHUzBRVmxBHrw0G=9K0twsfw@mail.gmail.com>
Date:   Wed, 30 Jan 2019 15:16:56 -0800
From:   Eric Dumazet <edumazet@...gle.com>
To:     Ivan Babrou <ivan@...udflare.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Michal Kubecek <mkubecek@...e.cz>,
        Linux Kernel Network Developers <netdev@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Ignat Korchagin <ignat@...udflare.com>,
        Shawn Bohrer <sbohrer@...udflare.com>,
        Jakub Sitnicki <jakub@...udflare.com>
Subject: Re: BUG: KASAN: double-free or invalid-free in ip_defrag after
 upgrade from 4.19.13

On Wed, Jan 30, 2019 at 3:13 PM Eric Dumazet <edumazet@...gle.com> wrote:
>
> On Wed, Jan 30, 2019 at 3:09 PM Ivan Babrou <ivan@...udflare.com> wrote:
> >
> > Eric,
> >
> > Are you going to propose the change then?
> >
> > I'm happy to test it out.
> >
>
> This is indeed a bug in linux stable tree only.
>
> The err=-EINVAL move was part of a patch that was not backported
> (since it was not a bug fix)
>
> commit 0ff89efb524631ac9901b81446b453c29711c376
> Author: Peter Oskolkov <posk@...gle.com>
> Date:   Tue Aug 28 11:36:19 2018 -0700
>
>     ip: fail fast on IP defrag errors
>
>

Greg, the fix for 4.19 (and maybe other stable trees ?) would be :

diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index f8bbd693c19c247e41839c2d0b5318ca51b23ee8..d95b32af4a0e3f552405c9e61cc372729834160c
100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -425,6 +425,7 @@ static int ip_frag_queue(struct ipq *qp, struct
sk_buff *skb)
         * fragment.
         */

+       err = -EINVAL;
        /* Find out where to put this fragment.  */
        prev_tail = qp->q.fragments_tail;
        if (!prev_tail)
@@ -501,7 +502,6 @@ static int ip_frag_queue(struct ipq *qp, struct
sk_buff *skb)

 discard_qp:
        inet_frag_kill(&qp->q);
-       err = -EINVAL;
        __IP_INC_STATS(net, IPSTATS_MIB_REASM_OVERLAPS);
 err:
        kfree_skb(skb);



> > On Wed, Jan 30, 2019 at 3:00 PM Michal Kubecek <mkubecek@...e.cz> wrote:
> > >
> > > On Wed, Jan 30, 2019 at 02:26:32PM -0800, Ivan Babrou wrote:
> > > > Hey,
> > > >
> > > > Continuing from this thread earlier today:
> > > >
> > > > * https://marc.info/?t=154886729100001&r=1&w=2
> > > >
> > > > We fired up KASAN enabled kernel one one of those machine and this is
> > > > what we saw:
> > > ...
> > > > This commit from 4.19.14 seems relevant:
> > > >
> > > > * https://github.com/torvalds/linux/commit/d5f9565c8d5ad3cf94982223cfcef1169b0bb60f
> > > >
> > > > As a reminder, we upgraded from 4.19.13 and started seeing crashes.
> > >
> > > Unfortunately I'm on vacation this week so that my capability to look
> > > deeper into this is limited but there seems to be one obvious problem
> > > with the 4.19.y backport: in mainline, there is
> > >
> > >         err = -EINVAL;
> > >
> > > right on top of the "Find out where to put this fragment." comment which
> > > had been added by commit 0ff89efb5246 ("ip: fail fast on IP defrag
> > > errors"). In 4.19.y backport of the commit, this assignment is missing
> > > so that the value of err at this point comes from earlier
> > > pskb_trim_rcsum() call so that it must be zero and if we take any of the
> > > "goto err" added by commit d5f9565c8d5a, we drop the packet by calling
> > > kfree_skb() but return zero so that caller doesn't know about it.
> > >
> > > Michal Kubecek
> > >