* Re: [PATCH] KVM: SVM: Fix potential memory leak in svm_cpu_init()
@ 2020-01-03 2:19 linmiaohe
0 siblings, 0 replies; 4+ messages in thread
From: linmiaohe @ 2020-01-03 2:19 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: liran.alon, kvm, linux-kernel, x86, pbonzini, rkrcmar,
sean.j.christopherson, wanpengli, jmattson, joro, tglx, mingo,
bp, hpa
Vitaly writes:
>> From: Miaohe Lin <linmiaohe@huawei.com>
>> 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 *),
>> 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.
The var r is really unnecessary and we should clean it up. Thanks for your good suggest. I would send a patch v2 soon.
>
>Anyways, your patch seems to be correct, so:
>
>Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Thanks for your review.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: SVM: Fix potential memory leak in svm_cpu_init()
2020-01-02 2:20 linmiaohe
2020-01-02 13:22 ` Liran Alon
@ 2020-01-02 13:30 ` Vitaly Kuznetsov
1 sibling, 0 replies; 4+ messages in thread
From: Vitaly Kuznetsov @ 2020-01-02 13:30 UTC (permalink / raw)
To: linmiaohe
Cc: liran.alon, kvm, linux-kernel, x86, pbonzini, rkrcmar,
sean.j.christopherson, wanpengli, jmattson, joro, tglx, mingo,
bp, hpa
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: SVM: Fix potential memory leak in svm_cpu_init()
2020-01-02 2:20 linmiaohe
@ 2020-01-02 13:22 ` Liran Alon
2020-01-02 13:30 ` Vitaly Kuznetsov
1 sibling, 0 replies; 4+ messages in thread
From: Liran Alon @ 2020-01-02 13:22 UTC (permalink / raw)
To: linmiaohe
Cc: Paolo Bonzini, rkrcmar, sean.j.christopherson, vkuznets,
wanpengli, jmattson, joro, tglx, mingo, bp, hpa, kvm,
linux-kernel, x86
> On 2 Jan 2020, at 4:20, linmiaohe <linmiaohe@huawei.com> wrote:
>
> 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>
Reviewed-by: Liran Alon <liran.alon@oracle.com>
-Liran
> ---
> 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;
> @@ -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;
>
> --
> 2.19.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] KVM: SVM: Fix potential memory leak in svm_cpu_init()
@ 2020-01-02 2:20 linmiaohe
2020-01-02 13:22 ` Liran Alon
2020-01-02 13:30 ` Vitaly Kuznetsov
0 siblings, 2 replies; 4+ messages in thread
From: linmiaohe @ 2020-01-02 2:20 UTC (permalink / raw)
To: pbonzini, rkrcmar, sean.j.christopherson, vkuznets, wanpengli,
jmattson, joro, tglx, mingo, bp, hpa
Cc: linmiaohe, liran.alon, kvm, linux-kernel, x86
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;
@@ -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;
--
2.19.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-01-03 2:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-03 2:19 [PATCH] KVM: SVM: Fix potential memory leak in svm_cpu_init() linmiaohe
-- strict thread matches above, loose matches on Subject: below --
2020-01-02 2:20 linmiaohe
2020-01-02 13:22 ` Liran Alon
2020-01-02 13:30 ` Vitaly Kuznetsov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).