bugdmidecode - Bugs: bug #50022, dmidecode output both _SM_ and...

 
 

bug #50022: dmidecode output both _SM_ and _SM3_ - replicating the output

Submitter:  None
Submitted:  Wed 11 Jan 2017 11:08:43 AM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  None Status:  Fixed
Privacy:  Public Assigned to:  khali
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sat 08 Jul 2017 09:27:14 AM UTC, comment #12: 

Kernel fix is in:

firmware: dmi_scan: Look for SMBIOS 3 entry point first
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=c9aba14362a6eec583819ec8f4b872c1816f5cbe

Your other request is not related to dmidecode so it is off-topic here. If you want it to be looked into, either write to LKML (Cc me) or open a bug on bugzilla.kernel.org (assign to me.)

Jean Delvare <khali>
Group administrator
Wed 18 Jan 2017 10:08:21 AM UTC, comment #11: 

Just noticed you mentioned you will also suggest same fix for the kernel (scan ordering) - this is a greatly appriciated.

It also worth expanding dmi_ids_string to 256bytes.  We have seen this overflowing in some cases.  Just my 2 cents.

Location: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/firmware/dmi_scan.c#n35

Anonymous
Wed 18 Jan 2017 10:01:05 AM UTC, comment #10: 

looks good.

Anonymous
Mon 16 Jan 2017 10:03:29 AM UTC, comment #9: 

OK, thanks. I have come up with something close but different. I like to keep the style and logic consistent with the rest of the code. Also there is no reason to look for SM before looking for DMI, as the latter is embedded into the former per specification, so SM will always show up first in memory anyway.

Please test and confirm it works for you. I'll post the patch to the devel list, and commit shortly if there are no objections. I'm also working on a similar fix for the kernel.

Jean Delvare <khali>
Group administrator
Thu 12 Jan 2017 02:27:18 PM UTC, comment #8: 

understood.  Attached.

(file #39448)

Anonymous
Thu 12 Jan 2017 02:06:01 PM UTC, comment #7: 

Please do not paste patches in comments. They are hard to read and impossible to apply that way. Instead, attach them as files. Thanks.

Jean Delvare <khali>
Group administrator
Thu 12 Jan 2017 11:41:12 AM UTC, comment #6: 

The sysfs interface used by dmidecode is not implemented in SLES 12 SP1, you will be able to use it if/when you upgrade to SLES 12 SP2.

Note that the sysfs interface in question is NOT related with CONFIG_DMI_SYSFS. I know it is confusing. CONFIG_DMI_SYSFS enables an "exploded" view of the DMI table in sysfs. The raw access to the whole table doesn't depend on this option and is always enabled (as long as your kernel is recent enough to support it, that is.)

The reason why you can't find the dmi-sysfs module is because it isn't supported by SUSE. It can be found in the kernel-default-extra package, which is not shipped by default in SLES. But again, this module is not relevant for what you are doing anyway, so you don't really care.

Jean Delvare <khali>
Group administrator
Thu 12 Jan 2017 11:06:15 AM UTC, comment #5: 

Suggested fix:

--- dmidecode.c.orig    2017-01-12 12:52:49.495159860 +0200
+++ dmidecode.c 2017-01-12 13:05:10.967165885 +0200
@@ -4878,30 +4878,23 @@ memory_scan:
                goto exit_free;
        }

+       found = 1;
        for (fp = 0; fp <= 0xFFF0; fp += 16)
-       {
                if (memcmp(buf + fp, "_SM3_", 5) == 0 && fp <= 0xFFE0)
-               {
                        if (smbios3_decode(buf + fp, opt.devmem, 0))
-                       {
-                               found++;
-                               fp += 16;
-                       }
-               }
-               else if (memcmp(buf + fp, "_SM_", 4) == 0 && fp <= 0xFFE0)
-               {
+                               goto done;
+
+       for (fp = 0; fp <= 0xFFF0; fp += 16)
+               if (memcmp(buf + fp, "_SM_", 4) == 0 && fp <= 0xFFE0)
                        if (smbios_decode(buf + fp, opt.devmem, 0))
-                       {
-                               found++;
-                               fp += 16;
-                       }
-               }
-               else if (memcmp(buf + fp, "_DMI_", 5) == 0)
-               {
+                               goto done;
+
+       for (fp = 0; fp <= 0xFFF0; fp += 16)
+               if (memcmp(buf + fp, "_DMI_", 5) == 0)
                        if (legacy_decode(buf + fp, opt.devmem, 0))
-                               found++;
-               }
-       }
+                               goto done;
+
+       found = 0;

 done:
        if (!found && !(opt.flags & FLAG_QUIET))

Anonymous
Thu 12 Jan 2017 10:03:46 AM UTC, comment #4: 

Hello Jean,

1. Unfortunately there is no order.  Depends on conditions the "HEADER" (SM3 vs. SM+DMI) may appear in different order.  So decoding "the first one found" doesn't work well for us :(.  The structures (the content, not the header) - are both placed just below the 4GB range.

2. you bring up a good point.  I  dont know why the sysfs doesn't come up (in /sys/firmware I see only acpi and memmap).  This system is heavily used, currently running "SUSE Linux Enterprise Server 12 SP1 - kernel 3.12.49-11-default", which per kernel config file has it as a module (CONFIG_DMI_SYSFS=m), but I can't see the module anywhere on the machine.  strange.  will look.


Anonymous
Thu 12 Jan 2017 09:01:42 AM UTC, comment #3: 

I agree it doesn't make sense to decode both.

Can I see the output of dmidecode (without your patch) on your system? Feel free to strip or obfuscate sensible information. I only want to see the versions and order of the entry points and locations of the entry points and tables, to have an idea how things are organized on such systems.

Also, may I ask why you have to use the /dev/mem interface? With a recent kernel you should be able to use the sysfs interface. It would be great if you could test that, because I wonder if the kernel picks the right one. If this is a UEFI system, it should now, but with a legacy BIOS, it depends on the order of the entry points, so it may pick the wrong one.

Jean Delvare <khali>
Group administrator
Thu 12 Jan 2017 01:18:48 AM UTC, comment #2: 

1 - yes, I do have a system (cant disclose vendor/model) with multiple entry points (2.1 and 3.0).  The reason is that the machine has >64K of table area, so the 2.1 is limited and had just part of the structs (for legacy OS support), while the 3.0 has all the structs.

2 - we tested the suggested fix, it worked.

3 - per spec, 3.0 must consist all 2.1 structs + additional ones, so atlead for me it doens't make sense to decode both.

Anonymous
Wed 11 Jan 2017 01:16:31 PM UTC, comment #1: 

This isn't really a bug, more of a design decision. The code was actually originally intended to try and decode all entry points found.

This was before the introduction of the new 64-bit SMBIOS 3.0 entry point. Before that, you were not supposed to have multiple entry points (and as a matter of fact I have never seen more than one entry point on any pre-SMBIOS-3.0 system) so reporting that case made some sense.

It was also before the sysfs interface to the DMI table was implemented. I agree it is kind of inconsistent that you only get one entry point from sysfs but may get several when reading from from /dev/mem.

So this design decision can certainly be revisited now. Stopping after the 1st entry point as you suggest would be consistent with what the Linux kernel is doing, but I suspect this is not the right way to handle it, as pointed out in bug #50024. We should search for an SMBIOS 3.0 entry point first to comply with the specification.

Do you actually have a system where the DMI tables can be read from /dev/mem, and there are 2 entry points, one for a 32-bit table and one for a 64-bit table? If so, which one is found first? I would like to get a binary dump to reproduce the problem, but I'm afraid "dmidecode --dump-bin" doesn't work properly on such a system?

Jean Delvare <khali>
Group administrator
Wed 11 Jan 2017 11:08:43 AM UTC, original submission:  

when working from memory, on system with both SM and SM3 dmidecode (latest) output the structures twice. 

This is due to a missing
  if (found) goto done;
at /dev/mem scan code.

more details:
- when reading sysfs, code searches for SM3, SM, DMI in that order and exit after the first found.
- when reading memory, code searches for SM3, SM, DMI in that order and does not evaluate "found" in the for loop - causing each one to be printed.  As DMI contined in SM it can not be printed if DMI found (fp is advanced by 16, skipping it), however both SM3 and SM will be printed.

By spec SM3 must contain on SM structs, so please fix. Fix is simple, add in line 4829:
  if (found) goto done;

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #39488:  look-for-SM3-first.patch added by khali (2KiB - text/x-patch - [PATCH] Only decode one DMI table)
file #39448:  dmidecode.patch added by None (965B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by khali (Updated the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-07-08 khali StatusNeed Info Fixed
        Open/ClosedOpen Closed
    2017-01-16 khali Attached File- Added look-for-SM3-first.patch, #39488
    2017-01-12 None Attached File- Added dmidecode.patch, #39448
    2017-01-12 khali StatusConfirmed Need Info
    2017-01-12 khali StatusNeed Info Confirmed
    2017-01-11 khali StatusNone Need Info
        Assigned toNone khali
    2017-01-11 khali PrivacyPrivate Public

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code