Re: [PATCH RFC] brcmfmac: sanitize DMI strings v2

[Hans de Goede]

Hi,

On 06-05-19 17:24, Victor Bravo wrote:
> On Mon, May 06, 2019 at 03:26:28PM +0300, Kalle Valo wrote:
> > Hans de Goede <hdegoede@xxxxxxxxxx> writes:
> >
> > > If we're going to do some filtering, then I suggest we play it safe and also
> > > disallow other chars which may be used as a separator somewhere, specifically
> > > ':' and ','.
> > >
> > > Currently upstream linux-firmware has these files which rely on the DMI
> > > matching:
> > >
> > > brcmfmac4330-sdio.Prowise-PT301.txt
> > > brcmfmac43430-sdio.Hampoo-D2D3_Vi8A1.txt
> > > brcmfmac43430a0-sdio.ONDA-V80 PLUS.txt
> > >
> > > The others are either part of the DMI override table for devices with unsuitable
> > > DMI strings like "Default String"; or are device-tree based.
> > >
> > > So as long as we don't break those 3 (or break the ONDA one but get a symlink
> > > in place) we can sanitize a bit more then just non-printable and '/'.
> > >
> > > Kalle, Arend, what is your opinion on this?
> > >
> > > Note I do not expect the ONDA V80 Plus to have a lot of Linux users,
> > > but it definitely has some.
> >
> > To me having spaces in filenames is a bad idea, but on the other hand we
> > do have the "don't break existing setups" rule, so it's not so simple. I
> > vote for not allowing spaces, I think that's the best for the long run,
> > but don't know what Arend thinks.
>
> I have found a fresh judicate on this:
> https://lkml.org/lkml/2018/12/22/221
>
> It seems clear that we have to support at least spaces for some time
> (maybe wih separate config option which will be deprecated but on by
> defaut until old files are considered gone).

Ah that issue, well that is not really comparable in that case a lot of
peoples setups were completely broken by that commit and it was a
quite surprising behavior change in a userspace facing API.

The nvram loading path already does 2 tries, I really don't want to
unnecessary complicate it with a third try.

The Onda V80 Plus is a X86 based Windows / Android dual boot tablet,
as said before I do not expect a ton of users to be running regular
Linux on it.

Given Kalle's clear preference for getting rid of the spaces lets
just do that. But first we must get a symlink added to linux-firmware
using the name with the _, newer kernels requiring a newer linux-firmware
to match is not unheard of AFAIK, so combined with the limited amount
of users I think this is a reasonable compromise.

Kalle, do you agree with getting the symlink added to linux-firmware
ASAP as a fix for the V80 Plus issue; or do you want to see a fallback
to the un-cleaned name as you suggested before ?

Regards,

Hans