diff mbox

答复: [PATCH] smbios: Add 1 terminator if there is any string field defined in given table.

Message ID 58211B8102000062000AE074@prv-mh.provo.novell.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lin Ma Nov. 7, 2016, 4:25 p.m. UTC
Ping.

>>> Lin Ma <lma@suse.com> 2016/9/6 星期二 下午 4:28 >>>
If user specifies binary file on command line to load smbios entries, then
will get error messages while decoding them in guest.

Reproducer:
1. dump a smbios table to a binary file from host or guest.(says table 1)
2. load the binary file through command line: 'qemu -smbios file=...'.
3. perform 'dmidecode' or 'dmidecode -t 1' in guest.

It reports 'Invalid entry length...' because qemu doesn't add terminator(s) for
the table correctly.
For smbios tables which have string field provided, qemu should add 1 terminator.
For smbios tables which dont have string field provided, qemu should add 2.

This patch fixed the issue.

Signed-off-by: Lin Ma <lma@suse.com>
---
hw/smbios/smbios.c		 | 90 ++++++++++++++++++++++++++++++++++++++++++++++
include/hw/smbios/smbios.h | 44 +++++++++++++++++++++++
2 files changed, 134 insertions(+)

Comments

Daniel P. Berrangé Nov. 7, 2016, 4:31 p.m. UTC | #1
On Mon, Nov 07, 2016 at 09:25:37AM -0700, Lin Ma wrote:
> Ping.
> 
> >>> Lin Ma <lma@suse.com> 2016/9/6 星期二 下午 4:28 >>>
> If user specifies binary file on command line to load smbios entries, then
> will get error messages while decoding them in guest.
> 
> Reproducer:
> 1. dump a smbios table to a binary file from host or guest.(says table 1)
> 2. load the binary file through command line: 'qemu -smbios file=...'.
> 3. perform 'dmidecode' or 'dmidecode -t 1' in guest.
> 
> It reports 'Invalid entry length...' because qemu doesn't add terminator(s) for
> the table correctly.
> For smbios tables which have string field provided, qemu should add 1 terminator.
> For smbios tables which dont have string field provided, qemu should add 2.
> 
> This patch fixed the issue.
> 
> Signed-off-by: Lin Ma <lma@suse.com>
> ---
> hw/smbios/smbios.c		 | 90 ++++++++++++++++++++++++++++++++++++++++++++++
> include/hw/smbios/smbios.h | 44 +++++++++++++++++++++++
> 2 files changed, 134 insertions(+)
> 
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 74c7102..6293bc5 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -885,6 +885,9 @@ void smbios_entry_add(QemuOpts *opts)
> {
> 	 const char *val;
> 
> +    int i, terminator_count = 2, table_str_field_count = 0;
> +    int *tables_str_field_offset = NULL;
> +
> 	 assert(!smbios_immutable);
> 
> 	 val = qemu_opt_get(opts, "file");
> @@ -926,7 +929,94 @@ void smbios_entry_add(QemuOpts *opts)
> 			 smbios_type4_count++;
> 		 }
> 
> +	    switch (header->type) {
> +	    case 0:
> +		    tables_str_field_offset = g_malloc0(sizeof(int) * \
> +											    TYPE_0_STR_FIELD_COUNT);
> +		    tables_str_field_offset = (int []){\
> +								    TYPE_0_STR_FIELD_OFFSET_VENDOR, \
> +								    TYPE_0_STR_FIELD_OFFSET_BIOS_VERSION, \
> +								    TYPE_0_STR_FIELD_OFFSET_BIOS_RELEASE_DATE};
> +		    table_str_field_count = sizeof(tables_str_field_offset) / \
> +								    sizeof(tables_str_field_offset[0]);
> +		    break;
> +	    case 1:
> +		    tables_str_field_offset = g_malloc0(sizeof(int) * \
> +											    TYPE_1_STR_FIELD_COUNT);
> +		    tables_str_field_offset = (int []){
> +								    TYPE_1_STR_FIELD_OFFSET_MANUFACTURER, \
> +								    TYPE_1_STR_FIELD_OFFSET_PRODUCT, \
> +								    TYPE_1_STR_FIELD_OFFSET_VERSION, \
> +								    TYPE_1_STR_FIELD_OFFSET_SERIAL, \
> +								    TYPE_1_STR_FIELD_OFFSET_SKU, \
> +								    TYPE_1_STR_FIELD_OFFSET_FAMILY};
> +		    table_str_field_count = sizeof(tables_str_field_offset) / \
> +								    sizeof(tables_str_field_offset[0]);
> +		    break;
> +	    case 2:
> +		    tables_str_field_offset = g_malloc0(sizeof(int) * \
> +											    TYPE_2_STR_FIELD_COUNT);
> +		    tables_str_field_offset = (int []){\
> +								    TYPE_2_STR_FIELD_OFFSET_MANUFACTURER, \
> +								    TYPE_2_STR_FIELD_OFFSET_PRODUCT, \
> +								    TYPE_2_STR_FIELD_OFFSET_VERSION, \
> +								    TYPE_2_STR_FIELD_OFFSET_SERIAL, \
> +								    TYPE_2_STR_FIELD_OFFSET_ASSET, \
> +								    TYPE_2_STR_FIELD_OFFSET_LOCATION};
> +		    table_str_field_count = sizeof(tables_str_field_offset) / \
> +								    sizeof(tables_str_field_offset[0]);
> +		    break;
> +	    case 3:
> +		    tables_str_field_offset = g_malloc0(sizeof(int) * \
> +											    TYPE_3_STR_FIELD_COUNT);
> +		    tables_str_field_offset = (int []){\
> +								    TYPE_3_STR_FIELD_OFFSET_MANUFACTURER, \
> +								    TYPE_3_STR_FIELD_OFFSET_VERSION, \
> +								    TYPE_3_STR_FIELD_OFFSET_SERIAL, \
> +								    TYPE_3_STR_FIELD_OFFSET_ASSET, \
> +								    TYPE_3_STR_FIELD_OFFSET_SKU};
> +		    table_str_field_count = sizeof(tables_str_field_offset) / \
> +								    sizeof(tables_str_field_offset[0]);
> +		    break;
> +	    case 4:
> +		    tables_str_field_offset = g_malloc0(sizeof(int) * \
> +											    TYPE_4_STR_FIELD_COUNT);
> +		    tables_str_field_offset = (int []){\
> +								    TYPE_4_STR_FIELD_OFFSET_SOCKET, \
> +								    TYPE_4_STR_FIELD_OFFSET_PROCESSOR_MANUFACTURER, \
> +								    TYPE_4_STR_FIELD_OFFSET_PROCESSOR_VERSION, \
> +								    TYPE_4_STR_FIELD_OFFSET_SERIAL, \
> +								    TYPE_4_STR_FIELD_OFFSET_ASSET, \
> +
>  TYPE_4_STR_FIELD_OFFSET_PART};
> +		    table_str_field_count = sizeof(tables_str_field_offset) / \
> +								    sizeof(tables_str_field_offset[0]);
> +		    break;
> +	    case 17:
> +		    tables_str_field_offset = g_malloc0(sizeof(int) * \
> +											    TYPE_17_STR_FIELD_COUNT);
> +		    tables_str_field_offset = (int []){\
> +								    TYPE_17_STR_FIELD_OFFSET_DEVICE_LOCATOR, \
> +								    TYPE_17_STR_FIELD_OFFSET_BANK_LOCATOR, \
> +								    TYPE_17_STR_FIELD_OFFSET_MANUFACTURER, \
> +								    TYPE_17_STR_FIELD_OFFSET_SERIAL, \
> +								    TYPE_17_STR_FIELD_OFFSET_ASSET, \
> +								    TYPE_17_STR_FIELD_OFFSET_PART};
> +		    table_str_field_count = sizeof(tables_str_field_offset) / \
> +								    sizeof(tables_str_field_offset[0]);
> +		    break;
> +	    default:
> +		    break;
> +	    }
> +
> +	    for (i = 0; i < table_str_field_count; i++) {
> +		    if (*(uint8_t *)(smbios_tables + tables_str_field_offset[i]) > 0) {
> +			    terminator_count = 1;
> +			    break;
> +		    }
> +	    }
> +
> 		 smbios_tables_len += size;
> +	    smbios_tables_len += terminator_count;
> 		 if (size > smbios_table_max) {
> 			 smbios_table_max = size;
> 		 }

Code identation is this patch looks totally mangled.

Regards,
Daniel
Lin Ma Nov. 7, 2016, 4:45 p.m. UTC | #2
>>> "Daniel P. Berrange" berrange@redhat.com> 2016/11/8 星期二 上午 12:31 >>
( mailto:berrange@redhat.com) 
......
>>
>
>Code identation is this patch looks totally mangled.
>
How about this one: (From the code style's perspective, it should be no big problem)
http://lists.nongnu.org/archive/html/qemu-devel/2016-09/msg01024.html
 
Thanks,
Lin
diff mbox

Patch

diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
index 74c7102..6293bc5 100644
--- a/hw/smbios/smbios.c
+++ b/hw/smbios/smbios.c
@@ -885,6 +885,9 @@  void smbios_entry_add(QemuOpts *opts)
{
	 const char *val;

+    int i, terminator_count = 2, table_str_field_count = 0;
+    int *tables_str_field_offset = NULL;
+
	 assert(!smbios_immutable);

	 val = qemu_opt_get(opts, "file");
@@ -926,7 +929,94 @@  void smbios_entry_add(QemuOpts *opts)
			 smbios_type4_count++;
		 }

+	    switch (header->type) {
+	    case 0:
+		    tables_str_field_offset = g_malloc0(sizeof(int) * \
+											    TYPE_0_STR_FIELD_COUNT);
+		    tables_str_field_offset = (int []){\
+								    TYPE_0_STR_FIELD_OFFSET_VENDOR, \
+								    TYPE_0_STR_FIELD_OFFSET_BIOS_VERSION, \
+								    TYPE_0_STR_FIELD_OFFSET_BIOS_RELEASE_DATE};
+		    table_str_field_count = sizeof(tables_str_field_offset) / \
+								    sizeof(tables_str_field_offset[0]);
+		    break;
+	    case 1:
+		    tables_str_field_offset = g_malloc0(sizeof(int) * \
+											    TYPE_1_STR_FIELD_COUNT);
+		    tables_str_field_offset = (int []){
+								    TYPE_1_STR_FIELD_OFFSET_MANUFACTURER, \
+								    TYPE_1_STR_FIELD_OFFSET_PRODUCT, \
+								    TYPE_1_STR_FIELD_OFFSET_VERSION, \
+								    TYPE_1_STR_FIELD_OFFSET_SERIAL, \
+								    TYPE_1_STR_FIELD_OFFSET_SKU, \
+								    TYPE_1_STR_FIELD_OFFSET_FAMILY};
+		    table_str_field_count = sizeof(tables_str_field_offset) / \
+								    sizeof(tables_str_field_offset[0]);
+		    break;
+	    case 2:
+		    tables_str_field_offset = g_malloc0(sizeof(int) * \
+											    TYPE_2_STR_FIELD_COUNT);
+		    tables_str_field_offset = (int []){\
+								    TYPE_2_STR_FIELD_OFFSET_MANUFACTURER, \
+								    TYPE_2_STR_FIELD_OFFSET_PRODUCT, \
+								    TYPE_2_STR_FIELD_OFFSET_VERSION, \
+								    TYPE_2_STR_FIELD_OFFSET_SERIAL, \
+								    TYPE_2_STR_FIELD_OFFSET_ASSET, \
+								    TYPE_2_STR_FIELD_OFFSET_LOCATION};
+		    table_str_field_count = sizeof(tables_str_field_offset) / \
+								    sizeof(tables_str_field_offset[0]);
+		    break;
+	    case 3:
+		    tables_str_field_offset = g_malloc0(sizeof(int) * \
+											    TYPE_3_STR_FIELD_COUNT);
+		    tables_str_field_offset = (int []){\
+								    TYPE_3_STR_FIELD_OFFSET_MANUFACTURER, \
+								    TYPE_3_STR_FIELD_OFFSET_VERSION, \
+								    TYPE_3_STR_FIELD_OFFSET_SERIAL, \
+								    TYPE_3_STR_FIELD_OFFSET_ASSET, \
+								    TYPE_3_STR_FIELD_OFFSET_SKU};
+		    table_str_field_count = sizeof(tables_str_field_offset) / \
+								    sizeof(tables_str_field_offset[0]);
+		    break;
+	    case 4:
+		    tables_str_field_offset = g_malloc0(sizeof(int) * \
+											    TYPE_4_STR_FIELD_COUNT);
+		    tables_str_field_offset = (int []){\
+								    TYPE_4_STR_FIELD_OFFSET_SOCKET, \
+								    TYPE_4_STR_FIELD_OFFSET_PROCESSOR_MANUFACTURER, \
+								    TYPE_4_STR_FIELD_OFFSET_PROCESSOR_VERSION, \
+								    TYPE_4_STR_FIELD_OFFSET_SERIAL, \
+								    TYPE_4_STR_FIELD_OFFSET_ASSET, \
+								   
 TYPE_4_STR_FIELD_OFFSET_PART};
+		    table_str_field_count = sizeof(tables_str_field_offset) / \
+								    sizeof(tables_str_field_offset[0]);
+		    break;
+	    case 17:
+		    tables_str_field_offset = g_malloc0(sizeof(int) * \
+											    TYPE_17_STR_FIELD_COUNT);
+		    tables_str_field_offset = (int []){\
+								    TYPE_17_STR_FIELD_OFFSET_DEVICE_LOCATOR, \
+								    TYPE_17_STR_FIELD_OFFSET_BANK_LOCATOR, \
+								    TYPE_17_STR_FIELD_OFFSET_MANUFACTURER, \
+								    TYPE_17_STR_FIELD_OFFSET_SERIAL, \
+								    TYPE_17_STR_FIELD_OFFSET_ASSET, \
+								    TYPE_17_STR_FIELD_OFFSET_PART};
+		    table_str_field_count = sizeof(tables_str_field_offset) / \
+								    sizeof(tables_str_field_offset[0]);
+		    break;
+	    default:
+		    break;
+	    }
+
+	    for (i = 0; i < table_str_field_count; i++) {
+		    if (*(uint8_t *)(smbios_tables + tables_str_field_offset[i]) > 0) {
+			    terminator_count = 1;
+			    break;
+		    }
+	    }
+
		 smbios_tables_len += size;
+	    smbios_tables_len += terminator_count;
		 if (size > smbios_table_max) {
			 smbios_table_max = size;
		 }
diff --git a/include/hw/smbios/smbios.h b/include/hw/smbios/smbios.h
index 1cd53cc..6d59c3d 100644
--- a/include/hw/smbios/smbios.h
+++ b/include/hw/smbios/smbios.h
@@ -267,4 +267,48 @@  void smbios_get_tables(const struct smbios_phys_mem_area *mem_array,
					    const unsigned int mem_array_size,
					    uint8_t **tables, size_t *tables_len,
					    uint8_t **anchor, size_t *anchor_len);
+
+#define TYPE_0_STR_FIELD_OFFSET_VENDOR 0x4
+#define TYPE_0_STR_FIELD_OFFSET_BIOS_VERSION 0x5
+#define TYPE_0_STR_FIELD_OFFSET_BIOS_RELEASE_DATE 0x8
+#define TYPE_0_STR_FIELD_COUNT 3
+
+#define TYPE_1_STR_FIELD_OFFSET_MANUFACTURER 0x4
+#define TYPE_1_STR_FIELD_OFFSET_PRODUCT 0x5
+#define TYPE_1_STR_FIELD_OFFSET_VERSION 0x6
+#define TYPE_1_STR_FIELD_OFFSET_SERIAL 0x7
+#define TYPE_1_STR_FIELD_OFFSET_SKU 0x19
+#define TYPE_1_STR_FIELD_OFFSET_FAMILY 0x1a
+#define TYPE_1_STR_FIELD_COUNT 6
+
+#define TYPE_2_STR_FIELD_OFFSET_MANUFACTURER 0x4
+#define TYPE_2_STR_FIELD_OFFSET_PRODUCT 0x5
+#define TYPE_2_STR_FIELD_OFFSET_VERSION 0x6
+#define TYPE_2_STR_FIELD_OFFSET_SERIAL 0x7
+#define TYPE_2_STR_FIELD_OFFSET_ASSET 0x8
+#define TYPE_2_STR_FIELD_OFFSET_LOCATION 0xa
+#define TYPE_2_STR_FIELD_COUNT 6
+
+#define TYPE_3_STR_FIELD_OFFSET_MANUFACTURER 0x4
+#define TYPE_3_STR_FIELD_OFFSET_VERSION 0x6
+#define TYPE_3_STR_FIELD_OFFSET_SERIAL 0x7
+#define TYPE_3_STR_FIELD_OFFSET_ASSET 0x8
+#define TYPE_3_STR_FIELD_OFFSET_SKU 0x14
+#define TYPE_3_STR_FIELD_COUNT 5
+
+#define TYPE_4_STR_FIELD_OFFSET_SOCKET 0x4
+#define TYPE_4_STR_FIELD_OFFSET_PROCESSOR_MANUFACTURER 0x7
+#define TYPE_4_STR_FIELD_OFFSET_PROCESSOR_VERSION 0x10
+#define TYPE_4_STR_FIELD_OFFSET_SERIAL 0x20
+#define TYPE_4_STR_FIELD_OFFSET_ASSET 0x21
+#define TYPE_4_STR_FIELD_OFFSET_PART 0x22
+#define TYPE_4_STR_FIELD_COUNT 6
+
+#define TYPE_17_STR_FIELD_OFFSET_DEVICE_LOCATOR 0x10
+#define TYPE_17_STR_FIELD_OFFSET_BANK_LOCATOR 0x11
+#define TYPE_17_STR_FIELD_OFFSET_MANUFACTURER 0x17
+#define TYPE_17_STR_FIELD_OFFSET_SERIAL 0x18
+#define TYPE_17_STR_FIELD_OFFSET_ASSET 0x19
+#define TYPE_17_STR_FIELD_OFFSET_PART 0x1a
+#define TYPE_17_STR_FIELD_COUNT 6
#endif /* QEMU_SMBIOS_H */