[PATCH] mm: don't warn about large allocations for slab

[PATCH] mm: don't warn about large allocations for slab

[Dmitry Vyukov]

From: Dmitry Vyukov <dvyukov@google.com>

This warning does not seem to be useful. Most of the time it fires when
allocation size depends on syscall arguments. We could add __GFP_NOWARN
to these allocation sites, but having a warning only to suppress it
does not make lots of sense. Moreover, this warnings never fires for
constant-size allocations and never for slub, because there are
additional checks and fallback to kmalloc_large() for large allocations
and kmalloc_large() does not warn. So the warning only fires for
non-constant allocations and only with slab, which is odd to begin with.
The warning leads to episodic unuseful syzbot reports. Remote it.

While we are here also fix the check. We should check against
KMALLOC_MAX_CACHE_SIZE rather than KMALLOC_MAX_SIZE. It all kinda
worked because for slab the constants are the same, and slub always
checks the size against KMALLOC_MAX_CACHE_SIZE before kmalloc_slab().
But if we get there with size > KMALLOC_MAX_CACHE_SIZE anyhow
bad things will happen.

Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Reported-by: syzbot+87829a10073277282ad1@syzkaller.appspotmail.com
Reported-by: syzbot+ef4e8fc3a06e9019bb40@syzkaller.appspotmail.com
Reported-by: syzbot+6e438f4036df52cbb863@syzkaller.appspotmail.com
Reported-by: syzbot+8574471d8734457d98aa@syzkaller.appspotmail.com
Reported-by: syzbot+af1504df0807a083dbd9@syzkaller.appspotmail.com
---
 mm/slab_common.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index 1f903589980f9..2733bddcfdc0c 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1023,10 +1023,8 @@ struct kmem_cache *kmalloc_slab(size_t size, gfp_t flags)
 {
 	unsigned int index;
 
-	if (unlikely(size > KMALLOC_MAX_SIZE)) {
-		WARN_ON_ONCE(!(flags & __GFP_NOWARN));
+	if (unlikely(size > KMALLOC_MAX_CACHE_SIZE))
 		return NULL;
-	}
 
 	if (size <= 192) {
 		if (!size)
-- 
2.19.0.605.g01d371f741-goog

Re: [PATCH] mm: don't warn about large allocations for slab

[Christopher Lameter]

On Thu, 27 Sep 2018, Dmitry Vyukov wrote:

> From: Dmitry Vyukov <dvyukov@google.com>
>
> This warning does not seem to be useful. Most of the time it fires when
> allocation size depends on syscall arguments. We could add __GFP_NOWARN
> to these allocation sites, but having a warning only to suppress it
> does not make lots of sense. Moreover, this warnings never fires for
> constant-size allocations and never for slub, because there are
> additional checks and fallback to kmalloc_large() for large allocations
> and kmalloc_large() does not warn. So the warning only fires for
> non-constant allocations and only with slab, which is odd to begin with.
> The warning leads to episodic unuseful syzbot reports. Remote it.

/Remove/

If its only for slab then KMALLOC_MAX_CACHE_SIZE and KMALLOC_MAX_SIZE are
the same value.

> While we are here also fix the check. We should check against
> KMALLOC_MAX_CACHE_SIZE rather than KMALLOC_MAX_SIZE. It all kinda
> worked because for slab the constants are the same, and slub always
> checks the size against KMALLOC_MAX_CACHE_SIZE before kmalloc_slab().
> But if we get there with size > KMALLOC_MAX_CACHE_SIZE anyhow
> bad things will happen.

Then the WARN_ON is correct just change the constant used. Ensure that
SLAB does the same checks as SLUB.

Re: [PATCH] mm: don't warn about large allocations for slab

[Dmitry Vyukov]

On Thu, Sep 27, 2018 at 5:51 PM, Christopher Lameter <cl@linux.com> wrote:
> On Thu, 27 Sep 2018, Dmitry Vyukov wrote:
>
>> From: Dmitry Vyukov <dvyukov@google.com>
>>
>> This warning does not seem to be useful. Most of the time it fires when
>> allocation size depends on syscall arguments. We could add __GFP_NOWARN
>> to these allocation sites, but having a warning only to suppress it
>> does not make lots of sense. Moreover, this warnings never fires for
>> constant-size allocations and never for slub, because there are
>> additional checks and fallback to kmalloc_large() for large allocations
>> and kmalloc_large() does not warn. So the warning only fires for
>> non-constant allocations and only with slab, which is odd to begin with.
>> The warning leads to episodic unuseful syzbot reports. Remote it.
>
> /Remove/
>
> If its only for slab then KMALLOC_MAX_CACHE_SIZE and KMALLOC_MAX_SIZE are
> the same value.
>
>> While we are here also fix the check. We should check against
>> KMALLOC_MAX_CACHE_SIZE rather than KMALLOC_MAX_SIZE. It all kinda
>> worked because for slab the constants are the same, and slub always
>> checks the size against KMALLOC_MAX_CACHE_SIZE before kmalloc_slab().
>> But if we get there with size > KMALLOC_MAX_CACHE_SIZE anyhow
>> bad things will happen.
>
> Then the WARN_ON is correct just change the constant used. Ensure that
> SLAB does the same checks as SLUB.

Mailed v2 which adds the checks to slab.

I think the warning is still slightly wrong. It means a bug in slab
code, it has nothing to do with user-passed flags.