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  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]
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

Powered by blists - more mailing lists