| Message ID | 20210406090435.19357-1-matthias.bgg@kernel.org |
|---|---|
| State | Accepted, archived |
| Commit | e31efe50b5cce46a2ee68c96db445e0a3d481aa8 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [v2] smbios: Fix table when no string is present | expand |
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
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 >
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; }