diff mbox series

[kernel] KVM: PPC: Book3S: Suppress warnings when allocating too big memory slots

Message ID 20210901084512.1658628-1-aik@ozlabs.ru
State New
Headers show
Series [kernel] KVM: PPC: Book3S: Suppress warnings when allocating too big memory slots | expand

Commit Message

Alexey Kardashevskiy Sept. 1, 2021, 8:45 a.m. UTC
The userspace can trigger "vmalloc size %lu allocation failure: exceeds
total pages" via the KVM_SET_USER_MEMORY_REGION ioctl.

This silences the warning by checking the limit before calling vzalloc()
and returns ENOMEM if failed.

This does not call underlying valloc helpers as __vmalloc_node() is only
exported when CONFIG_TEST_VMALLOC_MODULE and __vmalloc_node_range() is not
exported at all.

Spotted by syzkaller.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/kvm/book3s_hv.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Fabiano Rosas Sept. 1, 2021, 2:59 p.m. UTC | #1
Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> The userspace can trigger "vmalloc size %lu allocation failure: exceeds
> total pages" via the KVM_SET_USER_MEMORY_REGION ioctl.
>
> This silences the warning by checking the limit before calling vzalloc()
> and returns ENOMEM if failed.
>
> This does not call underlying valloc helpers as __vmalloc_node() is only
> exported when CONFIG_TEST_VMALLOC_MODULE and __vmalloc_node_range() is not
> exported at all.
>
> Spotted by syzkaller.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  arch/powerpc/kvm/book3s_hv.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 474c0cfde384..a59f1cccbcf9 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -4830,8 +4830,12 @@ static int kvmppc_core_prepare_memory_region_hv(struct kvm *kvm,
>  	unsigned long npages = mem->memory_size >> PAGE_SHIFT;
>
>  	if (change == KVM_MR_CREATE) {
> -		slot->arch.rmap = vzalloc(array_size(npages,
> -					  sizeof(*slot->arch.rmap)));
> +		unsigned long cb = array_size(npages, sizeof(*slot->arch.rmap));

What does cb mean?

> +
> +		if ((cb >> PAGE_SHIFT) > totalram_pages())
> +			return -ENOMEM;
> +
> +		slot->arch.rmap = vzalloc(cb);
>  		if (!slot->arch.rmap)
>  			return -ENOMEM;
>  	}
Alexey Kardashevskiy Sept. 2, 2021, 4:25 a.m. UTC | #2
On 02/09/2021 00:59, Fabiano Rosas wrote:
> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> 
>> The userspace can trigger "vmalloc size %lu allocation failure: exceeds
>> total pages" via the KVM_SET_USER_MEMORY_REGION ioctl.
>>
>> This silences the warning by checking the limit before calling vzalloc()
>> and returns ENOMEM if failed.
>>
>> This does not call underlying valloc helpers as __vmalloc_node() is only
>> exported when CONFIG_TEST_VMALLOC_MODULE and __vmalloc_node_range() is not
>> exported at all.
>>
>> Spotted by syzkaller.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>>   arch/powerpc/kvm/book3s_hv.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 474c0cfde384..a59f1cccbcf9 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -4830,8 +4830,12 @@ static int kvmppc_core_prepare_memory_region_hv(struct kvm *kvm,
>>   	unsigned long npages = mem->memory_size >> PAGE_SHIFT;
>>
>>   	if (change == KVM_MR_CREATE) {
>> -		slot->arch.rmap = vzalloc(array_size(npages,
>> -					  sizeof(*slot->arch.rmap)));
>> +		unsigned long cb = array_size(npages, sizeof(*slot->arch.rmap));
> 
> What does cb mean?

"count of bytes"

This is from my deep Windows past :)

https://docs.microsoft.com/en-us/windows/win32/stg/coding-style-conventions


> 
>> +
>> +		if ((cb >> PAGE_SHIFT) > totalram_pages())
>> +			return -ENOMEM;
>> +
>> +		slot->arch.rmap = vzalloc(cb);
>>   		if (!slot->arch.rmap)
>>   			return -ENOMEM;
>>   	}
Fabiano Rosas Sept. 2, 2021, 1:08 p.m. UTC | #3
Alexey Kardashevskiy <aik@ozlabs.ru> writes:

> On 02/09/2021 00:59, Fabiano Rosas wrote:
>> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>> 
>>> The userspace can trigger "vmalloc size %lu allocation failure: exceeds
>>> total pages" via the KVM_SET_USER_MEMORY_REGION ioctl.
>>>
>>> This silences the warning by checking the limit before calling vzalloc()
>>> and returns ENOMEM if failed.
>>>
>>> This does not call underlying valloc helpers as __vmalloc_node() is only
>>> exported when CONFIG_TEST_VMALLOC_MODULE and __vmalloc_node_range() is not
>>> exported at all.
>>>
>>> Spotted by syzkaller.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>>   arch/powerpc/kvm/book3s_hv.c | 8 ++++++--
>>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>>> index 474c0cfde384..a59f1cccbcf9 100644
>>> --- a/arch/powerpc/kvm/book3s_hv.c
>>> +++ b/arch/powerpc/kvm/book3s_hv.c
>>> @@ -4830,8 +4830,12 @@ static int kvmppc_core_prepare_memory_region_hv(struct kvm *kvm,
>>>   	unsigned long npages = mem->memory_size >> PAGE_SHIFT;
>>>
>>>   	if (change == KVM_MR_CREATE) {
>>> -		slot->arch.rmap = vzalloc(array_size(npages,
>>> -					  sizeof(*slot->arch.rmap)));
>>> +		unsigned long cb = array_size(npages, sizeof(*slot->arch.rmap));
>> 
>> What does cb mean?
>
> "count of bytes"
>
> This is from my deep Windows past :)
>
> https://docs.microsoft.com/en-us/windows/win32/stg/coding-style-conventions

=D How interesting! And according to that link 'sz' means "Zero terminated
String". Imagine the confusion.. haha

>> 
>>> +
>>> +		if ((cb >> PAGE_SHIFT) > totalram_pages())
>>> +			return -ENOMEM;
>>> +
>>> +		slot->arch.rmap = vzalloc(cb);
>>>   		if (!slot->arch.rmap)
>>>   			return -ENOMEM;
>>>   	}
David Laight Sept. 2, 2021, 1:23 p.m. UTC | #4
...
> > This is from my deep Windows past :)
> >
> > https://docs.microsoft.com/en-us/windows/win32/stg/coding-style-conventions
> 
> =D How interesting! And according to that link 'sz' means "Zero terminated
> String". Imagine the confusion.. haha

Is that document responsible for some of the general unreadability
of windows code?
(I'm not going to addle by brain by trying to read it.)

Types like DWORD_PTR really shouldn't exist.
You won't guess what it is...

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Michael Ellerman Dec. 15, 2021, 12:40 a.m. UTC | #5
On Wed, 1 Sep 2021 18:45:12 +1000, Alexey Kardashevskiy wrote:
> The userspace can trigger "vmalloc size %lu allocation failure: exceeds
> total pages" via the KVM_SET_USER_MEMORY_REGION ioctl.
> 
> This silences the warning by checking the limit before calling vzalloc()
> and returns ENOMEM if failed.
> 
> This does not call underlying valloc helpers as __vmalloc_node() is only
> exported when CONFIG_TEST_VMALLOC_MODULE and __vmalloc_node_range() is not
> exported at all.
> 
> [...]

Applied to powerpc/next.

[1/1] KVM: PPC: Book3S: Suppress warnings when allocating too big memory slots
      https://git.kernel.org/powerpc/c/511d25d6b789fffcb20a3eb71899cf974a31bd9d

cheers
diff mbox series

Patch

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 474c0cfde384..a59f1cccbcf9 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -4830,8 +4830,12 @@  static int kvmppc_core_prepare_memory_region_hv(struct kvm *kvm,
 	unsigned long npages = mem->memory_size >> PAGE_SHIFT;
 
 	if (change == KVM_MR_CREATE) {
-		slot->arch.rmap = vzalloc(array_size(npages,
-					  sizeof(*slot->arch.rmap)));
+		unsigned long cb = array_size(npages, sizeof(*slot->arch.rmap));
+
+		if ((cb >> PAGE_SHIFT) > totalram_pages())
+			return -ENOMEM;
+
+		slot->arch.rmap = vzalloc(cb);
 		if (!slot->arch.rmap)
 			return -ENOMEM;
 	}