[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/5] hw/smbios: report error if table size is too large
From: |
Igor Mammedov |
Subject: |
Re: [PATCH 2/5] hw/smbios: report error if table size is too large |
Date: |
Mon, 14 Sep 2020 10:02:52 +0200 |
On Tue, 8 Sep 2020 17:54:35 +0100
Daniel P. Berrangé <berrange@redhat.com> wrote:
> The SMBIOS 2.1 entry point uses a uint16 data type for reporting the
> total length of the tables. If the user passes -smbios configuration to
> QEMU that causes the table size to exceed this limit then various bad
> behaviours result, including
>
> - firmware hangs in an infinite loop
> - firmware triggers a KVM crash on bad memory access
> - firmware silently discards user's SMBIOS data replacing it with
> a generic data set.
>
> Limiting the size to 0xffff in QEMU avoids triggering most of these
> problems. There is a remaining bug in SeaBIOS which tries to prepend its
> own data for table 0, and does not check whether there is sufficient
> space before attempting this.
>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
even if we not going to add support for large entries,
this patch is good on it's own, so others won't have to
deal with debugging misconfiguration, and get a clear
error instead.
Michael,
Can you take this patch via your tree?
> ---
> hw/smbios/smbios.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/hw/smbios/smbios.c b/hw/smbios/smbios.c
> index 8450fad285..3c87be6c91 100644
> --- a/hw/smbios/smbios.c
> +++ b/hw/smbios/smbios.c
> @@ -365,6 +365,13 @@ static void smbios_register_config(void)
>
> opts_init(smbios_register_config);
>
> +/*
> + * The SMBIOS 2.1 "structure table length" field in the
> + * entry point uses a 16-bit integer, so we're limited
> + * in total table size
> + */
> +#define SMBIOS_21_MAX_TABLES_LEN 0xffff
> +
> static void smbios_validate_table(MachineState *ms)
> {
> uint32_t expect_t4_count = smbios_legacy ?
> @@ -375,6 +382,13 @@ static void smbios_validate_table(MachineState *ms)
> expect_t4_count, smbios_type4_count);
> exit(1);
> }
> +
> + if (smbios_ep_type == SMBIOS_ENTRY_POINT_21 &&
> + smbios_tables_len > SMBIOS_21_MAX_TABLES_LEN) {
> + error_report("SMBIOS 2.1 table length %zu exceeds %d",
> + smbios_tables_len, SMBIOS_21_MAX_TABLES_LEN);
> + exit(1);
> + }
> }
>
>