On Tue, 4 Jun 2024 at 07:47, Paolo Bonzini <[email protected]> wrote:
>
> From: Michael Roth <[email protected]>
>
> Currently all SEV/SEV-ES functionality is managed through a single
> 'sev-guest' QOM type. With upcoming support for SEV-SNP, taking this
> same approach won't work well since some of the properties/state
> managed by 'sev-guest' is not applicable to SEV-SNP, which will instead
> rely on a new QOM type with its own set of properties/state.
>
> To prepare for this, this patch moves common state into an abstract
> 'sev-common' parent type to encapsulate properties/state that are
> common to both SEV/SEV-ES and SEV-SNP, leaving only SEV/SEV-ES-specific
> properties/state in the current 'sev-guest' type. This should not
> affect current behavior or command-line options.
>
> As part of this patch, some related changes are also made:
>
> - a static 'sev_guest' variable is currently used to keep track of
> the 'sev-guest' instance. SEV-SNP would similarly introduce an
> 'sev_snp_guest' static variable. But these instances are now
> available via qdev_get_machine()->cgs, so switch to using that
> instead and drop the static variable.
>
> - 'sev_guest' is currently used as the name for the static variable
> holding a pointer to the 'sev-guest' instance. Re-purpose the name
> as a local variable referring the 'sev-guest' instance, and use
> that consistently throughout the code so it can be easily
> distinguished from sev-common/sev-snp-guest instances.
>
> - 'sev' is generally used as the name for local variables holding a
> pointer to the 'sev-guest' instance. In cases where that now points
> to common state, use the name 'sev_common'; in cases where that now
> points to state specific to 'sev-guest' instance, use the name
> 'sev_guest'
>
> In order to enable kernel-hashes for SNP, pull it from
> SevGuestProperties to its parent SevCommonProperties so
> it will be available for both SEV and SNP.
Hi; Coverity points out a problem in this code (CID 1546885):
> @@ -540,12 +491,21 @@ static SevCapability *sev_get_capabilities(Error **errp)
> return NULL;
> }
>
> - fd = open(DEFAULT_SEV_DEVICE, O_RDWR);
> + sev_common = SEV_COMMON(MACHINE(qdev_get_machine())->cgs);
> + if (!sev_common) {
> + error_setg(errp, "SEV is not configured");
Here we check for a NULL pointer, but we forget to "return",
so execution will continue on...
> + }
> +
> + sev_device = object_property_get_str(OBJECT(sev_common), "sev-device",
> + &error_abort);
...and QEMU will crash here when object_property_get_str()
dereferences the NULL pointer.
Adding a "return NULL;" should fix this.
thanks
-- PMM
