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

[Matthew Wilcox <willy@infradead.org>]

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@infradead.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;
+       }