Testing stops at dmicheck test 1 of 4

Bug #1947786 reported by Sunny Wang
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
Firmware Test Suite
Fix Released
Undecided
Alex Hung

Bug Description

We found this issue on RPi4 8GB, Arm FVP, and some other ARM platforms. I tried 21.09.00 (the latest), 21.07.00 (the same base as ACS 3.0 FWTS), and 21.06.00 (the first version) live images and got the same result on RPi4 8GB with FW 1.31 and FW 1.19.

Two Notes here:
1. ACS 3.0 FWTS (21.07.00) doesn't have this issue, so this issue looks like related to the Linux kernel.
2. somehow this issue can't be reproduced on RPi4 4GB, so this may be only reproducible on the system that has more than 4 GB of memory.

dmicheck: DMI/SMBIOS table tests.
--------------------------------------------------------------------------------
Test 1 of 4: Find and test SMBIOS Table Entry Points.
This test tries to find and sanity check the SMBIOS data structures.
PASSED: Test 1, Found SMBIOS Table Entry Point at 0x37200000
SMBIOS Entry Point Structure:
  Anchor String : _SM_
  Checksum : 0xfc
  Entry Point Length : 0x1f
  Major Version : 0x03
  Minor Version : 0x03
  Maximum Struct Size : 0x0081
  Entry Point Revision : 0x00
  Formatted Area : 0x00 0x00 0x00 0x00 0x00
  Intermediate Anchor : _DMI_
  Intermediate Checksum : 0x6e
  Structure Table Length : 0x035f
  Structure Table Address: 0x371f0000
  # of SMBIOS Structures : 0x000f
  SMBIOS BCD Revision : 33

PASSED: Test 1, SMBIOS Table Entry Point Checksum is valid.
PASSED: Test 1, SMBIOS Table Entry Point Length is valid.
PASSED: Test 1, SMBIOS Table Entry Intermediate Anchor String _DMI_ is valid.
PASSED: Test 1, SMBIOS Table Entry Point Intermediate Checksum is valid.
<Testing stops here>

Revision history for this message
Sunny Wang (sunnywang-arm) wrote :
Revision history for this message
Sunny Wang (sunnywang-arm) wrote :
Revision history for this message
Sunny Wang (sunnywang-arm) wrote :
Revision history for this message
Sunny Wang (sunnywang-arm) wrote :
Revision history for this message
Alex Hung (alexhung) wrote :

A patch seemed to be submitted but not recieved.

Details @ https://lists.ubuntu.com/archives/fwts-devel/2021-October/013385.html

Changed in fwts:
assignee: nobody → Alex Hung (alexhung)
status: New → In Progress
Revision history for this message
Sunny Wang (sunnywang-arm) wrote :

Sorry for the confusion. The patch that has been submitted is not the fix for this issue. We still need help from FWTS experts.
Details @ https://lists.ubuntu.com/archives/fwts-devel/2021-October/013386.html

Revision history for this message
Alex Hung (alexhung) wrote :

Thanks for the clarification.

Revision history for this message
Alex Hung (alexhung) wrote :

I don't have access to RPi4 8G now. Let me see what I can do with this bug.

Revision history for this message
Alex Hung (alexhung) wrote :

This is not unique to RPi4 8G and can be reproduced on RPi4 4G.

$ sudo ./src/fwts dmicheck
Running 1 tests, results appended to results.log
Test: DMI/SMBIOS table tests.
  Find and test SMBIOS Table Entry Points. : 0.0% -
Caught SIGNAL 7 (Bus error), aborting.
Backtrace:
0x0000ffff8075e1e8 /home/alexhung/src/fwts/src/lib/src/.libs/libfwts.so.1(+0x1a1e8)
0x0000ffff807ff7dc linux-vdso.so.1(__kernel_rt_sigreturn+0)
0x0000ffff80543958 /lib/aarch64-linux-gnu/libc.so.6(+0x9b958)
0x0000aaaac44e719c /home/alexhung/src/fwts/src/.libs/fwts(+0x4719c)
0x0000aaaac44e7960 /home/alexhung/src/fwts/src/.libs/fwts(+0x47960)
0x0000ffff80763a6c /home/alexhung/src/fwts/src/lib/src/.libs/libfwts.so.1(+0x1fa6c)
0x0000ffff80765654 /home/alexhung/src/fwts/src/lib/src/.libs/libfwts.so.1(fwts_framework_args+0x714)
0x0000aaaac44b67a4 /home/alexhung/src/fwts/src/.libs/fwts(+0x167a4)
0x0000ffff804d2ffc /lib/aarch64-linux-gnu/libc.so.6(+0x2affc)
0x0000ffff804d30c8 /lib/aarch64-linux-gnu/libc.so.6(__libc_start_main+0x94)
0x0000aaaac44b79f0 /home/alexhung/src/fwts/src/.libs/fwts(+0x179f0)

Revision history for this message
G Edhaya Chandran (edhay) wrote :

From my analysis by adding debug prints, the bus error at this point:
https://git.launchpad.net/fwts/tree/src/dmi/dmicheck/dmicheck.c#n394

  if (table)
   memcpy(table, mem, length); //point of failure
  (void)fwts_munmap(mem, length);
  return table;

The root is to be further debugged.

Revision history for this message
Alex Hung (alexhung) wrote :

This is likely to be the results of the config "CONFIG_STRICT_DEVMEM" is enabled in kernel for aarch64 - at least this is the case for Ubuntu kernel used in fwts-live.

A previous patch tried to address this issue: https://lists.ubuntu.com/archives/fwts-devel/2015-October/006929.html; however, the lack of "_SM_" in "/sys/firmware/dmi/tables/smbios_entry_point" triggers this bug (it has _SM3_ instead).

fwts_safe_memread was used to detect SIGSEGV/SIGBUS when accessing which are not generated either.

One way to fix this is to surround the code by "#if !(FWTS_ARCH_AARCH64)", but there may be other better solutions too.

Revision history for this message
Alex Hung (alexhung) wrote :

Arm Base Boot requirement seems to explicitly suggest only SMBIOS 3 is supported

9.1 SMBIOS version
 ...
 Legacy SMBIOS tables and formats are not supported.

===========================
Is it a good idea to complete skip it like below?

static int dmicheck_test1(fwts_framework *fw)
{

#if !(FWTS_ARCH_AARCH64)
        if (smbios_entry_check(fw) != FWTS_ERROR) {
                smbios_found = true;
        }
#endif

        if (smbios30_entry_check(fw) != FWTS_ERROR) {
                smbios30_found = true;
        }
        ....

Revision history for this message
Sunny Wang (sunnywang-arm) wrote :

Wow, so quick! Thanks so much, Alex. We should have reported this issue earlier. Lesson learned :)

Completely skipping smbios_entry_check(fw) for FWTS_ARCH_AARCH64 looks good to me. I think there should be a rare case or no case that a non-BBR-compliant ARM system wants to run this FWTS dmicheck test.

Moreover, since memcpy would still cause the issue in the worst case, do we need to remove the code block below? Or do we need to skip it when "CONFIG_STRICT_DEVMEM" is enabled?

 mem = fwts_mmap(addr, length);
 if (mem != FWTS_MAP_FAILED) {
  /* Can we safely copy the table? */
  if (fwts_safe_memread((void *)mem, length) != FWTS_OK) {
   fwts_log_info(fw, "SMBIOS table at %p cannot be read", (void *)addr);
   (void)fwts_munmap(mem, length);
   return NULL;
  }
  table = malloc(length);
  if (table)
   memcpy(table, mem, length);
  (void)fwts_munmap(mem, length);
  return table;
 }

Revision history for this message
Alex Hung (alexhung) wrote :

Checking "CONFIG_STRICT_DEVMEM" was a solution I had considered too.

kernel config files may depend on distro. fwts-live uses /boot/config-`uname -r` and that's the patch will be based on.

Revision history for this message
Sunny Wang (sunnywang-arm) wrote :

Hi Alex,

Thanks for checking my comment. However, I don't understand your reply.

My question is "Do we need to make two changes below for this?"
1. The solution that you proposed. Completely skipping smbios_entry_check(fw) for FWTS_ARCH_AARCH64.
2. A fix for the potential issue. Skip fwts_mmap code block when "CONFIG_STRICT_DEVMEM" is enabled.

In other words, we definitely need #1. I'm just not sure if we need to do #2 as well. Sorry for the confusion.

I assume you will send a patch for this. If you need us to send a patch for this, feel free to let us know.

Revision history for this message
Alex Hung (alexhung) wrote (last edit ):

Current aarch64 are impacted by this because of #2 so I think combining #1 and #2 is a better solution. I will send out a patch for reviews later.

Revision history for this message
Alex Hung (alexhung) wrote :

A patch was sent for review: https://patchwork.<email address hidden>/

Revision history for this message
Alex Hung (alexhung) wrote :

V2 was sent for review: https://patchwork<email address hidden>/

Alex Hung (alexhung)
Changed in fwts:
status: In Progress → Fix Committed
Alex Hung (alexhung)
Changed in fwts:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.