linux-kernel - Re: [PATCH v3 00/15] mtd: spi-nor: macronix: workaround for device id re-use

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: <2196b4e8-2c93-4a73-a717-28448d4668ab@linaro.org>
Date: Thu, 26 Sep 2024 11:47:33 +0100
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: Esben Haabendal <esben@...nix.com>, Michael Walle <mwalle@...nel.org>
Cc: Pratyush Yadav <pratyush@...nel.org>,
 Miquel Raynal <miquel.raynal@...tlin.com>,
 Richard Weinberger <richard@....at>, Vignesh Raghavendra <vigneshr@...com>,
 Nicolas Ferre <nicolas.ferre@...rochip.com>,
 Alexandre Belloni <alexandre.belloni@...tlin.com>,
 Claudiu Beznea <claudiu.beznea@...on.dev>, linux-mtd@...ts.infradead.org,
 linux-kernel@...r.kernel.org, Rasmus Villemoes <rasmus.villemoes@...vas.dk>,
 linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v3 00/15] mtd: spi-nor: macronix: workaround for device id
 re-use

Hi, Esben,

Thank you for the perseverance :D

On 9/26/24 8:56 AM, Esben Haabendal wrote:
> "Michael Walle" <mwalle@...nel.org> writes:
> 
>> Hi,
>>
>> On Thu Jul 11, 2024 at 3:00 PM CEST, Esben Haabendal wrote:
>>> As a consequence, the SPI_NOR_SKIP_SFDP flag is no more, and all
>>> drivers that have been doing optional SFDP is now marked explicitly to
>>> do that using the SPI_NOR_TRY_SFDP.
>>
>> First, I haven't looked at your patchset at the moment. But I'd like
>> to take the opportunity to discuss the following (and excuse me that
>> I didn't had this idea before all your work on this).
>>
>> First, I'd like to see it the other way around, that is, doing SFDP
>> by default and let the driver opt-out instead of opt-in. This will
>> also align with the current "SFDP only driver", i.e. if a flash is
>> not known we try SFDP anyway. Going forward, I'd say this is also
>> the sane behavior and we don't have to add any flags if someone want
>> to add support for an (older) flash with the same ID but without
>> SFDP. With the current approach, we'd have to add the TRY_SFDP flag.
>>
>> Now we might play it safe and add that SPI_NOR_SKIP_SFDP to any
>> flash which doesn't do the SFDP parsing (because it has size != 0
>> and not any of the magic flags set) - or we might just go ahead and
>> do the probing first for *any* flashes. Yes we might issue an
>> unsupported opcode, but we already do that with the generic SFDP
>> flash driver. So no big deal maybe (?). Vendors where we know for a
>> fact that they don't have any SFDP (Everspin I'm looking at you),
>> would of course set the SKIP_SFDP flag.

Issuing RDSFDP for flashes that don't support it shouldn't be too bad
indeed, it's not recommended, but it shall be fine. What I'm worried
about is flashes with wrong SFDP data (see all the SFDP fixup hooks that
we have). So my suggestion is to play it safe unless one of you guys
step up and commit that will fix or help fix each bug that results from
this.

I'd like you to also consider the SFDP versions and how many parameters
they are exposing. I can't tell exactly, but if I remember correctly,
rev A had 9 dwords for BFPT, revB 16, and revC and other more. If we
consider static init folowed by SFDP with rollback option, point 3/ in
your cover letter, are we sure that all the parameters that are
initialized before parsing SFDP are overwritten at SFDP parsing time? If
yes, we shall be fine.

Esben, to speed up the things on both ends, I recommend you for v4 to
draft just a core patch if you care, then you can handle the vendor
drivers. Or send us some pseudocode and we can talk on that.

Thanks,
ta