linux-kernel - Re: [PATCH 4.14 198/205] perf/core: Dont WARN() for impossible ring-buffer sizes

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] [day] [month] [year] [list]

Message-ID: <CAHk-=wgpU+5zAgakcDksXfEdJCtBFRdHm7Hqb_iDc+VWYKiqnA@mail.gmail.com>
Date:   Wed, 13 Feb 2019 10:08:35 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     "Rantala, Tommi T. (Nokia - FI/Espoo)" <tommi.t.rantala@...ia.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "mingo@...nel.org" <mingo@...nel.org>,
        "jolsa@...hat.com" <jolsa@...hat.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        "namhyung@...nel.org" <namhyung@...nel.org>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "acme@...hat.com" <acme@...hat.com>,
        "alexander.shishkin@...ux.intel.com" 
        <alexander.shishkin@...ux.intel.com>,
        "julien.thierry@....com" <julien.thierry@....com>
Subject: Re: [PATCH 4.14 198/205] perf/core: Dont WARN() for impossible
 ring-buffer sizes

On Wed, Feb 13, 2019 at 5:13 AM Peter Zijlstra <peterz@...radead.org> wrote:
>
> There is this queued:
>
>   http://lkml.kernel.org/r/tip-528871b456026e6127d95b1b2bd8e3a003dc1614@git.kernel.org

I think it would likely have been better to just remove the test
entirely, and instead just use __GFP_NOWARN.

This is an allocation essentially done for the user (as part of
mmap(), and we handle the failure case fine. So it doesn't matter
whether the warning is due to being out of memory or being a bad size,
we shouldn't cause a kernel warning.

That said, I think we should *indepdendently* have some sane limit for
"nr_pages", simply because we do that

        ...
        for (i = 0; i < nr_pages; i++) {
                rb->data_pages[i] = perf_mmap_alloc_page(cpu);

thing, which is really really expensive when "nr_pages" is in the
thousands (or millions). The fact that we end up almost _accidentally_
limiting nr_pages simply because the kzmalloc() size is limited is I
think a mistake.

So I'd rather see __GFP_NORWARN _and_ some kind of "don't allow crazy
perf ring buffers" check that is separate and independent from any
kmalloc issue..

For example, right now the maximum size you can allocate is (insanely)
tied to the page size. With a bigger page size, not only do you get a
bigger ring buffer anyway, but you also get more 'nr_pages', so it's
basically quadratic behavior. Does that make sense? It doesn't sound
that way to me.

                  Linus