2023-02-22 01:55:12

by Tiezhu Yang

[permalink] [raw]
Subject: [PATCH] firmware: dmi: Change size of dmi_ids_string[] to 256

The current size of dmi_ids_string[] is 128, the BIOS date
can not be seen if the total string length of system vendor,
product name, board name, BIOS version and BIOS date is too
long to over 128, it is better and enough to change size of
dmi_ids_string[] to 256 for most cases.

Without this patch:

[ 0.000000] DMI: Loongson Loongson-3A5000-7A1000-1w-A2101/Loongson-LS3A5000-7A1000-1w-A2101, BIOS vUDK2018-LoongArch-V4.0.05132-beta10 12/13/202

With this patch:

[ 0.000000] DMI: Loongson Loongson-3A5000-7A1000-1w-A2101/Loongson-LS3A5000-7A1000-1w-A2101, BIOS vUDK2018-LoongArch-V4.0.05132-beta10 12/13/2022

Signed-off-by: Tiezhu Yang <[email protected]>
---
drivers/firmware/dmi_scan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 015c95a..37c39f9 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -32,7 +32,7 @@ static u8 smbios_entry_point[32];
static int smbios_entry_point_size;

/* DMI system identification string used during boot */
-static char dmi_ids_string[128] __initdata;
+static char dmi_ids_string[256] __initdata;

static struct dmi_memdev_info {
const char *device;
--
2.1.0



2024-04-29 12:47:49

by Jean Delvare

[permalink] [raw]
Subject: Re: [PATCH] firmware: dmi: Change size of dmi_ids_string[] to 256

Hi Tiezhu,

Sorry for the very late answer, somehow your messages slipped through
the cracks.

On Wed, 22 Feb 2023 09:55:01 +0800, Tiezhu Yang wrote:
> The current size of dmi_ids_string[] is 128, the BIOS date
> can not be seen if the total string length of system vendor,
> product name, board name, BIOS version and BIOS date is too
> long to over 128, it is better and enough to change size of
> dmi_ids_string[] to 256 for most cases.

In order to convince me that the size of this buffer needs to be
increased, one would have to provide a real world example with valid
DMI data where the output doesn't fit. However...

> Without this patch:
>
> [ 0.000000] DMI: Loongson Loongson-3A5000-7A1000-1w-A2101/Loongson-LS3A5000-7A1000-1w-A2101, BIOS vUDK2018-LoongArch-V4.0.05132-beta10 12/13/202
>
> With this patch:
>
> [ 0.000000] DMI: Loongson Loongson-3A5000-7A1000-1w-A2101/Loongson-LS3A5000-7A1000-1w-A2101, BIOS vUDK2018-LoongArch-V4.0.05132-beta10 12/13/2022

This example is apparently taken from an engineering sample with rather
"low quality" strings or invalid string indexes. Specifically:
* The product name and the board name are the exact same string.
* Both duplicate the system vendor name ("Loongson").
* The BIOS version includes the architecture name "LoongArch", which
seems unnecessarily verbose.

So my feeling is that the issue would be better addressed by fixing the
DMI data of your board than increasing the buffer size.

Do you have any production-grade DMI table with proper strings where
the buffer is still not large enough?

--
Jean Delvare
SUSE L3 Support

2024-04-30 14:50:03

by Tiezhu Yang

[permalink] [raw]
Subject: Re: [PATCH] firmware: dmi: Change size of dmi_ids_string[] to 256

On 4/29/24 20:44, Jean Delvare wrote:
> Hi Tiezhu,
>
> Sorry for the very late answer, somehow your messages slipped through
> the cracks.

It does not matter, thank you very much for your reply.

>
> On Wed, 22 Feb 2023 09:55:01 +0800, Tiezhu Yang wrote:
>> The current size of dmi_ids_string[] is 128, the BIOS date
>> can not be seen if the total string length of system vendor,
>> product name, board name, BIOS version and BIOS date is too
>> long to over 128, it is better and enough to change size of
>> dmi_ids_string[] to 256 for most cases.
>
> In order to convince me that the size of this buffer needs to be
> increased, one would have to provide a real world example with valid
> DMI data where the output doesn't fit. However...
>
>> Without this patch:
>>
>> [ 0.000000] DMI: Loongson Loongson-3A5000-7A1000-1w-A2101/Loongson-LS3A5000-7A1000-1w-A2101, BIOS vUDK2018-LoongArch-V4.0.05132-beta10 12/13/202
>>
>> With this patch:
>>
>> [ 0.000000] DMI: Loongson Loongson-3A5000-7A1000-1w-A2101/Loongson-LS3A5000-7A1000-1w-A2101, BIOS vUDK2018-LoongArch-V4.0.05132-beta10 12/13/2022
>
> This example is apparently taken from an engineering sample with rather
> "low quality" strings or invalid string indexes. Specifically:
> * The product name and the board name are the exact same string.
> * Both duplicate the system vendor name ("Loongson").
> * The BIOS version includes the architecture name "LoongArch", which
> seems unnecessarily verbose.
>
> So my feeling is that the issue would be better addressed by fixing the
> DMI data of your board than increasing the buffer size.

I agree with you.

>
> Do you have any production-grade DMI table with proper strings where
> the buffer is still not large enough?

Not yet.

I did not find the string length of system vendor, product name,
board name, BIOS version and BIOS date in the spec, I only see
"There is no limit on the length of each individual text string."
in "6.1.3 Text strings", so it may be better to increase the size
to avoid the potential problems in theory at least.

https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.7.0.pdf

Thanks,
Tiezhu



2024-05-03 09:56:48

by Jean Delvare

[permalink] [raw]
Subject: Re: [PATCH] firmware: dmi: Change size of dmi_ids_string[] to 256

On Tue, 30 Apr 2024 22:49:46 +0800, Tiezhu Yang wrote:
> I did not find the string length of system vendor, product name,
> board name, BIOS version and BIOS date in the spec, I only see
> "There is no limit on the length of each individual text string."
> in "6.1.3 Text strings", so it may be better to increase the size
> to avoid the potential problems in theory at least.

There is indeed no limit to individual string length, nor to the total
length of the string section of a DMI table entry. The only limit is
for the overall table, and that theoretical limit is 4 GB. This isn't a
reasonable size for dmi_ids_string[].

This is the reason why the size of dmi_ids_string[] is based on
real-world samples and I will only increase it if a production-grade
product has strings which do not fit within the current limit.

Note that nothing really bad will happen when the strings do not fit,
they will be simply clipped to the buffer size, and the same data can
be retrieved from sysfs or using the dmidecode user-space tool. So
there's no compelling reason to increase the buffer size in advance.

--
Jean Delvare
SUSE L3 Support