| Message ID | 1552375733-24985-1-git-send-email-lirongqing@baidu.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: fix error handling in svm_cpu_init | expand |
On 3/12/19 2:28 AM, Li RongQing wrote: > 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> > --- > arch/x86/kvm/svm.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > Reviewed-by: Brijesh Singh <brijesh.singh@amd.com> thanks
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> > --- > arch/x86/kvm/svm.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index f13a3a24d360..1787a484d21c 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -1006,17 +1006,18 @@ static int svm_cpu_init(int cpu) > sizeof(void *), > GFP_KERNEL); > if (!sd->sev_vmcbs) > - goto err_1; > + goto err_2; Generally, 'err_1', 'err_2' are not good names for labels: when function becomes longer and you need to scroll it becomes hard to see if the cleanup path is correct ("Which one do I need here? err_5 or err_6? What are they doing"?) I'd suggest to use something like "err_free_saved_area" and "err_free_sd" here instead. > } > > per_cpu(svm_data, cpu) = sd; > > return 0; > > +err_2: > + __free_page(sd->save_area); > err_1: > kfree(sd); > return r; > - > } > > static bool valid_msr_intercept(u32 index)
> -----邮件原件----- > 发件人: Vitaly Kuznetsov [mailto:vkuznets@redhat.com] > 发送时间: 2019年3月14日 21:41 > 收件人: Li,Rongqing <lirongqing@baidu.com>; x86@kernel.org; > kvm@vger.kernel.org > 主题: Re: [PATCH] KVM: fix error handling in svm_cpu_init > > 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> > > --- > > arch/x86/kvm/svm.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index > > f13a3a24d360..1787a484d21c 100644 > > --- a/arch/x86/kvm/svm.c > > +++ b/arch/x86/kvm/svm.c > > @@ -1006,17 +1006,18 @@ static int svm_cpu_init(int cpu) > > sizeof(void *), > > GFP_KERNEL); > > if (!sd->sev_vmcbs) > > - goto err_1; > > + goto err_2; > > Generally, 'err_1', 'err_2' are not good names for labels: when function > becomes longer and you need to scroll it becomes hard to see if the cleanup > path is correct ("Which one do I need here? err_5 or err_6? What are they > doing"?) > > I'd suggest to use something like "err_free_saved_area" and "err_free_sd" > here instead. > OK, I will send v2 Thanks -RongQing > > } > > > > per_cpu(svm_data, cpu) = sd; > > > > return 0; > > > > +err_2: > > + __free_page(sd->save_area); > > err_1: > > kfree(sd); > > return r; > > - > > } > > > > static bool valid_msr_intercept(u32 index) > > -- > Vitaly
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index f13a3a24d360..1787a484d21c 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -1006,17 +1006,18 @@ static int svm_cpu_init(int cpu) sizeof(void *), GFP_KERNEL); if (!sd->sev_vmcbs) - goto err_1; + goto err_2; } per_cpu(svm_data, cpu) = sd; return 0; +err_2: + __free_page(sd->save_area); err_1: kfree(sd); return r; - } static bool valid_msr_intercept(u32 index)
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> --- arch/x86/kvm/svm.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)