Re: [PATCH -next] mm: delete oversized WARN_ON() in kvmalloc() calls

[Bixuan Cui <cuibixuan@linux.alibaba.com>]

[-- Attachment #1: Type: text/plain, Size: 2058 bytes --]


在 2021/12/2 上午11:46, Kees Cook 写道:
> On Thu, Dec 02, 2021 at 10:06:24AM +0800, Bixuan Cui wrote:
>> Delete the WARN_ON() and return NULL directly for oversized parameter
>> in kvmalloc() calls.
>> Also add unlikely().
>>
>> Fixes: 7661809d493b ("mm: don't allow oversized kvmalloc() calls")
>> Signed-off-by: Bixuan Cui<cuibixuan@linux.alibaba.com>
>> ---
>> There are a lot of oversize warnings and patches about kvmalloc() calls
>> recently. Maybe these warnings are not very necessary.
> It seems these warnings are working, yes? i.e. we're finding the places
> where giant values are coming in?
Yes,  It's working.
>
>> https://lore.kernel.org/all/YadOjJXMTjP85MQx@unreal
>>
>> The example of size check in __do_kmalloc_node():
>> __do_kmalloc_node(size_t size, gfp_t flags, int node, unsigned long caller)
>> {
>>          struct kmem_cache *cachep;
>>          void *ret;
>>
>>          if (unlikely(size > KMALLOC_MAX_CACHE_SIZE))
>>                  return NULL;
>>          cachep = kmalloc_slab(size, flags);
>>
>>   mm/util.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/util.c b/mm/util.c
>> index 7e433690..d26f19c 100644
>> --- a/mm/util.c
>> +++ b/mm/util.c
>> @@ -587,7 +587,7 @@ void *kvmalloc_node(size_t size, gfp_t flags, int node)
>>   		return ret;
>>   
>>   	/* Don't even allow crazy sizes */
>> -	if (WARN_ON_ONCE(size > INT_MAX))
>> +	if (unlikely(size > INT_MAX))
>>   		return NULL;
> If we're rejecting the value, then it's still a pathological size, so
> shouldn't the check be happening in the caller? I think the WARN is
> doing exactly what it was supposed to do: find the places where bad
> sizes can reach vmalloc.
In this way, we must check whether the size from the user exceeds INT_MAX
before calling kvmalloc() calls.  Generally speaking, the oversize check 
is rarely
done before.

Thanks,
Bixuan Cui


>
> -Kees
>
>>   
>>   	return __vmalloc_node(size, 1, flags, node,
>> -- 
>> 1.8.3.1
>>
> -- Kees Cook

[-- Attachment #2: Type: text/html, Size: 4119 bytes --]