| Message ID | 20211108134840.2757206-5-dovmurik@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SEV: add kernel-hashes=on for measured -kernel launch | expand |
On Mon, Nov 08, 2021 at 01:48:38PM +0000, Dov Murik wrote: > Commit cff03145ed3c ("sev/i386: Introduce sev_add_kernel_loader_hashes > for measured linux boot", 2021-09-30) introduced measured direct boot > with -kernel, using an OVMF-designated hashes table which QEMU fills. > > However, no checks are performed on the validity of the hashes area > designated by OVMF. Specifically, if OVMF publishes the > SEV_HASH_TABLE_RV_GUID entry but it is filled with zeroes, this will > cause QEMU to write the hashes entries over the first page of the > guest's memory (GPA 0). > > Add validity checks to the published area. If the hashes table area's > base address is zero, or its size is too small to fit the aligned hashes > table, display an error and stop the guest launch. In such case, the > following error will be displayed: > > qemu-system-x86_64: SEV: guest firmware hashes table area is invalid (base=0x0 size=0x0) > > Signed-off-by: Dov Murik <dovmurik@linux.ibm.com> > Reported-by: Brijesh Singh <brijesh.singh@amd.com> > --- > target/i386/sev.c | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel
diff --git a/target/i386/sev.c b/target/i386/sev.c index c71d23654f..2588bd623f 100644 --- a/target/i386/sev.c +++ b/target/i386/sev.c @@ -1221,7 +1221,7 @@ bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp) uint8_t kernel_hash[HASH_SIZE]; uint8_t *hashp; size_t hash_len = HASH_SIZE; - int aligned_len; + int aligned_len = ROUND_UP(sizeof(SevHashTable), 16); /* * Only add the kernel hashes if the sev-guest configuration explicitly @@ -1237,6 +1237,11 @@ bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp) return false; } area = (SevHashTableDescriptor *)data; + if (!area->base || area->size < aligned_len) { + error_setg(errp, "SEV: guest firmware hashes table area is invalid " + "(base=0x%x size=0x%x)", area->base, area->size); + return false; + } /* * Calculate hash of kernel command-line with the terminating null byte. If @@ -1295,7 +1300,6 @@ bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp) memcpy(ht->kernel.hash, kernel_hash, sizeof(ht->kernel.hash)); /* When calling sev_encrypt_flash, the length has to be 16 byte aligned */ - aligned_len = ROUND_UP(ht->len, 16); if (aligned_len != ht->len) { /* zero the excess data so the measurement can be reliably calculated */ memset(ht->padding, 0, aligned_len - ht->len);
Commit cff03145ed3c ("sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux boot", 2021-09-30) introduced measured direct boot with -kernel, using an OVMF-designated hashes table which QEMU fills. However, no checks are performed on the validity of the hashes area designated by OVMF. Specifically, if OVMF publishes the SEV_HASH_TABLE_RV_GUID entry but it is filled with zeroes, this will cause QEMU to write the hashes entries over the first page of the guest's memory (GPA 0). Add validity checks to the published area. If the hashes table area's base address is zero, or its size is too small to fit the aligned hashes table, display an error and stop the guest launch. In such case, the following error will be displayed: qemu-system-x86_64: SEV: guest firmware hashes table area is invalid (base=0x0 size=0x0) Signed-off-by: Dov Murik <dovmurik@linux.ibm.com> Reported-by: Brijesh Singh <brijesh.singh@amd.com> --- target/i386/sev.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)