smbios(1) does not check for success on libsmbios calls
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.
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.
Updated by Electric Monk 9 months ago
- Status changed from New to Closed
- % Done changed from 0 to 100
commit a7aaa5137d50ab434a012fa67192f75b383f3d48 Author: Robert Mustacchi <email@example.com> Date: 2021-12-24T18:31:11.000Z 14313 smbios(1) does not check for success on libsmbios calls Reviewed by: Yuri Pankov <firstname.lastname@example.org> Reviewed by: Andy Fiddaman <email@example.com> Reviewed by: Toomas Soome <firstname.lastname@example.org> Approved by: Gordon Ross <email@example.com>