linux-kernel - Re: [PATCH] mm: Make kvmalloc refuse to allocate more than 2GB

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]

Date:   Thu, 22 Jul 2021 01:14:29 +0100
From:   Matthew Wilcox <willy@...radead.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Al Viro <viro@...iv.linux.org.uk>,
        Qualys Security Advisory <qsa@...lys.com>,
        Eric Sandeen <sandeen@...hat.com>,
        Linux-MM <linux-mm@...ck.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mm: Make kvmalloc refuse to allocate more than 2GB

On Wed, Jul 21, 2021 at 01:46:09PM -0700, Linus Torvalds wrote:
> On Wed, Jul 21, 2021 at 11:42 AM Matthew Wilcox (Oracle)
> <willy@...radead.org> wrote:
> >
> > It's generally dangerous to allocate such large quantities of memory
> > within the kernel owing to our propensity to use 'int' to represent
> > a length.  If somebody really needs it, we can add a kvmalloc_large()
> > later, but let's default to "You can't allocate that much memory".
> 
> I really think that without the WARN_ON_ONCE(), this is just moving
> that failure point from a known good place ("we know this must not
> succeed") to a possibly bad place ("this might cause silent and
> hard-to-understand failures elsewhere").

To a certain extent, yes.  On the other hand, if you don't have any
error handling on your kvmalloc of 2GB, Qualys seems to have a reliable
way to run you out of vmalloc space, and that's going to get exercised.

My initial thought was to leverage the existing __GFP_NOWARN code:

        if (size > PAGE_SIZE) {
-               kmalloc_flags |= __GFP_NOWARN;
+               if (size <= INT_MAX)
+                       kmalloc_flags |= __GFP_NOWARN;

because that dumps some interesting information (ratelimited), which
might help the sysadmin realise they're under attack.  A WARN_ON_ONCE
is one-and-done, so an attacker can hide their tracks.  Unfortunately,
we actually bail out before getting there:

        if (unlikely(order >= MAX_ORDER)) {
                WARN_ON_ONCE(!(gfp & __GFP_NOWARN));
                return NULL;
        }

... maybe that should call warn_alloc() too.

So I'm now thinking (relative to the earlier patch):

-       if (size > INT_MAX)
+       if (size > INT_MAX) {
+               warn_alloc(flags, NULL, "oversized allocation:%zu", size);
                return NULL;
+       }