Re: [XEN PATCH 1/1] Add suport for SMBIOS tables 7,8,9,26,27,28 to improve virtual machine stealth from malware.

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

Hello,

On Mon, Nov 29, 2021 at 12:59:29PM +0000, Anton Belousov wrote:

Than ks for the patch, I'm afraid this requires a proper commit
message and a Signed-off-by tag. See:

https://wiki.xenproject.org/wiki/Submitting_Xen_Project_Patches#Title_and_description_of_the_patch

> ---
>  tools/firmware/hvmloader/smbios.c       | 146 ++++++++++++++++++++++++
>  tools/firmware/hvmloader/smbios_types.h |  76 ++++++++++++
>  2 files changed, 222 insertions(+)
> 
> diff --git a/tools/firmware/hvmloader/smbios.c 
> b/tools/firmware/hvmloader/smbios.c
> index 97a054e9e3..f5e61c1159 100644
> --- a/tools/firmware/hvmloader/smbios.c
> +++ b/tools/firmware/hvmloader/smbios.c
> @@ -33,12 +33,18 @@
>  #define SMBIOS_HANDLE_TYPE2   0x0200
>  #define SMBIOS_HANDLE_TYPE3   0x0300
>  #define SMBIOS_HANDLE_TYPE4   0x0400
> +#define SMBIOS_HANDLE_TYPE7   0x0700
> +#define SMBIOS_HANDLE_TYPE8   0x0800
> +#define SMBIOS_HANDLE_TYPE9   0x0900
>  #define SMBIOS_HANDLE_TYPE11  0x0B00
>  #define SMBIOS_HANDLE_TYPE16  0x1000
>  #define SMBIOS_HANDLE_TYPE17  0x1100
>  #define SMBIOS_HANDLE_TYPE19  0x1300
>  #define SMBIOS_HANDLE_TYPE20  0x1400
>  #define SMBIOS_HANDLE_TYPE22  0x1600
> +#define SMBIOS_HANDLE_TYPE26  0x1A00
> +#define SMBIOS_HANDLE_TYPE27  0x1B00
> +#define SMBIOS_HANDLE_TYPE28  0x1C00
>  #define SMBIOS_HANDLE_TYPE32  0x2000
>  #define SMBIOS_HANDLE_TYPE39  0x2700
>  #define SMBIOS_HANDLE_TYPE127 0x7f00
> @@ -77,6 +83,12 @@ static void *
>  smbios_type_4_init(void *start, unsigned int cpu_number,
>                     char *cpu_manufacturer);
>  static void *
> +smbios_type_7_init(void *start);
> +static void *
> +smbios_type_8_init(void *start);
> +static void *
> +smbios_type_9_init(void *start);
> +static void *
>  smbios_type_11_init(void *start);
>  static void *
>  smbios_type_16_init(void *start, uint32_t memory_size_mb, int nr_mem_devs);
> @@ -89,6 +101,12 @@ smbios_type_20_init(void *start, uint32_t memory_size_mb, 
> int instance);
>  static void *
>  smbios_type_22_init(void *start);
>  static void *
> +smbios_type_26_init(void *start);
> +static void *
> +smbios_type_27_init(void *start);
> +static void *
> +smbios_type_28_init(void *start);
> +static void *
>  smbios_type_32_init(void *start);
>  static void *
>  smbios_type_39_init(void *start);
> @@ -205,6 +223,9 @@ write_smbios_tables(void *ep, void *start,
>      do_struct(smbios_type_3_init(p));
>      for ( cpu_num = 1; cpu_num <= vcpus; cpu_num++ )
>          do_struct(smbios_type_4_init(p, cpu_num, cpu_manufacturer));
> +    do_struct(smbios_type_7_init(p));
> +    do_struct(smbios_type_8_init(p));
> +    do_struct(smbios_type_9_init(p));
>      do_struct(smbios_type_11_init(p));
>  
>      /* Each 'memory device' covers up to 16GB of address space. */
> @@ -221,6 +242,9 @@ write_smbios_tables(void *ep, void *start,
>      }
>  
>      do_struct(smbios_type_22_init(p));
> +    do_struct(smbios_type_26_init(p));
> +    do_struct(smbios_type_28_init(p));
> +    do_struct(smbios_type_27_init(p));
>      do_struct(smbios_type_32_init(p));
>      do_struct(smbios_type_39_init(p));
>      do_struct(smbios_type_vendor_oem_init(p));
> @@ -700,6 +724,66 @@ smbios_type_4_init(
>      return start+1;
>  }
>  
> +/* Type 7 -- Cache Information */
> +static void *
> +smbios_type_7_init(void *start)
> +{
> +    struct smbios_type_7 *p = (struct smbios_type_7 *)start;
> +
> +    void *pts;
> +    uint32_t length;
> +
> +    pts = get_smbios_pt_struct(7, &length);
> +    if ( (pts != NULL)&&(length > 0) )
> +    {
> +        memcpy(start, pts, length);
> +        p->header.handle = SMBIOS_HANDLE_TYPE7;
> +        return (start + length);
> +    }

Here and below for the added types: would it make sense to fill them
with some default information in the absence of any data passed in?

I'm afraid this requires some commit message in order to properly
review it.

Thanks, Roger.