Re: [PATCH AUTOSEL for 4.15 142/189] firmware: dmi_scan: Fix handling of empty DMI strings

[Jean Delvare]

Hi Parag,

On Sun, 8 Apr 2018 21:21:19 -0400, Parag Warudkar wrote:
> On Sun, Apr 8, 2018 at 8:18 PM, Sasha Levin <Alexander.Levin@xxxxxxxxxxxxx>
> wrote:
> 
> > From: Jean Delvare <jdelvare@xxxxxxx>
> >
> > [ Upstream commit a7770ae194569e96a93c48aceb304edded9cc648 ]
> >
> > [snip]
> >  
> 
> 
> > * Strings starting with 8 spaces are all considered empty, even if
> >   non-space characters follow (sounds like a weird thing to do, but
> >   I have actually seen occurrences of this in DMI tables before.)
> >  
> 
> Unless I am misreading the patch we will now allocate memory for
> len(spaces)+len(string) for this case.

Correct.

> Have you seen BIOSes with lots of <space>string occurrences?

A fair number, yes, but most of them were thankfully not in strings we
are saving. Also note that most of them only have 1 or 2 leading
spaces, not 8+, so this commit makes no difference for them.

> If so what's the point of allocating memory for the leading spaces?

Presented like that, it sounds like we are silly. But look at things
the other way around: what's the point of storing these leading spaces
in the DMI table in the first place? Whenever this happens, the
hardware manufacturer is to blame, not us.

As much as possible, we should stick to the specification and assume
the other party does as well. Stripping the leading spaces would be
trivial, but hiding their mistakes does not help hardware manufacturers
in the long run anyway. If they can't see that it's wrong, it's harder
for them to fix it.

> IOW, what happens if we strip leading space before allocating?

At boot time, we save a few bytes of memory, on the affected systems
only. No code cost, but possibly some confusion as to why we strip
leading spaces (which are rare) but not trailing spaces (which in my
experience are more frequent.)

At run time, the exact matching of the affected DMI strings would be
less error-prone (although strings having leading spaces are likely to
also have trailing spaces, annihilating the benefits.) Non-exact
matching would be marginally faster, again only for the affected
systems.

As a conclusion, it's doable, but the benefit is very small and limited
to a few broken systems, and it has the downside of not discouraging
low-quality tables, so my position is that it's not worth it and not
desirable.

-- 
Jean Delvare
SUSE L3 Support