kvm: x86: mmu: fix memoryleak in kvm_mmu_vendor_module_init()

[PATCH] kvm: x86: mmu: fix memoryleak in kvm_mmu_vendor_module_init()

Posted by Miaohe Lin 3 years, 7 months ago

When register_shrinker() fails, we forgot to release the percpu counter
kvm_total_used_mmu_pages leading to memoryleak. Fix this issue by calling
percpu_counter_destroy() when register_shrinker() fails.

Fixes: ab271bd4dfd5 ("x86: kvm: propagate register_shrinker return code")
Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
---
 arch/x86/kvm/mmu/mmu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index e418ef3ecfcb..d25d55b1f0b5 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -6702,10 +6702,12 @@ int kvm_mmu_vendor_module_init(void)
 
 	ret = register_shrinker(&mmu_shrinker, "x86-mmu");
 	if (ret)
-		goto out;
+		goto out_shrinker;
 
 	return 0;
 
+out_shrinker:
+	percpu_counter_destroy(&kvm_total_used_mmu_pages);
 out:
 	mmu_destroy_caches();
 	return ret;
-- 
2.23.0

Re: [PATCH] kvm: x86: mmu: fix memoryleak in kvm_mmu_vendor_module_init()

Posted by Sean Christopherson 3 years, 7 months ago

On Tue, Aug 23, 2022, Miaohe Lin wrote:
> When register_shrinker() fails, we forgot to release the percpu counter
> kvm_total_used_mmu_pages leading to memoryleak. Fix this issue by calling
> percpu_counter_destroy() when register_shrinker() fails.
> 
> Fixes: ab271bd4dfd5 ("x86: kvm: propagate register_shrinker return code")
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---

Pushed to branch `for_paolo/6.1` at:

    https://github.com/sean-jc/linux.git

Unless you hear otherwise, it will make its way to kvm/queue "soon".

Note, the commit IDs are not guaranteed to be stable.

Re: [PATCH] kvm: x86: mmu: fix memoryleak in kvm_mmu_vendor_module_init()

Posted by Huang, Kai 3 years, 7 months ago

On Tue, 2022-08-30 at 21:40 +0000, Sean Christopherson wrote:
> On Tue, Aug 23, 2022, Miaohe Lin wrote:
> > When register_shrinker() fails, we forgot to release the percpu counter

> > kvm_total_used_mmu_pages leading to memoryleak. Fix this issue by calling
> > percpu_counter_destroy() when register_shrinker() fails.
> > 
> > Fixes: ab271bd4dfd5 ("x86: kvm: propagate register_shrinker return code")
> > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> > ---
> 
> Pushed to branch `for_paolo/6.1` at:
> 
>     https://github.com/sean-jc/linux.git
> 
> Unless you hear otherwise, it will make its way to kvm/queue "soon".
> 
> Note, the commit IDs are not guaranteed to be stable.

Sorry for late reply.

The commit message has "we".  Should we get rid of it?

-- 
Thanks,
-Kai

Re: [PATCH] kvm: x86: mmu: fix memoryleak in kvm_mmu_vendor_module_init()

Posted by Miaohe Lin 3 years, 7 months ago

On 2022/8/31 6:42, Huang, Kai wrote:
> On Tue, 2022-08-30 at 21:40 +0000, Sean Christopherson wrote:
>> On Tue, Aug 23, 2022, Miaohe Lin wrote:
>>> When register_shrinker() fails, we forgot to release the percpu counter
> 
>>> kvm_total_used_mmu_pages leading to memoryleak. Fix this issue by calling
>>> percpu_counter_destroy() when register_shrinker() fails.
>>>
>>> Fixes: ab271bd4dfd5 ("x86: kvm: propagate register_shrinker return code")
>>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>>> ---
>>
>> Pushed to branch `for_paolo/6.1` at:
>>
>>     https://github.com/sean-jc/linux.git
>>
>> Unless you hear otherwise, it will make its way to kvm/queue "soon".
>>
>> Note, the commit IDs are not guaranteed to be stable.
> 
> Sorry for late reply.
> 
> The commit message has "we".  Should we get rid of it?

Sean has kindly tweaked the commit message. Many thanks both.

BTW: Is it unsuitable to use "we" in the commit message? If so, I will try to not use it
in later patches.

Thanks,
Miaohe Lin

Re: [PATCH] kvm: x86: mmu: fix memoryleak in kvm_mmu_vendor_module_init()

Posted by Sean Christopherson 3 years, 7 months ago

On Tue, Aug 30, 2022, Huang, Kai wrote:
> On Tue, 2022-08-30 at 21:40 +0000, Sean Christopherson wrote:
> > On Tue, Aug 23, 2022, Miaohe Lin wrote:
> > > When register_shrinker() fails, we forgot to release the percpu counter
> 
> > > kvm_total_used_mmu_pages leading to memoryleak. Fix this issue by calling
> > > percpu_counter_destroy() when register_shrinker() fails.
> > > 
> > > Fixes: ab271bd4dfd5 ("x86: kvm: propagate register_shrinker return code")
> > > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> > > ---
> > 
> > Pushed to branch `for_paolo/6.1` at:
> > 
> >     https://github.com/sean-jc/linux.git
> > 
> > Unless you hear otherwise, it will make its way to kvm/queue "soon".
> > 
> > Note, the commit IDs are not guaranteed to be stable.
> 
> Sorry for late reply.
> 
> The commit message has "we".  Should we get rid of it?

Avoiding pronouns is obviously my preference, but sometimes even I set aside my
crusade to move things along :-)

RE: [PATCH] kvm: x86: mmu: fix memoryleak in kvm_mmu_vendor_module_init()

Posted by Huang, Kai 3 years, 7 months ago

> On Tue, Aug 30, 2022, Huang, Kai wrote:
> > On Tue, 2022-08-30 at 21:40 +0000, Sean Christopherson wrote:
> > > On Tue, Aug 23, 2022, Miaohe Lin wrote:
> > > > When register_shrinker() fails, we forgot to release the percpu
> > > > counter
> >
> > > > kvm_total_used_mmu_pages leading to memoryleak. Fix this issue by
> > > > calling
> > > > percpu_counter_destroy() when register_shrinker() fails.
> > > >
> > > > Fixes: ab271bd4dfd5 ("x86: kvm: propagate register_shrinker return
> > > > code")
> > > > Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> > > > ---
> > >
> > > Pushed to branch `for_paolo/6.1` at:
> > >
> > >     https://github.com/sean-jc/linux.git
> > >
> > > Unless you hear otherwise, it will make its way to kvm/queue "soon".
> > >
> > > Note, the commit IDs are not guaranteed to be stable.
> >
> > Sorry for late reply.
> >
> > The commit message has "we".  Should we get rid of it?
> 
> Avoiding pronouns is obviously my preference, but sometimes even I set aside
> my crusade to move things along :-)

Yeah sure.  I was replying just in case you didn't notice it :)

Re: [PATCH] kvm: x86: mmu: fix memoryleak in kvm_mmu_vendor_module_init()

Posted by Miaohe Lin 3 years, 7 months ago

On 2022/8/31 9:07, Huang, Kai wrote:
>> On Tue, Aug 30, 2022, Huang, Kai wrote:
>>> On Tue, 2022-08-30 at 21:40 +0000, Sean Christopherson wrote:
>>>> On Tue, Aug 23, 2022, Miaohe Lin wrote:
>>>>> When register_shrinker() fails, we forgot to release the percpu
>>>>> counter
>>>
>>>>> kvm_total_used_mmu_pages leading to memoryleak. Fix this issue by
>>>>> calling
>>>>> percpu_counter_destroy() when register_shrinker() fails.
>>>>>
>>>>> Fixes: ab271bd4dfd5 ("x86: kvm: propagate register_shrinker return
>>>>> code")
>>>>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>>>>> ---
>>>>
>>>> Pushed to branch `for_paolo/6.1` at:
>>>>
>>>>     https://github.com/sean-jc/linux.git
>>>>
>>>> Unless you hear otherwise, it will make its way to kvm/queue "soon".
>>>>
>>>> Note, the commit IDs are not guaranteed to be stable.
>>>
>>> Sorry for late reply.
>>>
>>> The commit message has "we".  Should we get rid of it?
>>
>> Avoiding pronouns is obviously my preference, but sometimes even I set aside
>> my crusade to move things along :-)
> 
> Yeah sure.  I was replying just in case you didn't notice it :)

Please ignore my previous question in another thread. I just saw this patch. ;)

Many thanks.

> 
> .
>

Re: [PATCH] kvm: x86: mmu: fix memoryleak in kvm_mmu_vendor_module_init()

Posted by Sean Christopherson 3 years, 7 months ago

Nit for future patches, please use

  KVM: x86/mmu:

for the scope.

On Tue, Aug 23, 2022, Miaohe Lin wrote:
> When register_shrinker() fails, we forgot to release the percpu counter
> kvm_total_used_mmu_pages leading to memoryleak. Fix this issue by calling
> percpu_counter_destroy() when register_shrinker() fails.
> 
> Fixes: ab271bd4dfd5 ("x86: kvm: propagate register_shrinker return code")
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---

Reviewed-by: Sean Christopherson <seanjc@google.com>

Re: [PATCH] kvm: x86: mmu: fix memoryleak in kvm_mmu_vendor_module_init()

Posted by Miaohe Lin 3 years, 7 months ago

On 2022/8/24 23:44, Sean Christopherson wrote:
> Nit for future patches, please use
> 
>   KVM: x86/mmu:
> 
> for the scope.

Will take care of it.

> 
> On Tue, Aug 23, 2022, Miaohe Lin wrote:
>> When register_shrinker() fails, we forgot to release the percpu counter
>> kvm_total_used_mmu_pages leading to memoryleak. Fix this issue by calling
>> percpu_counter_destroy() when register_shrinker() fails.
>>
>> Fixes: ab271bd4dfd5 ("x86: kvm: propagate register_shrinker return code")
>> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
>> ---
> 
> Reviewed-by: Sean Christopherson <seanjc@google.com>

Many thanks for your review.

Thanks,
Miaohe Lin