Re: [PATCH][resend] KVM: fix error handling in svm_cpu_init

Re: [PATCH][resend] KVM: fix error handling in svm_cpu_init

[linmiaohe]

Hi,
Li RongQing <lirongqing@baidu.com> writes:
>
>sd->save_area should be freed in error path
>
>Fixes: 70cd94e60c733 ("KVM: SVM: VMRUN should use associated ASID when SEV is enabled")
>Signed-off-by: Li RongQing <lirongqing@baidu.com>
>Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
>Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>---
> arch/x86/kvm/svm.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)

Oh, it's strange. This is already fixed in my previous patch : [PATCH v2] KVM: SVM: Fix potential memory leak in svm_cpu_init().
And Vitaly and Liran gave me Reviewed-by tags and Paolo queued it one month ago. But I can't found it in master or queue
branch. There might be something wrong. :(

答复: [PATCH][resend] KVM: fix error handling in svm_cpu_init

[Li,Rongqing]

> -----邮件原件-----
> 发件人: linmiaohe [mailto:linmiaohe@huawei.com]
> 发送时间: 2020年2月20日 16:18
> 收件人: Li,Rongqing <lirongqing@baidu.com>; pbonzini@redhat.com;
> vkuznets@redhat.com
> 抄送: Liran Alon <liran.alon@oracle.com>; kvm@vger.kernel.org;
> linux-kernel@vger.kernel.org; x86@kernel.org
> 主题: Re: [PATCH][resend] KVM: fix error handling in svm_cpu_init
> 
> Hi,
> Li RongQing <lirongqing@baidu.com> writes:
> >
> >sd->save_area should be freed in error path
> >
> >Fixes: 70cd94e60c733 ("KVM: SVM: VMRUN should use associated ASID when
> >SEV is enabled")
> >Signed-off-by: Li RongQing <lirongqing@baidu.com>
> >Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
> >Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> >---
> > arch/x86/kvm/svm.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> 
> Oh, it's strange. This is already fixed in my previous patch : [PATCH v2] KVM:
> SVM: Fix potential memory leak in svm_cpu_init().
> And Vitaly and Liran gave me Reviewed-by tags and Paolo queued it one month
> ago. But I can't found it in master or queue branch. There might be something
> wrong. :(


In fact, I send this patch 2019/02/, and get Reviewed-by,  but did not queue

https://patchwork.kernel.org/patch/10853973/

and resend it 2019/07

https://patchwork.kernel.org/patch/11032081/


-Li

Re: [PATCH][resend] KVM: fix error handling in svm_cpu_init

[linmiaohe]

Paolo Bonzini <pbonzini@redhat.com> wrote:
>On 20/02/20 09:40, linmiaohe wrote:
>> Li,Rongqing <lirongqing@baidu.com> writes:
>>>> Hi,
>>>> Li RongQing <lirongqing@baidu.com> writes:
>>>>>
>> 
>> Oh, it's really a pit. And in this case, we can get rid of the var r 
>> as '-ENOMEM' is actually the only possible outcome here, as suggested 
>> by Vitaly, which looks like this: https://lkml.org/lkml/2020/1/15/933
>
>I queued your patch again, sorry to both of you.
>

There are tons of patches every day. It's really hard to escape from forgetting someone. :)
Many thanks for your great work all the time!

Re: [PATCH][resend] KVM: fix error handling in svm_cpu_init

[Paolo Bonzini]

On 20/02/20 09:40, linmiaohe wrote:
> Li,Rongqing <lirongqing@baidu.com> writes:
>>> Hi,
>>> Li RongQing <lirongqing@baidu.com> writes:
>>>>
>>>> sd->save_area should be freed in error path
>>> Oh, it's strange. This is already fixed in my previous patch : [PATCH v2] KVM:
>>> SVM: Fix potential memory leak in svm_cpu_init().
>>> And Vitaly and Liran gave me Reviewed-by tags and Paolo queued it one 
>>> month ago. But I can't found it in master or queue branch. There might 
>>> be something wrong. :(
>>
>> In fact, I send this patch 2019/02/, and get Reviewed-by,  but did not queue
>>
>> https://patchwork.kernel.org/patch/10853973/
>>
>> and resend it 2019/07
>>
>> https://patchwork.kernel.org/patch/11032081/
>>
> 
> Oh, it's really a pit. And in this case, we can get rid of the var r as '-ENOMEM' is actually the only possible outcome here, as
> suggested by Vitaly, which looks like this: https://lkml.org/lkml/2020/1/15/933

I queued your patch again, sorry to both of you.

Paolo

Re: [PATCH][resend] KVM: fix error handling in svm_cpu_init

[linmiaohe]

Li,Rongqing <lirongqing@baidu.com> writes:
>> Hi,
>> Li RongQing <lirongqing@baidu.com> writes:
>> >
>> >sd->save_area should be freed in error path
>> Oh, it's strange. This is already fixed in my previous patch : [PATCH v2] KVM:
>> SVM: Fix potential memory leak in svm_cpu_init().
>> And Vitaly and Liran gave me Reviewed-by tags and Paolo queued it one 
>> month ago. But I can't found it in master or queue branch. There might 
>> be something wrong. :(
>
>In fact, I send this patch 2019/02/, and get Reviewed-by,  but did not queue
>
>https://patchwork.kernel.org/patch/10853973/
>
>and resend it 2019/07
>
>https://patchwork.kernel.org/patch/11032081/
>

Oh, it's really a pit. And in this case, we can get rid of the var r as '-ENOMEM' is actually the only possible outcome here, as
suggested by Vitaly, which looks like this: https://lkml.org/lkml/2020/1/15/933
Thanks. :)

[PATCH][resend] KVM: fix error handling in svm_cpu_init

[Li RongQing]

sd->save_area should be freed in error path

Fixes: 70cd94e60c733 ("KVM: SVM: VMRUN should use associated ASID when SEV is enabled")
Signed-off-by: Li RongQing <lirongqing@baidu.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.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 a3e32d61d60c..b8e948c65f51 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1014,7 +1014,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 err_free_sd;
 
 	if (svm_sev_enabled()) {
 		r = -ENOMEM;
@@ -1022,14 +1022,16 @@ static int svm_cpu_init(int cpu)
 					      sizeof(void *),
 					      GFP_KERNEL);
 		if (!sd->sev_vmcbs)
-			goto err_1;
+			goto err_free_saved_area;
 	}
 
 	per_cpu(svm_data, cpu) = sd;
 
 	return 0;
 
-err_1:
+err_free_saved_area:
+	__free_page(sd->save_area);
+err_free_sd:
 	kfree(sd);
 	return r;
 
-- 
2.16.2

[PATCH][resend] KVM: fix error handling in svm_cpu_init

[Li RongQing]

sd->save_area should be freed in error path

Fixes: 70cd94e60c733 ("KVM: SVM: VMRUN should use associated ASID when SEV is enabled")
Signed-off-by: Li RongQing <lirongqing@baidu.com>
Reviewed-by: Brijesh Singh <brijesh.singh@amd.com>
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.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 48c865a4e5dd..a69eff0aa40b 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -999,7 +999,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 err_free_sd;
 
 	if (svm_sev_enabled()) {
 		r = -ENOMEM;
@@ -1007,14 +1007,16 @@ static int svm_cpu_init(int cpu)
 					      sizeof(void *),
 					      GFP_KERNEL);
 		if (!sd->sev_vmcbs)
-			goto err_1;
+			goto err_free_saved_area;
 	}
 
 	per_cpu(svm_data, cpu) = sd;
 
 	return 0;
 
-err_1:
+err_free_saved_area:
+	__free_page(sd->save_area);
+err_free_sd:
 	kfree(sd);
 	return r;
 
-- 
2.16.2