| 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 |
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; > }
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; >> }
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; >>> }
... > > 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)
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 --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; }
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(-)