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(-)

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

* 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
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

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>