[PATCH v4 1/6] kvm: svm: Fix gctx page leak on invalid inputs

[PATCH v4 1/6] kvm: svm: Fix gctx page leak on invalid inputs

[Dionna Glaze]

Ensure that snp gctx page allocation is adequately deallocated on
failure during snp_launch_start.

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>

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 714c517dd4b72..f6e96ec0a5caa 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -2212,10 +2212,6 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	if (sev->snp_context)
 		return -EINVAL;
 
-	sev->snp_context = snp_context_create(kvm, argp);
-	if (!sev->snp_context)
-		return -ENOTTY;
-
 	if (params.flags)
 		return -EINVAL;
 
@@ -2230,6 +2226,10 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
 	if (params.policy & SNP_POLICY_MASK_SINGLE_SOCKET)
 		return -EINVAL;
 
+	sev->snp_context = snp_context_create(kvm, argp);
+	if (!sev->snp_context)
+		return -ENOTTY;
+
 	start.gctx_paddr = __psp_pa(sev->snp_context);
 	start.policy = params.policy;
 	memcpy(start.gosvw, params.gosvw, sizeof(params.gosvw));
-- 
2.47.0.199.ga7371fff76-goog

[PATCH v4 6/6] KVM: SVM: Delay legacy platform initialization on SNP

[Dionna Glaze]

When no SEV or SEV-ES guests are active, then the firmware can be
updated while (SEV-SNP) VM guests are active.

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>

Co-developed-by: Ashish Kalra <ashish.kalra@amd.com>
Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
Reviewed-by: Ashish Kalra <ashish.kalra@amd.com>
Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
---
 arch/x86/kvm/svm/sev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index f6e96ec0a5caa..3c2079ba7c76f 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -444,7 +444,7 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
 	if (ret)
 		goto e_no_asid;
 
-	init_args.probe = false;
+	init_args.probe = vm_type != KVM_X86_SEV_VM && vm_type != KVM_X86_SEV_ES_VM;
 	ret = sev_platform_init(&init_args);
 	if (ret)
 		goto e_free;
-- 
2.47.0.199.ga7371fff76-goog

Re: [PATCH v4 1/6] kvm: svm: Fix gctx page leak on invalid inputs

[Tom Lendacky]

On 11/4/24 19:05, Dionna Glaze wrote:
> Ensure that snp gctx page allocation is adequately deallocated on
> failure during snp_launch_start.
> 
> 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>
> 
> Signed-off-by: Dionna Glaze <dionnaglaze@google.com>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.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 714c517dd4b72..f6e96ec0a5caa 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2212,10 +2212,6 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  	if (sev->snp_context)
>  		return -EINVAL;
>  
> -	sev->snp_context = snp_context_create(kvm, argp);
> -	if (!sev->snp_context)
> -		return -ENOTTY;
> -
>  	if (params.flags)
>  		return -EINVAL;
>  
> @@ -2230,6 +2226,10 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  	if (params.policy & SNP_POLICY_MASK_SINGLE_SOCKET)
>  		return -EINVAL;
>  
> +	sev->snp_context = snp_context_create(kvm, argp);
> +	if (!sev->snp_context)
> +		return -ENOTTY;
> +
>  	start.gctx_paddr = __psp_pa(sev->snp_context);
>  	start.policy = params.policy;
>  	memcpy(start.gosvw, params.gosvw, sizeof(params.gosvw));

Re: [PATCH v4 1/6] kvm: svm: Fix gctx page leak on invalid inputs

[Sean Christopherson]

KVM: SVM:

In the future, please post bug fixes separately from new features series, especially
when the fix has very little to do with the rest of the series (AFAICT, this has
no relation whatsoever beyond SNP).

On Tue, Nov 05, 2024, Dionna Glaze wrote:
> Ensure that snp gctx page allocation is adequately deallocated on
> failure during snp_launch_start.
> 
> Fixes: 136d8bc931c8 ("KVM: SEV: Add KVM_SEV_SNP_LAUNCH_START command")

This needs

Cc: stable@vger.kernel.org

especially if it doesn't get into 6.12.

> 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>
> 
> Signed-off-by: Dionna Glaze <dionnaglaze@google.com>

Acked-by: Sean Christopherson <seanjc@google.com>

Paolo, do you want to grab this one for 6.12 too?

> ---
>  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 714c517dd4b72..f6e96ec0a5caa 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2212,10 +2212,6 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  	if (sev->snp_context)
>  		return -EINVAL;
>  
> -	sev->snp_context = snp_context_create(kvm, argp);
> -	if (!sev->snp_context)
> -		return -ENOTTY;
> -
>  	if (params.flags)
>  		return -EINVAL;
>  
> @@ -2230,6 +2226,10 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
>  	if (params.policy & SNP_POLICY_MASK_SINGLE_SOCKET)
>  		return -EINVAL;
>  
> +	sev->snp_context = snp_context_create(kvm, argp);
> +	if (!sev->snp_context)
> +		return -ENOTTY;

Related to this fix, the return values from snp_context_create() are garbage.  It
should return ERR_PTR(), not NULL.  -ENOTTY on an OOM scenatio is blatantly wrong,
as -ENOTTY on any SEV_CMD_SNP_GCTX_CREATE failure is too.

> +
>  	start.gctx_paddr = __psp_pa(sev->snp_context);
>  	start.policy = params.policy;
>  	memcpy(start.gosvw, params.gosvw, sizeof(params.gosvw));
> -- 
> 2.47.0.199.ga7371fff76-goog
> 

Re: [PATCH v4 6/6] KVM: SVM: Delay legacy platform initialization on SNP

[Tom Lendacky]

On 11/4/24 19:05, Dionna Glaze wrote:
> When no SEV or SEV-ES guests are active, then the firmware can be
> updated while (SEV-SNP) VM guests are active.
> 
> 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>
> 
> Co-developed-by: Ashish Kalra <ashish.kalra@amd.com>
> Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> Reviewed-by: Ashish Kalra <ashish.kalra@amd.com>
> Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
> ---
>  arch/x86/kvm/svm/sev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index f6e96ec0a5caa..3c2079ba7c76f 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -444,7 +444,7 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
>  	if (ret)
>  		goto e_no_asid;
>  
> -	init_args.probe = false;
> +	init_args.probe = vm_type != KVM_X86_SEV_VM && vm_type != KVM_X86_SEV_ES_VM;

Add a comment above the setting of probe to indicate the intent of doing
this. Otherwise you have to go to the ccp code to understand what is
happening.

Thanks,
Tom

>  	ret = sev_platform_init(&init_args);
>  	if (ret)
>  		goto e_free;

Re: [PATCH v4 1/6] kvm: svm: Fix gctx page leak on invalid inputs

[Dionna Amalie Glaze]

On Wed, Nov 6, 2024 at 6:34 AM Sean Christopherson <seanjc@google.com> wrote:
>
> KVM: SVM:
>
> In the future, please post bug fixes separately from new features series, especially
> when the fix has very little to do with the rest of the series (AFAICT, this has
> no relation whatsoever beyond SNP).
>

Understood. Are dependent series best shared through links to a dev
branch containing all patches?

> On Tue, Nov 05, 2024, Dionna Glaze wrote:
> > Ensure that snp gctx page allocation is adequately deallocated on
> > failure during snp_launch_start.
> >
> > Fixes: 136d8bc931c8 ("KVM: SEV: Add KVM_SEV_SNP_LAUNCH_START command")
>
> This needs
>
> Cc: stable@vger.kernel.org
>
> especially if it doesn't get into 6.12.
>
> > 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>
> >
> > Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
>
> Acked-by: Sean Christopherson <seanjc@google.com>
>
> Paolo, do you want to grab this one for 6.12 too?
>
> > ---
> >  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 714c517dd4b72..f6e96ec0a5caa 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -2212,10 +2212,6 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
> >       if (sev->snp_context)
> >               return -EINVAL;
> >
> > -     sev->snp_context = snp_context_create(kvm, argp);
> > -     if (!sev->snp_context)
> > -             return -ENOTTY;
> > -
> >       if (params.flags)
> >               return -EINVAL;
> >
> > @@ -2230,6 +2226,10 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
> >       if (params.policy & SNP_POLICY_MASK_SINGLE_SOCKET)
> >               return -EINVAL;
> >
> > +     sev->snp_context = snp_context_create(kvm, argp);
> > +     if (!sev->snp_context)
> > +             return -ENOTTY;
>
> Related to this fix, the return values from snp_context_create() are garbage.  It
> should return ERR_PTR(), not NULL.  -ENOTTY on an OOM scenatio is blatantly wrong,
> as -ENOTTY on any SEV_CMD_SNP_GCTX_CREATE failure is too.
>

I caught this too. I'll be changing that behavior with the new gctx
management API from ccp in v5, i.e.,

/**
 * sev_snp_create_context - allocates an SNP context firmware page
 *
 * Associates the created context with the ASID that an activation
 * call after SNP_LAUNCH_START will commit. The association is needed
 * to track active GCTX pages to refresh during firmware hotload.
 *
 * @asid:    The ASID allocated to the caller that will be used in a
subsequent SNP_ACTIVATE.
 * @psp_ret: sev command return code.
 *
 * Returns:
 * A pointer to the SNP context page, or an ERR_PTR of
 * -%ENODEV    if the PSP device is not available
 * -%ENOTSUPP  if PSP device does not support SEV
 * -%ETIMEDOUT if the SEV command timed out
 * -%EIO       if PSP device returned a non-zero return code
 */
void *sev_snp_create_context(int asid, int *psp_ret);

/**
 * sev_snp_activate_asid - issues SNP_ACTIVATE for the asid and
associated GCTX page.
 *
 * @asid:    The ASID to activate.
 * @psp_ret: sev command return code.
 *
 * Returns:
 * 0 if the SEV device successfully processed the command
 * -%ENODEV    if the PSP device is not available
 * -%ENOTSUPP  if PSP device does not support SEV
 * -%ETIMEDOUT if the SEV command timed out
 * -%EIO       if PSP device returned a non-zero return code
 */
int sev_snp_activate_asid(int asid, int *psp_ret);

/**
 * sev_snp_guest_decommission - issues SNP_DECOMMISSION for an asid's
GCTX page and frees it.
 *
 * @asid:    The ASID to activate.
 * @psp_ret: sev command return code.
 *
 * Returns:
 * 0 if the SEV device successfully processed the command
 * -%ENODEV    if the PSP device is not available
 * -%ENOTSUPP  if PSP device does not support SEV
 * -%ETIMEDOUT if the SEV command timed out
 * -%EIO       if PSP device returned a non-zero return code
 */
int sev_snp_guest_decommission(int asid, int *psp_ret);

> > +
> >       start.gctx_paddr = __psp_pa(sev->snp_context);
> >       start.policy = params.policy;
> >       memcpy(start.gosvw, params.gosvw, sizeof(params.gosvw));
> > --
> > 2.47.0.199.ga7371fff76-goog
> >



-- 
-Dionna Glaze, PhD, CISSP, CCSP (she/her)

Re: [PATCH v4 1/6] kvm: svm: Fix gctx page leak on invalid inputs

[Sean Christopherson]

On Wed, Nov 06, 2024, Dionna Amalie Glaze wrote:
> On Wed, Nov 6, 2024 at 6:34 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > KVM: SVM:
> >
> > In the future, please post bug fixes separately from new features series, especially
> > when the fix has very little to do with the rest of the series (AFAICT, this has
> > no relation whatsoever beyond SNP).
> >
> 
> Understood. Are dependent series best shared through links to a dev
> branch containing all patches?

I don't follow.  There is no dependency here.  If this series were moving
snp_context_create() out of KVM, then that would be a different story, i.e. then
it _would_ be appropriate to include the fix at the front of the series.

If you end up a situation where a dependency is created after the initial posting,
e.g. you post this fix, then later decide to move snp_context_create() out of KVM,
then simply call that out in the cover letter and provide a lore.kernel.org link.

For large scale dependencies, e.g. multi-patch series that build on other multi-patch
series, then providing a link to a git branch is helpful.  But for something this
trivial, it's overkill.

> > > ---
> > >  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 714c517dd4b72..f6e96ec0a5caa 100644
> > > --- a/arch/x86/kvm/svm/sev.c
> > > +++ b/arch/x86/kvm/svm/sev.c
> > > @@ -2212,10 +2212,6 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
> > >       if (sev->snp_context)
> > >               return -EINVAL;
> > >
> > > -     sev->snp_context = snp_context_create(kvm, argp);
> > > -     if (!sev->snp_context)
> > > -             return -ENOTTY;
> > > -
> > >       if (params.flags)
> > >               return -EINVAL;
> > >
> > > @@ -2230,6 +2226,10 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
> > >       if (params.policy & SNP_POLICY_MASK_SINGLE_SOCKET)
> > >               return -EINVAL;
> > >
> > > +     sev->snp_context = snp_context_create(kvm, argp);
> > > +     if (!sev->snp_context)
> > > +             return -ENOTTY;
> >
> > Related to this fix, the return values from snp_context_create() are garbage.  It
> > should return ERR_PTR(), not NULL.  -ENOTTY on an OOM scenatio is blatantly wrong,
> > as -ENOTTY on any SEV_CMD_SNP_GCTX_CREATE failure is too.
> 
> I caught this too. I'll be changing that behavior with the new gctx
> management API from ccp in v5, i.e.,

Please fix the KVM flaws before moving code out of KVM, i.e. ensure the flaws are
cleaned up even if we opt not to go the route of moving the code out of KVM (which
I assume is what you plan to do with sev_snp_create_context()).

Re: [PATCH v4 1/6] kvm: svm: Fix gctx page leak on invalid inputs

[Paolo Bonzini]

On 11/6/24 15:29, Tom Lendacky wrote:
> On 11/4/24 19:05, Dionna Glaze wrote:
>> Ensure that snp gctx page allocation is adequately deallocated on
>> failure during snp_launch_start.
>>
>> 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>
>>
>> Signed-off-by: Dionna Glaze <dionnaglaze@google.com>
> 
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.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 714c517dd4b72..f6e96ec0a5caa 100644
>> --- a/arch/x86/kvm/svm/sev.c
>> +++ b/arch/x86/kvm/svm/sev.c
>> @@ -2212,10 +2212,6 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
>>   	if (sev->snp_context)
>>   		return -EINVAL;
>>   
>> -	sev->snp_context = snp_context_create(kvm, argp);
>> -	if (!sev->snp_context)
>> -		return -ENOTTY;
>> -
>>   	if (params.flags)
>>   		return -EINVAL;
>>   
>> @@ -2230,6 +2226,10 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
>>   	if (params.policy & SNP_POLICY_MASK_SINGLE_SOCKET)
>>   		return -EINVAL;
>>   
>> +	sev->snp_context = snp_context_create(kvm, argp);
>> +	if (!sev->snp_context)
>> +		return -ENOTTY;
>> +
>>   	start.gctx_paddr = __psp_pa(sev->snp_context);
>>   	start.policy = params.policy;
>>   	memcpy(start.gosvw, params.gosvw, sizeof(params.gosvw));
> 

Applied, thanks.

Paolo