lkml.org 
[lkml]   [2020]   [Jan]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH] KVM: SVM: Fix potential memory leak in svm_cpu_init()
Date
linmiaohe <linmiaohe@huawei.com> writes:

> From: Miaohe Lin <linmiaohe@huawei.com>
>
> When kmalloc memory for sd->sev_vmcbs failed, we forget to free the page
> held by sd->save_area.
>
> Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
> ---
> arch/x86/kvm/svm.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 8f1b715dfde8..89eb382e8580 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -1012,7 +1012,7 @@ static int svm_cpu_init(int cpu)
> r = -ENOMEM;
> sd->save_area = alloc_page(GFP_KERNEL);
> if (!sd->save_area)
> - goto err_1;
> + goto free_cpu_data;
>
> if (svm_sev_enabled()) {
> r = -ENOMEM;

Not your fault but this assignment to 'r' seem to be redundant: it is
already set to '-ENOMEM' above, but this is also not perfect as ...

> @@ -1020,14 +1020,16 @@ static int svm_cpu_init(int cpu)
> sizeof(void *),
> GFP_KERNEL);
> if (!sd->sev_vmcbs)
> - goto err_1;
> + goto free_save_area;
> }
>
> per_cpu(svm_data, cpu) = sd;
>
> return 0;
>
> -err_1:
> +free_save_area:
> + __free_page(sd->save_area);
> +free_cpu_data:
> kfree(sd);
> return r;

... '-ENOMEM' is actually the only possible outcome here. In case you'll
be re-submitting, I'd suggest we drop 'r' entirely and just reture
-ENOMEM here.

Anyways, your patch seems to be correct, so:

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>

--
Vitaly

\
 
 \ /
  Last update: 2020-01-02 14:31    [W:0.073 / U:14.312 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site