[PATCH v2] smbios: Fix table when no string is present

[PATCH v2] smbios: Fix table when no string is present

[matthias.bgg at kernel.org]

From: Matthias Brugger <mbrugger@suse.com>

When no string is present in a table, next_ptr points to the same
location as eos. When calculating the string table length, we would only
reserve one \0. By spec a SMBIOS table has to end with two \0\0 when no
strings a present.

Signed-off-by: Matthias Brugger <mbrugger@suse.com>

---

Changes in v2:
- check in smbios_string_table_len if no string present and return value
  accordingly

 lib/smbios.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/smbios.c b/lib/smbios.c
index 9eb226ec9f..fd57d8694f 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -191,6 +191,10 @@ int smbios_update_version(const char *version)
  */
 static int smbios_string_table_len(const struct smbios_ctx *ctx)
 {
+	/* In case no string is defined we have to return two \0 */
+	if (ctx->next_ptr == ctx->eos)
+		return 2;
+
 	/* Allow for the final \0 after all strings */
 	return (ctx->next_ptr + 1) - ctx->eos;
 }
-- 
2.30.2

[PATCH v2] smbios: Fix table when no string is present

[Simon Glass]

Hi Matthias,

On Tue, 6 Apr 2021 at 21:04, <matthias.bgg@kernel.org> wrote:
>
> From: Matthias Brugger <mbrugger@suse.com>
>
> When no string is present in a table, next_ptr points to the same
> location as eos. When calculating the string table length, we would only
> reserve one \0. By spec a SMBIOS table has to end with two \0\0 when no
> strings a present.
>
> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
>
> ---
>
> Changes in v2:
> - check in smbios_string_table_len if no string present and return value
>   accordingly

This looks like a better idea to me. But where are the \0 bytes
actually written? Perhaps that should be in smbios_set_eos()?

>
>  lib/smbios.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/lib/smbios.c b/lib/smbios.c
> index 9eb226ec9f..fd57d8694f 100644
> --- a/lib/smbios.c
> +++ b/lib/smbios.c
> @@ -191,6 +191,10 @@ int smbios_update_version(const char *version)
>   */
>  static int smbios_string_table_len(const struct smbios_ctx *ctx)
>  {
> +       /* In case no string is defined we have to return two \0 */
> +       if (ctx->next_ptr == ctx->eos)
> +               return 2;
> +
>         /* Allow for the final \0 after all strings */
>         return (ctx->next_ptr + 1) - ctx->eos;
>  }
> --
> 2.30.2
>

Regards,
Simon

Re: [PATCH v2] smbios: Fix table when no string is present

[Heinrich Schuchardt]

On 4/7/21 18:14, Simon Glass wrote:
> Hi Matthias,
> 
> On Tue, 6 Apr 2021 at 21:04, <matthias.bgg@kernel.org> wrote:
>>
>> From: Matthias Brugger <mbrugger@suse.com>
>>
>> When no string is present in a table, next_ptr points to the same
>> location as eos. When calculating the string table length, we would only
>> reserve one \0. By spec a SMBIOS table has to end with two \0\0 when no
>> strings a present.
>>
>> Signed-off-by: Matthias Brugger <mbrugger@suse.com>
>>
>> ---
>>
>> Changes in v2:
>> - check in smbios_string_table_len if no string present and return value
>>    accordingly
> 
> This looks like a better idea to me. But where are the \0 bytes
> actually written? Perhaps that should be in smbios_set_eos()?

We have defined SMBIOS_STRUCT_EOS_BYTES as 2 and we have an array
char eos[SMBIOS_STRUCT_EOS_BYTES];
at the end of each SMBIOS structure.

We call memset(t, 0, sizeof(struct smbios_typeX)); for each table.
This is enough to ensure two zero bytes exist if there are no strings.

Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>

> 
>>
>>   lib/smbios.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/lib/smbios.c b/lib/smbios.c
>> index 9eb226ec9f..fd57d8694f 100644
>> --- a/lib/smbios.c
>> +++ b/lib/smbios.c
>> @@ -191,6 +191,10 @@ int smbios_update_version(const char *version)
>>    */
>>   static int smbios_string_table_len(const struct smbios_ctx *ctx)
>>   {
>> +       /* In case no string is defined we have to return two \0 */
>> +       if (ctx->next_ptr == ctx->eos)
>> +               return 2;
>> +
>>          /* Allow for the final \0 after all strings */
>>          return (ctx->next_ptr + 1) - ctx->eos;
>>   }
>> --
>> 2.30.2
>>
> 
> Regards,
> Simon
>