qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] smbios: catch zero-length strings
@ 2014-01-21 12:24 Gerd Hoffmann
  2014-01-21 13:26 ` [Qemu-devel] [SeaBIOS] " Markus Armbruster
  2014-01-21 19:52 ` Kevin O'Connor
  0 siblings, 2 replies; 4+ messages in thread
From: Gerd Hoffmann @ 2014-01-21 12:24 UTC (permalink / raw)
  To: seabios; +Cc: armbru, qemu-devel, Gerd Hoffmann

qemu may pass us zero-length strings for smbios fields, when starting
qemu this way  ...

	qemu -smbios type=1,version=,serial=test

... for example.

Today we don't specifically handle them and simply append them to the
string list.  Therefore we get two string-terminating zeros in a row.
Result is that we by accident create a end-of-entry marker in the middle
of the entry.

Fix this by handling zero-length strings like non-present strings.

Cc: armbru@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 src/fw/smbios.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/fw/smbios.c b/src/fw/smbios.c
index affb9be..d549329 100644
--- a/src/fw/smbios.c
+++ b/src/fw/smbios.c
@@ -133,20 +133,24 @@ get_external(int type, char **p, unsigned *nr_structs,
     do {                                                                \
         size = get_field(type, offsetof(struct smbios_type_##type,      \
                                         field), end);                   \
-        if (size > 0) {                                                 \
+        if (size == 1) {                                                \
+            /* zero-length string, skip to avoid bogous end marker */   \
+            p->field = 0;                                               \
+        } else if (size > 1) {                                          \
             end += size;                                                \
+            p->field = ++str_index;                                     \
         } else {                                                        \
             memcpy(end, def, sizeof(def));                              \
             end += sizeof(def);                                         \
+            p->field = ++str_index;                                     \
         }                                                               \
-        p->field = ++str_index;                                         \
     } while (0)
 
 #define load_str_field_or_skip(type, field)                             \
     do {                                                                \
         size = get_field(type, offsetof(struct smbios_type_##type,      \
                                         field), end);                   \
-        if (size > 0) {                                                 \
+        if (size > 1) {                                                 \
             end += size;                                                \
             p->field = ++str_index;                                     \
         } else {                                                        \
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [SeaBIOS] [PATCH] smbios: catch zero-length strings
  2014-01-21 12:24 [Qemu-devel] [PATCH] smbios: catch zero-length strings Gerd Hoffmann
@ 2014-01-21 13:26 ` Markus Armbruster
  2014-01-22  7:58   ` Gerd Hoffmann
  2014-01-21 19:52 ` Kevin O'Connor
  1 sibling, 1 reply; 4+ messages in thread
From: Markus Armbruster @ 2014-01-21 13:26 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: seabios, qemu-devel

Gerd Hoffmann <kraxel@redhat.com> writes:

> qemu may pass us zero-length strings for smbios fields, when starting
> qemu this way  ...
>
> 	qemu -smbios type=1,version=,serial=test
>
> ... for example.
>
> Today we don't specifically handle them and simply append them to the
> string list.  Therefore we get two string-terminating zeros in a row.
> Result is that we by accident create a end-of-entry marker in the middle
> of the entry.

And dmidecode screaming bloody murder :)  More detail than you probably
want to know at https://bugzilla.redhat.com/show_bug.cgi?id=1052837

> Fix this by handling zero-length strings like non-present strings.
>
> Cc: armbru@redhat.com
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

Spelling correction inline.

Reviewed-by: Markus Armbruster <armbru@redhat.com>

> ---
>  src/fw/smbios.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/src/fw/smbios.c b/src/fw/smbios.c
> index affb9be..d549329 100644
> --- a/src/fw/smbios.c
> +++ b/src/fw/smbios.c
> @@ -133,20 +133,24 @@ get_external(int type, char **p, unsigned *nr_structs,
>      do {                                                                \
>          size = get_field(type, offsetof(struct smbios_type_##type,      \
>                                          field), end);                   \
> -        if (size > 0) {                                                 \
> +        if (size == 1) {                                                \
> +            /* zero-length string, skip to avoid bogous end marker */   \

s/bogous/bogus/

> +            p->field = 0;                                               \
> +        } else if (size > 1) {                                          \
>              end += size;                                                \
> +            p->field = ++str_index;                                     \
>          } else {                                                        \
>              memcpy(end, def, sizeof(def));                              \
>              end += sizeof(def);                                         \
> +            p->field = ++str_index;                                     \
>          }                                                               \
> -        p->field = ++str_index;                                         \
>      } while (0)
>  
>  #define load_str_field_or_skip(type, field)                             \
>      do {                                                                \
>          size = get_field(type, offsetof(struct smbios_type_##type,      \
>                                          field), end);                   \
> -        if (size > 0) {                                                 \
> +        if (size > 1) {                                                 \
>              end += size;                                                \
>              p->field = ++str_index;                                     \
>          } else {                                                        \

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [SeaBIOS] [PATCH] smbios: catch zero-length strings
  2014-01-21 12:24 [Qemu-devel] [PATCH] smbios: catch zero-length strings Gerd Hoffmann
  2014-01-21 13:26 ` [Qemu-devel] [SeaBIOS] " Markus Armbruster
@ 2014-01-21 19:52 ` Kevin O'Connor
  1 sibling, 0 replies; 4+ messages in thread
From: Kevin O'Connor @ 2014-01-21 19:52 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: seabios, armbru, qemu-devel

On Tue, Jan 21, 2014 at 01:24:02PM +0100, Gerd Hoffmann wrote:
> qemu may pass us zero-length strings for smbios fields, when starting
> qemu this way  ...
> 
> 	qemu -smbios type=1,version=,serial=test
> 
> ... for example.
> 
> Today we don't specifically handle them and simply append them to the
> string list.  Therefore we get two string-terminating zeros in a row.
> Result is that we by accident create a end-of-entry marker in the middle
> of the entry.
> 
> Fix this by handling zero-length strings like non-present strings.

Looks okay to me.

I wish we could get all of the smbios stuff moved to qemu.

-Kevin

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [SeaBIOS] [PATCH] smbios: catch zero-length strings
  2014-01-21 13:26 ` [Qemu-devel] [SeaBIOS] " Markus Armbruster
@ 2014-01-22  7:58   ` Gerd Hoffmann
  0 siblings, 0 replies; 4+ messages in thread
From: Gerd Hoffmann @ 2014-01-22  7:58 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: seabios, qemu-devel

On Di, 2014-01-21 at 14:26 +0100, Markus Armbruster wrote:
> > +            /* zero-length string, skip to avoid bogous end marker
> */   \
> 
> s/bogous/bogus/

Applied spell fix and pushed.

cheers,
  Gerd

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-01-22  7:58 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-21 12:24 [Qemu-devel] [PATCH] smbios: catch zero-length strings Gerd Hoffmann
2014-01-21 13:26 ` [Qemu-devel] [SeaBIOS] " Markus Armbruster
2014-01-22  7:58   ` Gerd Hoffmann
2014-01-21 19:52 ` Kevin O'Connor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).