x86/tboot: include all valid frame table entries in S3 integrity check

[PATCH] x86/tboot: include all valid frame table entries in S3 integrity check

Posted by Jan Beulich 2 years, 11 months ago

The difference of two pdx_to_page() return values is a number of pages,
not the number of bytes covered by the corresponding frame table entries.

Fixes: 3cb68d2b59ab ("tboot: fix S3 issue for Intel Trusted Execution Technology.")
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/tboot.c
+++ b/xen/arch/x86/tboot.c
@@ -323,12 +323,12 @@ static void tboot_gen_frametable_integri
         if ( nidx >= max_idx )
             break;
         vmac_update((uint8_t *)pdx_to_page(sidx * PDX_GROUP_COUNT),
-                       pdx_to_page(eidx * PDX_GROUP_COUNT)
-                       - pdx_to_page(sidx * PDX_GROUP_COUNT), &ctx);
+                    (eidx - sidx) * PDX_GROUP_COUNT * sizeof(*frame_table),
+                    &ctx);
     }
     vmac_update((uint8_t *)pdx_to_page(sidx * PDX_GROUP_COUNT),
-                   pdx_to_page(max_pdx - 1) + 1
-                   - pdx_to_page(sidx * PDX_GROUP_COUNT), &ctx);
+                (max_pdx - sidx * PDX_GROUP_COUNT) * sizeof(*frame_table),
+                &ctx);
 
     *mac = vmac(NULL, 0, nonce, NULL, &ctx);

Ping: [PATCH] x86/tboot: include all valid frame table entries in S3 integrity check

Posted by Jan Beulich 2 years, 10 months ago

On 19.05.2021 17:48, Jan Beulich wrote:
> The difference of two pdx_to_page() return values is a number of pages,
> not the number of bytes covered by the corresponding frame table entries.
> 
> Fixes: 3cb68d2b59ab ("tboot: fix S3 issue for Intel Trusted Execution Technology.")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/x86/tboot.c
> +++ b/xen/arch/x86/tboot.c
> @@ -323,12 +323,12 @@ static void tboot_gen_frametable_integri
>          if ( nidx >= max_idx )
>              break;
>          vmac_update((uint8_t *)pdx_to_page(sidx * PDX_GROUP_COUNT),
> -                       pdx_to_page(eidx * PDX_GROUP_COUNT)
> -                       - pdx_to_page(sidx * PDX_GROUP_COUNT), &ctx);
> +                    (eidx - sidx) * PDX_GROUP_COUNT * sizeof(*frame_table),
> +                    &ctx);
>      }
>      vmac_update((uint8_t *)pdx_to_page(sidx * PDX_GROUP_COUNT),
> -                   pdx_to_page(max_pdx - 1) + 1
> -                   - pdx_to_page(sidx * PDX_GROUP_COUNT), &ctx);
> +                (max_pdx - sidx * PDX_GROUP_COUNT) * sizeof(*frame_table),
> +                &ctx);
>  
>      *mac = vmac(NULL, 0, nonce, NULL, &ctx);
>  
>

Re: [PATCH] x86/tboot: include all valid frame table entries in S3 integrity check

Posted by Lukasz Hawrylko 2 years, 10 months ago

On Wed, 2021-05-19 at 17:48 +0200, Jan Beulich wrote:
> The difference of two pdx_to_page() return values is a number of pages,
> not the number of bytes covered by the corresponding frame table entries.
> 
> Fixes: 3cb68d2b59ab ("tboot: fix S3 issue for Intel Trusted Execution Technology.")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 

Reviewed-by: Lukasz Hawrylko <lukasz.hawrylko@linux.intel.com>