| Message ID | 1577931640-29420-1-git-send-email-linmiaohe@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: SVM: Fix potential memory leak in svm_cpu_init() | expand |
> 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 >
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>
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;