Re: SVSM Attestation and vTPM specification additions - v0.61

["Jörg Rödel" <jroedel@suse.de>]

On Tue, Feb 21, 2023 at 04:07:46PM -0600, Tom Lendacky wrote:
> VCPU create doesn't actually replace the VMSA from the hypervisor point of
> view. The guest is still required to invoke the GHCB interface to notify the
> hypervisor.
> 
> The SVSM PoC leaves any current VMSA in the list of all VMSAs, but does
> replace the current / in-use version of the VMSA for the specified VCPU.
> This is used to examine the register associated with a request, so the guest
> can shoot itself in the foot if it doesn't switch that VCPU to the new VMSA.
> 
> I'm ok with either doing that or, as you suggest below, checking if there's
> an existing VMSA for the VMPL level and failing the request until the the
> existing VMSA is deleted. That does require the deletion/creation to be
> offloaded to another VCPU, though (unless we allow the self-deletion change
> talked about below).

Yeah, so the current protocol expects a bit too much from the code
running at VMPL1 for my personal taste. I think it would be better if we
put some constraints into the handling of CREATE_VCPU and DELETE_VCPU
calls to keep the number of pages marked as VMSA as small as possible.

With the current semantics the guest is required to call DELETE_VCPU on
any VMSA it no longer uses. But it is not required to do so. Also there
is a race window between switching to the new VMSA and deleting the old
one where the HV could run the old and the new VMSA in parallel.

The SVSM will not get to see any old VMSA before it is retired (e.g. to
clear EFER.SVME and make it invalid) until the DELETE_VCPU call, but at
that time the new VMSA could already be running.

I don't think we can fully prevent such attacks, but we can make them
more complicated by having some constraints who and when VCPUs can be
created and deleted.

In particular:

	1) VCPUs can only delete themselves. Exception is VCPU 0
	   (or whichever VCPU the BSP is).

	2) VCPUs can only be created for same or higher VMPLs on any
	   VCPU in the guest, given that there is no VMSA configured for
	   this (VCPU, VMPL) combination.

These are constraints the SVSM can enforce and it should keep the number
of VMSAs in the system minimal.

It requires some changes to the current OVMF patches for SNP and SVSM,
though.

> It might be desirable to allow another APIC to delete a VMSA. Maybe kexec
> processing would be able to benefit from that.

Yeah, maybe. On the other side kexec would benefit if VCPUs delete
themselves in the old kernel. Otherwise the new kernel needs to find the
VMSA GPAs und delete them manually.

There is already a code-path for kexec/kdump where the non-boot CPUs are
parked. This could be instrumented. If a VCPU deletes itself the SVSM
will go into HLT until a new VCPU is created.

> Should the clearing of the SVME bit fail, then a FAIL_INUSE return code
> would be returned. I wouldn't think that the synchronization would be that
> difficult, but I'm not sure. If we do allow self-deletion, then, I agree,
> that would work nicely.

The question here is, can we reliably detect within the SVSM that
clearing EFER.SVME failed?

Regards,

-- 
Jörg Rödel
jroedel@suse.de

SUSE Software Solutions Germany GmbH
Frankenstraße 146
90461 Nürnberg
Germany

(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman