Re: [PATCH v1 1/4] crypto/ccp: Do not initialize SNP for SEV ioctls

[Tycho Andersen]

On Tue, Apr 28, 2026 at 04:56:36PM -0500, Tom Lendacky wrote:
> On 4/27/26 11:15, 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?
> > 
> > sev_move_to_init_state() is called for ioctls requiring only SEV firmware:
> > SEV_PEK_GEN, SEV_PDH_GEN, SEV_PEK_CSR, SEV_PEK_CERT_IMPORT, and
> > SEV_PDH_CERT_EXPORT. After the firmware command, it does SEV_SHUTDOWN on
> > the SEV firmware. Since these commands do not require SNP to be
> > initialized, skip it by calling __sev_platform_init_locked() which only
> > initializes the SEV firmware. This way SNP is not Initialized at all, and
> > HSAVE_PA is not cleared.
> > 
> > The previous code saved any SEV initialization firmware error to
> > init_args.error and then threw it away and hardcoded the return value of
> > INVALID_PLATFORM_STATE regardless of the real firmware error. This patch
> > changes it to surface the underlying error, which is hopefully both more
> > useful and doesn't cause any problems.
> > 
> > Note that it is still safe to call __sev_firmware_shutdown() directly: it
> > calls __sev_snp_shutdown_locked(), which skips SNP shutdown if SNP was not
> > initialized.
> > 
> > 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
> > Signed-off-by: Tycho Andersen (AMD) <tycho@xxxxxxxxxx>
> 
> I have a similar patch that I hadn't gotten out that added an argument to
> _sev_platform_init_locked() to skip/prevent SNP initialization. I wonder
> if adding something to sev_platform_init_args would be better? This could
> then be expanded to prevent SNP initialization if the KVM sev_snp module
> parameter was set to false.

Yeah, I will also need additional params to init_args here:
https://lore.kernel.org/all/20260427204847.112899-2-tycho@xxxxxxxxxx/
so I think adding it there makes sense.

> But for a fix, this is probably simpler. It does skip some of the checks
> that _sev_platform_init_locked() has, but I think all of the checks that
> matter are performed for the paths that call sev_move_to_init_state().
> 
> Should this go to stable?

Yes, they all should as you point out, I'll add that for v2.

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

Thanks,

Tycho