Re: [PATCH v3 42/49] i386/sev: Add support for SNP CPUID validation

[Daniel P . Berrangé]

On Wed, Mar 20, 2024 at 03:39:38AM -0500, Michael Roth wrote:
> SEV-SNP firmware allows a special guest page to be populated with a
> table of guest CPUID values so that they can be validated through
> firmware before being loaded into encrypted guest memory where they can
> be used in place of hypervisor-provided values[1].
> 
> As part of SEV-SNP guest initialization, use this interface to validate
> the CPUID entries reported by KVM_GET_CPUID2 prior to initial guest
> start and populate the CPUID page reserved by OVMF with the resulting
> encrypted data.
> 
> [1] SEV SNP Firmware ABI Specification, Rev. 0.8, 8.13.2.6
> 
> Signed-off-by: Michael Roth <michael.roth@amd.com>
> ---
>  target/i386/sev.c | 159 +++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 158 insertions(+), 1 deletion(-)
> 

> +static void
> +sev_snp_cpuid_report_mismatches(SnpCpuidInfo *old,
> +                                SnpCpuidInfo *new)
> +{
> +    size_t i;
> +
> +    if (old->count != new->count) {
> +        error_report("SEV-SNP: CPUID validation failed due to count 
> mismatch, provided: %d, expected: %d",
> +                     old->count, new->count);
> +    }

Missing 'return' here, may result in array out of bounds on 'new->entries'
in the next loop

> +
> +    for (i = 0; i < old->count; i++) {
> +        SnpCpuidFunc *old_func, *new_func;
> +
> +        old_func = &old->entries[i];
> +        new_func = &new->entries[i];
> +
> +        if (memcmp(old_func, new_func, sizeof(SnpCpuidFunc))) {
> +            error_report("SEV-SNP: CPUID validation failed for function 
> 0x%x, index: 0x%x.\n"
> +                         "provided: eax:0x%08x, ebx: 0x%08x, ecx: 0x%08x, 
> edx: 0x%08x\n"
> +                         "expected: eax:0x%08x, ebx: 0x%08x, ecx: 0x%08x, 
> edx: 0x%08x",
> +                         old_func->eax_in, old_func->ecx_in,
> +                         old_func->eax, old_func->ebx, old_func->ecx, 
> old_func->edx,
> +                         new_func->eax, new_func->ebx, new_func->ecx, 
> new_func->edx);
> +        }
> +    }
> +}
> +


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|