| Message ID | 20211101102136.1706421-4-dovmurik@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | SEV: fixes for -kernel launch with incompatible OVMF | expand |
* Dov Murik (dovmurik@linux.ibm.com) wrote: > In sev_add_kernel_loader_hashes, the sizes of structs are known at > compile-time, so calculate needed padding at compile-time. > > No functional change intended. > > Signed-off-by: Dov Murik <dovmurik@linux.ibm.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > target/i386/sev.c | 28 ++++++++++++++++++---------- > 1 file changed, 18 insertions(+), 10 deletions(-) > > diff --git a/target/i386/sev.c b/target/i386/sev.c > index a20ddb545e..c09de9c6f0 100644 > --- a/target/i386/sev.c > +++ b/target/i386/sev.c > @@ -109,9 +109,19 @@ typedef struct QEMU_PACKED SevHashTable { > SevHashTableEntry cmdline; > SevHashTableEntry initrd; > SevHashTableEntry kernel; > - uint8_t padding[]; > } SevHashTable; > > +/* > + * Data encrypted by sev_encrypt_flash() must be padded to a multiple of > + * 16 bytes. > + */ > +typedef struct QEMU_PACKED PaddedSevHashTable { > + SevHashTable ht; > + uint8_t padding[ROUND_UP(sizeof(SevHashTable), 16) - sizeof(SevHashTable)]; > +} PaddedSevHashTable; > + > +QEMU_BUILD_BUG_ON(sizeof(PaddedSevHashTable) % 16 != 0); > + > static SevGuestState *sev_guest; > static Error *sev_mig_blocker; > > @@ -1196,19 +1206,19 @@ bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp) > uint8_t *data; > SevHashTableDescriptor *area; > SevHashTable *ht; > + PaddedSevHashTable *padded_ht; > uint8_t cmdline_hash[HASH_SIZE]; > uint8_t initrd_hash[HASH_SIZE]; > uint8_t kernel_hash[HASH_SIZE]; > uint8_t *hashp; > size_t hash_len = HASH_SIZE; > - int aligned_len = ROUND_UP(sizeof(SevHashTable), 16); > > if (!pc_system_ovmf_table_find(SEV_HASH_TABLE_RV_GUID, &data, NULL)) { > warn_report("SEV: kernel specified but OVMF has no hash table guid"); > return false; > } > area = (SevHashTableDescriptor *)data; > - if (!area->base || area->size < aligned_len) { > + if (!area->base || area->size < sizeof(PaddedSevHashTable)) { > warn_report("SEV: OVMF's hashes table area is invalid (base=0x%x size=0x%x)", > area->base, area->size); > return false; > @@ -1253,7 +1263,8 @@ bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp) > * Populate the hashes table in the guest's memory at the OVMF-designated > * area for the SEV hashes table > */ > - ht = qemu_map_ram_ptr(NULL, area->base); > + padded_ht = qemu_map_ram_ptr(NULL, area->base); > + ht = &padded_ht->ht; > > ht->guid = sev_hash_table_header_guid; > ht->len = sizeof(*ht); > @@ -1270,13 +1281,10 @@ bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp) > ht->kernel.len = sizeof(ht->kernel); > memcpy(ht->kernel.hash, kernel_hash, sizeof(ht->kernel.hash)); > > - /* When calling sev_encrypt_flash, the length has to be 16 byte aligned */ > - if (aligned_len != ht->len) { > - /* zero the excess data so the measurement can be reliably calculated */ > - memset(ht->padding, 0, aligned_len - ht->len); > - } > + /* zero the excess data so the measurement can be reliably calculated */ > + memset(padded_ht->padding, 0, sizeof(padded_ht->padding)); > > - if (sev_encrypt_flash((uint8_t *)ht, aligned_len, errp) < 0) { > + if (sev_encrypt_flash((uint8_t *)padded_ht, sizeof(*padded_ht), errp) < 0) { > return false; > } > > -- > 2.25.1 >
On 02/11/2021 13:36, Dr. David Alan Gilbert wrote: > * Dov Murik (dovmurik@linux.ibm.com) wrote: >> In sev_add_kernel_loader_hashes, the sizes of structs are known at >> compile-time, so calculate needed padding at compile-time. >> >> No functional change intended. >> >> Signed-off-by: Dov Murik <dovmurik@linux.ibm.com> > > > > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > Thanks Dave. -Dov
On 11/1/21 11:21, Dov Murik wrote: > In sev_add_kernel_loader_hashes, the sizes of structs are known at > compile-time, so calculate needed padding at compile-time. > > No functional change intended. > > Signed-off-by: Dov Murik <dovmurik@linux.ibm.com> > --- > target/i386/sev.c | 28 ++++++++++++++++++---------- > 1 file changed, 18 insertions(+), 10 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
diff --git a/target/i386/sev.c b/target/i386/sev.c index a20ddb545e..c09de9c6f0 100644 --- a/target/i386/sev.c +++ b/target/i386/sev.c @@ -109,9 +109,19 @@ typedef struct QEMU_PACKED SevHashTable { SevHashTableEntry cmdline; SevHashTableEntry initrd; SevHashTableEntry kernel; - uint8_t padding[]; } SevHashTable; +/* + * Data encrypted by sev_encrypt_flash() must be padded to a multiple of + * 16 bytes. + */ +typedef struct QEMU_PACKED PaddedSevHashTable { + SevHashTable ht; + uint8_t padding[ROUND_UP(sizeof(SevHashTable), 16) - sizeof(SevHashTable)]; +} PaddedSevHashTable; + +QEMU_BUILD_BUG_ON(sizeof(PaddedSevHashTable) % 16 != 0); + static SevGuestState *sev_guest; static Error *sev_mig_blocker; @@ -1196,19 +1206,19 @@ bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp) uint8_t *data; SevHashTableDescriptor *area; SevHashTable *ht; + PaddedSevHashTable *padded_ht; uint8_t cmdline_hash[HASH_SIZE]; uint8_t initrd_hash[HASH_SIZE]; uint8_t kernel_hash[HASH_SIZE]; uint8_t *hashp; size_t hash_len = HASH_SIZE; - int aligned_len = ROUND_UP(sizeof(SevHashTable), 16); if (!pc_system_ovmf_table_find(SEV_HASH_TABLE_RV_GUID, &data, NULL)) { warn_report("SEV: kernel specified but OVMF has no hash table guid"); return false; } area = (SevHashTableDescriptor *)data; - if (!area->base || area->size < aligned_len) { + if (!area->base || area->size < sizeof(PaddedSevHashTable)) { warn_report("SEV: OVMF's hashes table area is invalid (base=0x%x size=0x%x)", area->base, area->size); return false; @@ -1253,7 +1263,8 @@ bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp) * Populate the hashes table in the guest's memory at the OVMF-designated * area for the SEV hashes table */ - ht = qemu_map_ram_ptr(NULL, area->base); + padded_ht = qemu_map_ram_ptr(NULL, area->base); + ht = &padded_ht->ht; ht->guid = sev_hash_table_header_guid; ht->len = sizeof(*ht); @@ -1270,13 +1281,10 @@ bool sev_add_kernel_loader_hashes(SevKernelLoaderContext *ctx, Error **errp) ht->kernel.len = sizeof(ht->kernel); memcpy(ht->kernel.hash, kernel_hash, sizeof(ht->kernel.hash)); - /* When calling sev_encrypt_flash, the length has to be 16 byte aligned */ - if (aligned_len != ht->len) { - /* zero the excess data so the measurement can be reliably calculated */ - memset(ht->padding, 0, aligned_len - ht->len); - } + /* zero the excess data so the measurement can be reliably calculated */ + memset(padded_ht->padding, 0, sizeof(padded_ht->padding)); - if (sev_encrypt_flash((uint8_t *)ht, aligned_len, errp) < 0) { + if (sev_encrypt_flash((uint8_t *)padded_ht, sizeof(*padded_ht), errp) < 0) { return false; }
In sev_add_kernel_loader_hashes, the sizes of structs are known at compile-time, so calculate needed padding at compile-time. No functional change intended. Signed-off-by: Dov Murik <dovmurik@linux.ibm.com> --- target/i386/sev.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-)