Project

General

Profile

Actions

Bug #14313

closed

smbios(1) does not check for success on libsmbios calls

Added by Robert Mustacchi 10 months ago. Updated 9 months ago.

Status:
Closed
Priority:
Normal
Category:
cmd - userland programs
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:
External Bug:

Description

Many libsmbios invocations from smbios(1) do not actually check for success. This was made a bit more obvious by recent work. Take for example the case of #14312. Here we have a table that is not the correct length for the smbios version because it is missing the OEM data field. As of the work done in #14233 and #14234, the library routine smbios_info_chassis actually detects this case now, while it didn't previously. This results in us return an error. However, the smbios(1) implementation just simply casts this to void and ignores it and proceeds to try to print things. At this point it just drives on and prints it assuming it had valid data. While it's true that we used to not check this particular part of table length for this entry, this is a general problem and we really shouldn't be ignoring all these errors and assuming they can't happen. If we have what we believe is a bad table it is up to us to say so.

Actions #1

Updated by Electric Monk 10 months ago

  • Gerrit CR set to 1877
Actions #2

Updated by Robert Mustacchi 10 months ago

Because the smbios test suite does not use the smbios(1) command, I went through and tested this manually in a few ways:

The first thing I did was to test this against the broken chassis table from #14312. Rather than just driving on with stack garbage, we now see:

smbios -t3 ~/smbios.out 
ID    SIZE TYPE
4     44   SMB_TYPE_CHASSIS (type 3) (system enclosure or chassis)

  Manufacturer: illumos
  Version: 1.0
  Serial Number: None
  Asset Tag: None

failed to read chassis information: Structure table is shorter than expected

The next thing I did was to figure out a few good ways to regression test this. Unfortunately the smbios test suite doesn't actually test the smbios(1) command at this time. So instead I took the smbios tables from a few systems and ran the before/after just through smbios > file and then diffed the two. Except in the case of the table from a bhyve instance impacted by #14312 , all data was the same both times.

Actions #3

Updated by Electric Monk 9 months ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit a7aaa5137d50ab434a012fa67192f75b383f3d48

commit  a7aaa5137d50ab434a012fa67192f75b383f3d48
Author: Robert Mustacchi <rm@fingolfin.org>
Date:   2021-12-24T18:31:11.000Z

    14313 smbios(1) does not check for success on libsmbios calls
    Reviewed by: Yuri Pankov <ypankov@tintri.com>
    Reviewed by: Andy Fiddaman <andy@omnios.org>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Approved by: Gordon Ross <gordon.w.ross@gmail.com>

Actions

Also available in: Atom PDF