All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Lin Ma <lma@suse.com>
Cc: imammedo@redhat.com, qemu-devel@nongnu.org, famz@redhat.com,
	lersek@redhat.com
Subject: Re: [Qemu-devel] [PATCH] smbios: Add 1 terminator if there is any string field defined in given table.
Date: Thu, 10 Nov 2016 17:09:30 +0200	[thread overview]
Message-ID: <20161110170840-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <20160906082833.25428-1-lma@suse.com>

On Tue, Sep 06, 2016 at 04:28:33PM +0800, Lin Ma wrote:
> 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>

Seems to make sense superficially

Acked-by: Michael S. Tsirkin <mst@redhat.com>

Fam, would you like to take this?


> ---
>  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;
>          }
> 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 */
> -- 
> 2.9.2

  parent reply	other threads:[~2016-11-10 15:09 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-06  8:28 [Qemu-devel] [PATCH] smbios: Add 1 terminator if there is any string field defined in given table Lin Ma
2016-11-07 16:25 ` [Qemu-devel] 答复: " Lin Ma
2016-11-07 16:31   ` Daniel P. Berrange
2016-11-07 16:45     ` Lin Ma
2016-11-10 15:09 ` Michael S. Tsirkin [this message]
2016-11-10 17:18   ` [Qemu-devel] " Laszlo Ersek
2016-11-10 17:31     ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20161110170840-mutt-send-email-mst@kernel.org \
    --to=mst@redhat.com \
    --cc=famz@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=lersek@redhat.com \
    --cc=lma@suse.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.