linux-kernel - Re: [RFC PATCH v4 3/5] PCI: Check platform specific ECAM quirks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKv+Gu9=0=qw70o1aYNg=tDyv4TguiG2WwXNK8RbWM1-ZcabUg@mail.gmail.com>
Date:	Wed, 29 Jun 2016 15:56:50 +0200
From:	Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:	Christopher Covington <cov@...eaurora.org>
Cc:	Tomasz Nowicki <tn@...ihalf.com>, Duc Dang <dhdang@....com>,
	Bjorn Helgaas <helgaas@...nel.org>,
	Arnd Bergmann <arnd@...db.de>,
	Will Deacon <will.deacon@....com>,
	Catalin Marinas <catalin.marinas@....com>,
	Rafael Wysocki <rafael@...nel.org>,
	Hanjun Guo <hanjun.guo@...aro.org>,
	Lorenzo Pieralisi <Lorenzo.Pieralisi@....com>,
	Sinan Kaya <okaya@...eaurora.org>,
	Jayachandran C <jchandra@...adcom.com>,
	Robert Richter <robert.richter@...iumnetworks.com>,
	Marcin Wojtas <mw@...ihalf.com>,
	Liviu Dudau <Liviu.Dudau@....com>,
	David Daney <ddaney@...iumnetworks.com>,
	Yijing Wang <wangyijing@...wei.com>,
	Mark Salter <msalter@...hat.com>, linux-pci@...r.kernel.org,
	linux-arm <linux-arm-kernel@...ts.infradead.org>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Linaro ACPI Mailman List <linaro-acpi@...ts.linaro.org>,
	Jon Masters <jcm@...hat.com>,
	Andrea Gallo <andrea.gallo@...aro.org>,
	Jeremy Linton <jeremy.linton@....com>,
	Dongdong Liu <liudongdong3@...wei.com>,
	Gabriele Paoloni <gabriele.paoloni@...wei.com>,
	Jeffrey Hugo <jhugo@...eaurora.org>
Subject: Re: [RFC PATCH v4 3/5] PCI: Check platform specific ECAM quirks

On 29 June 2016 at 15:34, Christopher Covington <cov@...eaurora.org> wrote:
> Hi Tomasz,
>
> On 06/29/2016 06:48 AM, Tomasz Nowicki wrote:
>> On 28.06.2016 18:12, Duc Dang wrote:
>>> On Tue, Jun 28, 2016 at 6:04 AM, Christopher Covington
>>> <cov@...eaurora.org> wrote:
>>>> Hi Tomasz,
>
>>>> Ard's comments on v3 included:
>>>>
>>>> "... exact OEM table/rev id matches ..."
>>>> "... substring match ... out of the question ..."
>
> Digging through the archives I see Jon Master commented earlier to "be
> careful with substring match".
>
>>> I think having OEM Table ID as "PLAT " and then "PLAT2 " (the the
>>> next version of the SoC) is common. So yes, matching full string is
>>> better as we can use "PLAT2 " in MCFG table and not worry about the
>>> "PLAT" sub-string match causes the quirk to be applied
>>> unintentionally.
>
>> Note that platforms already shipped where OEM string has no padding will
>
> I'm confused by this statement. OEMID is defined as 6 bytes long and OEM
> Table ID as 8 bytes long in the ACPI specification. As far as I can
> tell, if your string isn't exactly that long, padding up to that length
> is required.
>
>> have change the firmware or add 0 padding to our quirk array IDs.
>
> The fixed 6 or 8 character string compare, as used v2 of this patchset,
> will be compatible with existing firmware as best I can tell. Adding
> padding to the quirk array IDs is exactly what I'm suggesting, although
> all the strings I've seen are space padded rather than null padded.
>

I don't think any interpretation of the 6 or 8 byte wide OEM fields is
necessary to be able to match it against a list of known values as
used by the quirky platforms. We need an exact match against whatever
we know is in the table of an affected system, and whether a space
qualifies as padding or as a character is irrelevant.

> Matches:
> {"APM   ", "XGENE   ", 1}
> {"CAVIUM", "THUNDERX", 1}
> {"HISI  ", "HISI-D02", 1}
> {"HISI  ", "HISI-D03", 1}
> {"QCOM  ", "QDF2432 ", 1}
>

I would not mind listing these as

{ { 'A','P','M',' ',' ',' ',' '}, {'X','G','E','N','E',' ',' ',' '}, 1}
...

just to stress that we are not dealing with C strings (and to avoid
having to deal with the implicit NUL terminator).
That also means memcmp() with a fixed length is the most appropriate
to perform the comparison

> Given the above tuples, won't accidentally match:
> (guessing at possible future ids)
> {"APM   ", "XGENEi  ", 1}
> {"CAVIUM", "THUNDERX", i} i != 1
> {"CAVIUM", "THUNDERi", 1}
> {"CAVIUM", "THUNDRXi", 1}
> {"HISI  ", "HISI-D0i", 1} i != 2 && i != 3
> {"QCOM  ", "QDF24ij ", 1} i != 3 && j != 2
>
> References for APM, HiSilicon IDs:
> https://lists.linaro.org/pipermail/linaro-acpi/2016-June/007108.html
> https://lists.linaro.org/pipermail/linaro-acpi/2016-June/007043.html
>
> Thanks,
> Cov
>
> --
> Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project