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

[Sean Christopherson <seanjc@google.com>]

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
>