Messages in this thread | | | Subject | Re: [PATCH RFC] brcmfmac: sanitize DMI strings v2 | From | Hans de Goede <> | Date | Mon, 6 May 2019 18:05:15 +0200 |
| |
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@redhat.com> 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
| |