Re: [PATCH v2 3/4] crypto/ccp: Do not initialize SNP for ioctl(SNP_VLEK_LOAD)

[Tom Lendacky]

On 5/4/26 11:51, Tycho Andersen wrote:
> From: "Tycho Andersen (AMD)" <tycho@xxxxxxxxxx>
> 
> Sashiko notes:
> 
>> if SEV initialization fails and KVM is actively running normal VMs, could a
>> userspace process trigger this code path via /dev/sev ioctls (e.g.,
>> SEV_PDH_GEN) and zero out MSR_VM_HSAVE_PA globally? Would the next VMRUN
>> execution for an active VM trigger a general protection fault and crash the
>> host?
> 
> The SEV firmware docs for SNP_VLEK_LOAD note:
> 
>> On SNP_SHUTDOWN, the VLEK is deleted.
> 
> That is, the initialization/shutdown wrapper here is pointless, because the
> firmware immediately throws away the key anyway. Instead, refuse to do
> anything if SNP has not been previously initialized.
> 
> This is an ABI break: before, this was a no-op and almost certainly a
> mistake by userspace, and now it returns -ENODEV. ABI compatibility could be
> maintained here by simply returning 0 in the check instead.
> 
> Fixes: ceac7fb89e8d ("crypto: ccp - Ensure implicit SEV/SNP init and shutdown in ioctls")
> Reported-by: Sashiko
> Assisted-by: Gemini:gemini-3.1-pro-preview
> Link: https://sashiko.dev/#/patchset/20260324161301.1353976-1-tycho%40kernel.org
> CC: <stable@xxxxxxxxxxxxxxx>
> Signed-off-by: Tycho Andersen (AMD) <tycho@xxxxxxxxxx>

Reviewed-by: Tom Lendacky <thomas.lendacky@xxxxxxx>

> ---
>  drivers/crypto/ccp/sev-dev.c | 17 ++++-------------
>  1 file changed, 4 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
> index 572f06368d4b..ad6c2525a305 100644
> --- a/drivers/crypto/ccp/sev-dev.c
> +++ b/drivers/crypto/ccp/sev-dev.c
> @@ -2481,9 +2481,8 @@ static int sev_ioctl_do_snp_vlek_load(struct sev_issue_cmd *argp, bool writable)
>  {
>  	struct sev_device *sev = psp_master->sev_data;
>  	struct sev_user_data_snp_vlek_load input;
> -	bool shutdown_required = false;
> -	int ret, error;
>  	void *blob;
> +	int ret;
>  
>  	if (!argp->data)
>  		return -EINVAL;
> @@ -2491,6 +2490,9 @@ static int sev_ioctl_do_snp_vlek_load(struct sev_issue_cmd *argp, bool writable)
>  	if (!writable)
>  		return -EPERM;
>  
> +	if (!sev->snp_initialized)
> +		return -ENODEV;
> +
>  	if (copy_from_user(&input, u64_to_user_ptr(argp->data), sizeof(input)))
>  		return -EFAULT;
>  
> @@ -2504,18 +2506,7 @@ static int sev_ioctl_do_snp_vlek_load(struct sev_issue_cmd *argp, bool writable)
>  
>  	input.vlek_wrapped_address = __psp_pa(blob);
>  
> -	if (!sev->snp_initialized) {
> -		ret = snp_move_to_init_state(argp, &shutdown_required);
> -		if (ret)
> -			goto cleanup;
> -	}
> -
>  	ret = __sev_do_cmd_locked(SEV_CMD_SNP_VLEK_LOAD, &input, &argp->error);
> -
> -	if (shutdown_required)
> -		__sev_snp_shutdown_locked(&error, false);
> -
> -cleanup:
>  	kfree(blob);
>  
>  	return ret;