[XEN PATCH] tools/firmware/hvmloader/smbios.c: Add new SMBIOS tables (7,8,9,26,27,28)

Anton Belousov posted 1 patch 1 week, 3 days ago
Test gitlab-ci failed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/AS8P193MB171873609ED0D3F997083C2DA3549@AS8P193MB1718.EURP193.PROD.OUTLOOK.COM
tools/firmware/hvmloader/smbios.c       | 146 ++++++++++++++++++++++++
tools/firmware/hvmloader/smbios_types.h |  77 +++++++++++++
2 files changed, 223 insertions(+)

[XEN PATCH] tools/firmware/hvmloader/smbios.c: Add new SMBIOS tables (7,8,9,26,27,28)

Posted by Anton Belousov 1 week, 3 days ago
SMBIOS tables like 7,8,9,26,27,28 are neccessary to prevent sandbox detection by malware
using WMI-queries. New tables can be mapped to memory from binary file specified in
"smbios_firmware" parameter of domain configuration. If particular table is absent
in binary file, then it will not be mapped to memory. This method works for Windows
domains as tables 7,8,9,26,27,28 are not critical for OS boot and runtime. Also if "smbios_firmware"
parameter is not provided, these tables will be skipped in write_smbios_tables function.

Signed-off-by: Anton Belousov <blsvntn@outlook.com>
---
 tools/firmware/hvmloader/smbios.c       | 146 ++++++++++++++++++++++++
 tools/firmware/hvmloader/smbios_types.h |  77 +++++++++++++
 2 files changed, 223 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);
+    }
+
+    return start;
+}
+
+/* Type 8 -- Port Connector Information */
+static void *
+smbios_type_8_init(void *start)
+{
+    struct smbios_type_8 *p = (struct smbios_type_8 *)start;
+
+    void *pts;
+    uint32_t length;
+
+    pts = get_smbios_pt_struct(8, &length);
+    if ( (pts != NULL)&&(length > 0) )
+    {
+        memcpy(start, pts, length);
+        p->header.handle = SMBIOS_HANDLE_TYPE8;
+        return (start + length);
+    }
+
+    return start;
+}
+
+/* Type 9 -- System Slots */
+static void *
+smbios_type_9_init(void *start)
+{
+    struct smbios_type_9 *p = (struct smbios_type_9 *)start;
+
+    void *pts;
+    uint32_t length;
+
+    pts = get_smbios_pt_struct(9, &length);
+    if ( (pts != NULL)&&(length > 0) )
+    {
+        memcpy(start, pts, length);
+        p->header.handle = SMBIOS_HANDLE_TYPE9;
+        return (start + length);
+    }
+
+    return start;
+}
+
 /* Type 11 -- OEM Strings */
 static void *
 smbios_type_11_init(void *start) 
@@ -923,6 +1007,68 @@ smbios_type_22_init(void *start)
     return start+1; 
 }
 
+/* Type 26 -- Voltage Probe */
+static void *
+smbios_type_26_init(void *start)
+{
+    struct smbios_type_26 *p = (struct smbios_type_26 *)start;
+
+    void *pts;
+    uint32_t length;
+
+    pts = get_smbios_pt_struct(26, &length);
+    if ( (pts != NULL)&&(length > 0) )
+    {
+        memcpy(start, pts, length);
+        p->header.handle = SMBIOS_HANDLE_TYPE26;
+        return (start + length);
+    }
+
+    return start;
+}
+
+/* Type 27 -- Cooling Device */
+static void *
+smbios_type_27_init(void *start)
+{
+    struct smbios_type_27 *p = (struct smbios_type_27 *)start;
+
+    void *pts;
+    uint32_t length;
+
+    pts = get_smbios_pt_struct(27, &length);
+    if ( (pts != NULL)&&(length > 0) )
+    {
+        memcpy(start, pts, length);
+        p->header.handle = SMBIOS_HANDLE_TYPE27;
+        p->temperature_probe_handle = SMBIOS_HANDLE_TYPE28;
+        p->cooling_unit_group = 0;
+        return (start + length);
+    }
+
+    return start;
+}
+
+/* Type 28 -- Temperature Probe */
+static void *
+smbios_type_28_init(void *start)
+{
+    struct smbios_type_28 *p = (struct smbios_type_28 *)start;
+
+    void *pts;
+    uint32_t length;
+
+    pts = get_smbios_pt_struct(28, &length);
+    if ( (pts != NULL)&&(length > 0) )
+    {
+        memcpy(start, pts, length);
+        p->header.handle = SMBIOS_HANDLE_TYPE28;
+        return (start + length);
+    }
+
+    return start;
+}
+
 /* Type 32 -- System Boot Information */
 static void *
 smbios_type_32_init(void *start)
diff --git a/tools/firmware/hvmloader/smbios_types.h b/tools/firmware/hvmloader/smbios_types.h
index 7c648ece71..d196c62229 100644
--- a/tools/firmware/hvmloader/smbios_types.h
+++ b/tools/firmware/hvmloader/smbios_types.h
@@ -149,6 +149,44 @@ struct smbios_type_4 {
     uint8_t part_number_str;
 } __attribute__ ((packed));
 
+/* SMBIOS type 7 - Cache Information */
+struct smbios_type_7 {
+    struct smbios_structure_header header;
+    uint8_t socket_designation_str;
+    uint16_t cache_configuration;
+    uint16_t maximum_cache_size;
+    uint16_t installed_size;
+    uint16_t supported_SRAM_type;
+    uint16_t current_SRAM_type;
+    uint8_t cache_speed;
+    uint8_t error_connection_type;
+    uint8_t system_cache_type;
+    uint8_t associativity;
+} __attribute__ ((packed));
+
+/* SMBIOS type 8 - Port Connector Information */
+struct smbios_type_8 {
+    struct smbios_structure_header header;
+    uint8_t internal_reference_designator_str;
+    uint8_t internal_connector_type;
+    uint8_t external_reference_designator_str;
+    uint8_t external_connector_type;
+    uint8_t port_type;
+} __attribute__ ((packed));
+
+/* SMBIOS type 9 - System Slots */
+struct smbios_type_9 {
+    struct smbios_structure_header header;
+    uint8_t slot_designation_str;
+    uint8_t slot_type;
+    uint8_t slot_data_bus_width;
+    uint8_t current_usage;
+    uint8_t slot_length;
+    uint16_t slot_id;
+    uint8_t slot_characteristics_1;
+    uint8_t slot_characteristics_2;
+} __attribute__ ((packed));
+
 /* SMBIOS type 11 - OEM Strings */
 struct smbios_type_11 {
     struct smbios_structure_header header;
@@ -232,6 +270,45 @@ struct smbios_type_22 {
     uint32_t oem_specific;
 } __attribute__ ((packed));
 
+/* SMBIOS type 26 - Voltage Probe */
+struct smbios_type_26 {
+    struct smbios_structure_header header;
+    uint8_t description_str;
+    uint8_t location_and_status;
+    uint16_t maximum_value;
+    uint16_t minimum_value;
+    uint16_t resolution;
+    uint16_t tolerance;
+    uint16_t accuracy;
+    uint32_t oem_defined;
+    uint16_t nominal_value;
+} __attribute__ ((packed));
+
+/* SMBIOS type 27 - Cooling Device */
+struct smbios_type_27 {
+    struct smbios_structure_header header;
+    uint16_t temperature_probe_handle;
+    uint8_t device_type_and_status;
+    uint8_t cooling_unit_group;
+    uint32_t oem_defined;
+    uint16_t nominal_speed;
+    uint8_t description_str;
+} __attribute__ ((packed));
+
+/* SMBIOS type 28 - Temperature Probe */
+struct smbios_type_28 {
+    struct smbios_structure_header header;
+    uint8_t description_str;
+    uint8_t location_and_status;
+    uint16_t maximum_value;
+    uint16_t minimum_value;
+    uint16_t resolution;
+    uint16_t tolerance;
+    uint16_t accuracy;
+    uint32_t oem_defined;
+    uint16_t nominal_value;
+} __attribute__ ((packed));
+
 /* SMBIOS type 32 - System Boot Information */
 struct smbios_type_32 {
     struct smbios_structure_header header;
-- 
2.25.1


Re: [XEN PATCH] tools/firmware/hvmloader/smbios.c: Add new SMBIOS tables (7,8,9,26,27,28)

Posted by Jan Beulich 1 week ago
On 14.01.2022 09:52, Anton Belousov wrote:
> SMBIOS tables like 7,8,9,26,27,28 are neccessary to prevent sandbox detection by malware
> using WMI-queries.

This is a statement without any proof. It may seem obvious to you, but
could you please make us properly see the value of your addition as
well as allow justification towards the completeness of your changes
(i.e. that there aren't further types specifying of which may also
help)?

Furthermore, with the original intention of the machinery having been
to pass through host SMBIOS structures, you will also want to say a
word on the (intended) source of the data here. If it's again the host
table, then you will want to also discuss the security of doing so.

> New tables can be mapped to memory from binary file specified in
> "smbios_firmware" parameter of domain configuration. If particular table is absent
> in binary file, then it will not be mapped to memory. This method works for Windows
> domains as tables 7,8,9,26,27,28 are not critical for OS boot and runtime. Also if "smbios_firmware"
> parameter is not provided, these tables will be skipped in write_smbios_tables function.
> 
> Signed-off-by: Anton Belousov <blsvntn@outlook.com>
> ---
>  tools/firmware/hvmloader/smbios.c       | 146 ++++++++++++++++++++++++
>  tools/firmware/hvmloader/smbios_types.h |  77 +++++++++++++
>  2 files changed, 223 insertions(+)

Somewhere here please have a brief description of what has changed
between versions. Also please properly tag you patch with a version
(this one looks to be v2).

> @@ -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;

Please avoid introducing casts when none are necessary.

> +    void *pts;
> +    uint32_t length;
> +
> +    pts = get_smbios_pt_struct(7, &length);
> +    if ( (pts != NULL)&&(length > 0) )

In reply to v1 I did ask you to avoid copying existing style violations.
In the example here there are missing blanks on either side of &&.

I further wonder whether indeed any non-zero length is fine. IOW it may
be worthwhile to first harden the pre-existing cases a little before
further cloning them. Actually, seeing that the smbios_type_<N>_init()
functions are all largely identical, it may be worthwhile to introduce
a single function doing what is needed for all overridable types.

Jan