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

From: Matthew Wilcox
Date: Thu Jul 22 2021 - 00:02:08 EST


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