Re: [dmidecode] Bug: dmidecode -ut4 segfaults

[Jerry Hoemann]

On Mon, Jan 18, 2021 at 01:49:37PM +0100, Jean Delvare wrote:
> Hi Jerry,
> 
> On Mon, 18 Jan 2021 01:26:10 -0700, Jerry Hoemann wrote:
> > On current dmidecode I build, I am seeing core dumps (SEGFAULT) when 
> > running:
> > 
> >     ./dmidecode -ut4 --from-dump _file_
> > 
> > 
> > For most, but not all, of my saved HPE dmi binary dumps.
> > 
> > e.g. the DL385G10.dmi I sent you earlier segfaults.
> > 
> > The segfault is coming from dmidecode.c:5239
> > 
> >                 display = ((opt.type == NULL || opt.type[h.type])
> >                         && (opt.handle == ~0U || opt.handle == h.handle)
> >                         && !((opt.flags & FLAG_QUIET) && (h.type == 126 || 
> > h.type == 127))
> >                         && !opt.string);
> > 
> > When run under gdb, it appears that the "opt" structure was over
> > written by ascii characters.
> > 
> > I bisected the submits and the issue first appears in:
> > 
> >     da06888 dmidecode: Use the print helpers in dump mode too
> > 
> > 
> > I haven't had a chance to dig into submit(da06888) so have not RCA'd
> > the issue.
> > 
> > Can you reproduce in your environment?
> 
> Thanks for reporting. Yes, I can reproduce it on my systems and with
> various dumps too. I confirm that reverting commit da06888 fixes the
> problem.
> 
> Valrgind complains about the following:
> 
> ==13097== Invalid read of size 1
> ==13097==    at 0x40B0B6: dmi_table_decode (dmidecode.c:5239)
> ==13097==    by 0x40B5A7: dmi_table (dmidecode.c:5397)
> ==13097==    by 0x40BAE8: smbios_decode (dmidecode.c:5522)
> ==13097==    by 0x40C02D: main (dmidecode.c:5714)
> 
> which matches the lines of code you quoted above.
> 
> I can't really see how this specific line can crash, as the only
> dereferencing done in that statement, as far as I can see, is
> opt.type[h.type]. Given that h.type is an 8-bit entity and opt.type is

For me, the opt structure was being overwritten with ascii data.
This made opt.type a non-zero but otherwise invalid pointer.


> a 256-byte array, that should not possibly crash even if the values are
> wrong.
> 
> That being said, there is one very obvious bug in the output of
> dmidecode -u before the crash happens. The binary form of the string
> part of the structure is messed up:
> 
> Handle 0x0001, DMI type 4, 42 bytes
>       Header and Data:
>               04 2A 01 00 04 03 CD 02 A9 06 03 00 FF FB EB BF
>               01 89 64 00 28 0A 28 0A 41 21 03 00 04 00 05 00
>               03 05 06 02 02 04 04 00 CD 00
>       Strings:
>               49 6E 74 65 6C 28 52 29 20 43 6F 72 65 28 54 4D
>               49 6E 74 65 6C 28 52 29 20 43 6F 72 65 28 54 4D29 20 69 35 2D 
> 33 33 32 30 4D 20 43 50 55 20 40
>               49 6E 74 65 6C 28 52 29 20 43 6F 72 65 28 54 4D29 20 69 35 2D 
> 33 33 32 30 4D 20 43 50 55 20 4020 32 2E 36 30 47 48 7A 00
>               Intel(R) Core(TM) i5-3320M CPU @ 2.60GHz
> 
> As you can see, for the second and third lines, the hexadecimal values
> are appended to the values from the previous line, instead of replacing
> them. As these values are printed into a 48-byte buffer before being
> displayes, we are obviously over-runing the buffer and thus trashing the
> stack. A trashed stack can lead to pretty much anything bad, including
> what we are seeing. Fixing that should solve the problem.
> 
> What this means is that I really need to extend my test suite to cover
> the -u option. Such a regression should really have been caught before
> being committed :(
> 
> Working on a fix.
> 
> -- 
> Jean Delvare
> SUSE L3 Support

-- 

-----------------------------------------------------------------------------
Jerry Hoemann                  Software Engineer   Hewlett Packard Enterprise
-----------------------------------------------------------------------------