Hi Erwan, Jerry,

On Fri, 6 Nov 2020 22:21:20 +0100, Erwan Velu wrote:
> Le ven. 6 nov. 2020 à 20:38, Jerry Hoemann <jerry.hoem...@hpe.com> a écrit :
> > Why can you not put this change inside of dmi_decode_hp()?
>
> That's was my first try but it failed as the vendor is not yet known when
> we are parsing the DMI table.
> As the vendor is still undefined, the hpe quirks cannot be used.
> I suggested to Ehsan (you probably know from the chasing team) to move this
> OEM handle after the regular one so we have the vendor defined.
> That's what I saw on a XL225N with Bios 1.30.

Yes, I noticed this before on at least one HPE server (ProLiant DL385
Gen10 Plus with BIOS A42 v1.10). My naive expectations were that all
vendors would sort the DMI entries by type, so type 1 (from which we
get the vendor) would always come before OEM types (128-255). Alas, I
was wrong. In this specific case, the first OEM type already comes as
the 3rd entry, immediately after 2 PSU entries. The System Information
entry (type 1) comes much later in the table.

I would like to blame the vendors for doing weird things, however I
can't find anything in the specification that would prevent them from
putting the entries in the DMI table in any order they like. Therefore
I'm afraid we have to support all cases.

The only way I can think of to actually support that is to go for a
2-pass decoding. First pass will only look for entry type 1, record the
vendor and stop. Second pass will be identical to what we have today,
except that it won't set the vendor when seeing type 1, because that
was already done during pass 1. This will make decoding a bit slower
obviously, but thankfully most vendors *do* sort the entries by type
number, so type 1 should be found quickly and the first pass should be
pretty fast in most cases.

The following fix should do the trick:

From: Jean Delvare <jdelv...@suse.de>
Subject: dmidecode: Set vendor early

If OEM type entries come before the type 1 entry (System
Information), we aren't able to decode them because we don't know
the vendor yet. To fix this, figure out the vendor during a first
pass, and walk the table again once we have the information.

Signed-off-by: Jean Delvare <jdelv...@suse.de>
Reported-by: Erwan Velu <e.v...@criteo.com>
---
 dmidecode.c |   48 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)

--- dmidecode.orig/dmidecode.c  2020-11-09 10:27:50.685870685 +0100
+++ dmidecode/dmidecode.c       2020-11-09 11:02:16.204084481 +0100
@@ -5182,6 +5182,50 @@ static void dmi_table_decode(u8 *buf, u3
        u8 *data;
        int i = 0;
 
+       /* First pass: Save the vendor so that so that we can decode OEM types 
*/
+       data = buf;
+       while ((i < num || !num)
+           && data + 4 <= buf + len) /* 4 is the length of an SMBIOS structure 
header */
+       {
+               u8 *next;
+               struct dmi_header h;
+
+               to_dmi_header(&h, data);
+
+               /*
+                * If a short entry is found (less than 4 bytes), not only it
+                * is invalid, but we cannot reliably locate the next entry.
+                * Also stop at end-of-table marker if so instructed.
+                */
+               if (h.length < 4 ||
+                   (h.type == 127 &&
+                    (opt.flags & (FLAG_QUIET | FLAG_STOP_AT_EOT))))
+                       break;
+               i++;
+
+               /* Look for the next handle */
+               next = data + h.length;
+               while ((unsigned long)(next - buf + 1) < len
+                   && (next[0] != 0 || next[1] != 0))
+                       next++;
+               next += 2;
+
+               /* Make sure the whole structure fits in the table */
+               if ((unsigned long)(next - buf) > len)
+                       break;
+
+               /* Assign vendor for vendor-specific decodes later */
+               if (h.type == 1 && h.length >= 5)
+               {
+                       dmi_set_vendor(dmi_string(&h, data[0x04]));
+                       break;
+               }
+
+               data = next;
+       }
+
+       /* Second pass: Actually decode the data */
+       i = 0;
        data = buf;
        while ((i < num || !num)
            && data + 4 <= buf + len) /* 4 is the length of an SMBIOS structure 
header */
@@ -5241,10 +5285,6 @@ static void dmi_table_decode(u8 *buf, u3
                        break;
                }
 
-               /* assign vendor for vendor-specific decodes later */
-               if (h.type == 1 && h.length >= 5)
-                       dmi_set_vendor(dmi_string(&h, data[0x04]));
-
                /* Fixup a common mistake */
                if (h.type == 34)
                        dmi_fixup_type_34(&h, display);

-- 
Jean Delvare
SUSE L3 Support

_______________________________________________
https://lists.nongnu.org/mailman/listinfo/dmidecode-devel

Reply via email to