| Message ID | 20241112232253.3379178-3-dionnaglaze@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v6,1/8] KVM: SVM: Fix gctx page leak on invalid inputs | expand |
On 11/12/24 17:22, Dionna Glaze wrote: > Failure to allocate should not return -ENOTTY. > Command failure has multiple possible error modes. > > Fixes: 136d8bc931c8 ("KVM: SEV: Add KVM_SEV_SNP_LAUNCH_START command") > > CC: Sean Christopherson <seanjc@google.com> > CC: Paolo Bonzini <pbonzini@redhat.com> > CC: Thomas Gleixner <tglx@linutronix.de> > CC: Ingo Molnar <mingo@redhat.com> > CC: Borislav Petkov <bp@alien8.de> > CC: Dave Hansen <dave.hansen@linux.intel.com> > CC: Ashish Kalra <ashish.kalra@amd.com> > CC: Tom Lendacky <thomas.lendacky@amd.com> > CC: John Allen <john.allen@amd.com> > CC: Herbert Xu <herbert@gondor.apana.org.au> > CC: "David S. Miller" <davem@davemloft.net> > CC: Michael Roth <michael.roth@amd.com> > CC: Luis Chamberlain <mcgrof@kernel.org> > CC: Russ Weight <russ.weight@linux.dev> > CC: Danilo Krummrich <dakr@redhat.com> > CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > CC: "Rafael J. Wysocki" <rafael@kernel.org> > CC: Tianfei zhang <tianfei.zhang@intel.com> > CC: Alexey Kardashevskiy <aik@amd.com> > CC: stable@vger.kernel.org > > Signed-off-by: Dionna Glaze <dionnaglaze@google.com> > --- > arch/x86/kvm/svm/sev.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > index 357906375ec59..d0e0152aefb32 100644 > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -2171,7 +2171,7 @@ static void *snp_context_create(struct kvm *kvm, struct kvm_sev_cmd *argp) > /* Allocate memory for context page */ > context = snp_alloc_firmware_page(GFP_KERNEL_ACCOUNT); > if (!context) > - return NULL; > + return ERR_PTR(-ENOMEM); > > data.address = __psp_pa(context); > rc = __sev_issue_cmd(argp->sev_fd, SEV_CMD_SNP_GCTX_CREATE, &data, &argp->error); > @@ -2179,7 +2179,7 @@ static void *snp_context_create(struct kvm *kvm, struct kvm_sev_cmd *argp) > pr_warn("Failed to create SEV-SNP context, rc %d fw_error %d", > rc, argp->error); > snp_free_firmware_page(context); > - return NULL; > + return ERR_PTR(rc); > } > > return context; > @@ -2227,8 +2227,8 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp) > return -EINVAL; > > sev->snp_context = snp_context_create(kvm, argp); Since you can now get an error value set into sev->snp_context, a lot of the NULL checks will be altered. You should create a local variable to hold the returned value of snp_context_create() and only set sev->snp_context if not an error. Thanks, Tom > - if (!sev->snp_context) > - return -ENOTTY; > + if (IS_ERR(sev->snp_context)) > + return PTR_ERR(sev->snp_context); > > start.gctx_paddr = __psp_pa(sev->snp_context); > start.policy = params.policy;
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index 357906375ec59..d0e0152aefb32 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -2171,7 +2171,7 @@ static void *snp_context_create(struct kvm *kvm, struct kvm_sev_cmd *argp) /* Allocate memory for context page */ context = snp_alloc_firmware_page(GFP_KERNEL_ACCOUNT); if (!context) - return NULL; + return ERR_PTR(-ENOMEM); data.address = __psp_pa(context); rc = __sev_issue_cmd(argp->sev_fd, SEV_CMD_SNP_GCTX_CREATE, &data, &argp->error); @@ -2179,7 +2179,7 @@ static void *snp_context_create(struct kvm *kvm, struct kvm_sev_cmd *argp) pr_warn("Failed to create SEV-SNP context, rc %d fw_error %d", rc, argp->error); snp_free_firmware_page(context); - return NULL; + return ERR_PTR(rc); } return context; @@ -2227,8 +2227,8 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp) return -EINVAL; sev->snp_context = snp_context_create(kvm, argp); - if (!sev->snp_context) - return -ENOTTY; + if (IS_ERR(sev->snp_context)) + return PTR_ERR(sev->snp_context); start.gctx_paddr = __psp_pa(sev->snp_context); start.policy = params.policy;
Failure to allocate should not return -ENOTTY. Command failure has multiple possible error modes. Fixes: 136d8bc931c8 ("KVM: SEV: Add KVM_SEV_SNP_LAUNCH_START command") CC: Sean Christopherson <seanjc@google.com> CC: Paolo Bonzini <pbonzini@redhat.com> CC: Thomas Gleixner <tglx@linutronix.de> CC: Ingo Molnar <mingo@redhat.com> CC: Borislav Petkov <bp@alien8.de> CC: Dave Hansen <dave.hansen@linux.intel.com> CC: Ashish Kalra <ashish.kalra@amd.com> CC: Tom Lendacky <thomas.lendacky@amd.com> CC: John Allen <john.allen@amd.com> CC: Herbert Xu <herbert@gondor.apana.org.au> CC: "David S. Miller" <davem@davemloft.net> CC: Michael Roth <michael.roth@amd.com> CC: Luis Chamberlain <mcgrof@kernel.org> CC: Russ Weight <russ.weight@linux.dev> CC: Danilo Krummrich <dakr@redhat.com> CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org> CC: "Rafael J. Wysocki" <rafael@kernel.org> CC: Tianfei zhang <tianfei.zhang@intel.com> CC: Alexey Kardashevskiy <aik@amd.com> CC: stable@vger.kernel.org Signed-off-by: Dionna Glaze <dionnaglaze@google.com> --- arch/x86/kvm/svm/sev.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)