| Message ID | 20210109004714.1341275-2-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: SVM: Misc SEV cleanups | expand |
On 1/8/21 6:47 PM, Sean Christopherson wrote: > Free sev_asid_bitmap if the reclaim bitmap allocation fails, othwerise > it will be leaked as sev_hardware_teardown() frees the bitmaps if and > only if SEV is fully enabled (which obviously isn't the case if SEV > setup fails). The svm_sev_enabled() function is only based on CONFIG_KVM_AMD_SEV and max_sev_asid. So sev_hardware_teardown() should still free everything if it was allocated since we never change max_sev_asid, no? Thanks, Tom > > Fixes: 33af3a7ef9e6 ("KVM: SVM: Reduce WBINVD/DF_FLUSH invocations") > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Cc: stable@vger.kernel.org > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/svm/sev.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > index c8ffdbc81709..0eeb6e1b803d 100644 > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -1274,8 +1274,10 @@ void __init sev_hardware_setup(void) > goto out; > > sev_reclaim_asid_bitmap = bitmap_zalloc(max_sev_asid, GFP_KERNEL); > - if (!sev_reclaim_asid_bitmap) > + if (!sev_reclaim_asid_bitmap) { > + bitmap_free(sev_asid_bitmap); > goto out; > + } > > pr_info("SEV supported: %u ASIDs\n", max_sev_asid - min_sev_asid + 1); > sev_supported = true; >
On Mon, Jan 11, 2021, Tom Lendacky wrote: > On 1/8/21 6:47 PM, Sean Christopherson wrote: > > Free sev_asid_bitmap if the reclaim bitmap allocation fails, othwerise > > it will be leaked as sev_hardware_teardown() frees the bitmaps if and > > only if SEV is fully enabled (which obviously isn't the case if SEV > > setup fails). > > The svm_sev_enabled() function is only based on CONFIG_KVM_AMD_SEV and > max_sev_asid. So sev_hardware_teardown() should still free everything if it > was allocated since we never change max_sev_asid, no? Aha! You're correct. This is needed once svm_sev_enabled() is gone, but is not an actual bug in upstream. I created the commit before the long New Years weekend, but stupidly didn't write a changelog. I then forgot that my series effectively introduce the bug and incorrectly moved this patch to the front and treated it as an upstream bug fix when retroactively writing the changelog. Thanks!
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index c8ffdbc81709..0eeb6e1b803d 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -1274,8 +1274,10 @@ void __init sev_hardware_setup(void) goto out; sev_reclaim_asid_bitmap = bitmap_zalloc(max_sev_asid, GFP_KERNEL); - if (!sev_reclaim_asid_bitmap) + if (!sev_reclaim_asid_bitmap) { + bitmap_free(sev_asid_bitmap); goto out; + } pr_info("SEV supported: %u ASIDs\n", max_sev_asid - min_sev_asid + 1); sev_supported = true;
Free sev_asid_bitmap if the reclaim bitmap allocation fails, othwerise it will be leaked as sev_hardware_teardown() frees the bitmaps if and only if SEV is fully enabled (which obviously isn't the case if SEV setup fails). Fixes: 33af3a7ef9e6 ("KVM: SVM: Reduce WBINVD/DF_FLUSH invocations") Cc: Tom Lendacky <thomas.lendacky@amd.com> Cc: stable@vger.kernel.org Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/svm/sev.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)