linux-kernel - Re: [PATCH v2 1/2] mtd: spi-nor: core: add flag for doing optional SFDP

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: <D1XWS6SSXALZ.3NKR71449UYAO@kernel.org>
Date: Wed, 12 Jun 2024 10:51:11 +0200
From: "Michael Walle" <mwalle@...nel.org>
To: "Esben Haabendal" <esben@...nix.com>, "Tudor Ambarus"
 <tudor.ambarus@...aro.org>
Cc: "Pratyush Yadav" <pratyush@...nel.org>, "Miquel Raynal"
 <miquel.raynal@...tlin.com>, "Richard Weinberger" <richard@....at>,
 "Vignesh Raghavendra" <vigneshr@...com>, <linux-mtd@...ts.infradead.org>,
 <linux-kernel@...r.kernel.org>, "Rasmus Villemoes"
 <rasmus.villemoes@...vas.dk>
Subject: Re: [PATCH v2 1/2] mtd: spi-nor: core: add flag for doing optional
 SFDP

Hi,

On Fri Jun 7, 2024 at 3:30 PM CEST, Esben Haabendal wrote:
> But other than avoiding the "magic flags decide whether we use SFDP",
> should I be doing anything different?
>
> I assume we should still be calling the default_init() fixup functions,
> both for manufacturer and flash level. Or should we leave this for the
> deprecated case only?
No, they have to be handled.

> If the semantics is basically the same as for the deprecated, why not
> simply change the implementation of the deprecated approach to what we
> need? So having 3 cases:
>
> (1) SFDP only [indicated by size==0]
> (2) static config only [indicated by no_sfdp_flags & SPI_NOR_SKIP_SFDP]
> (3) SFDP with fallback to static config [indicated with size!=0 and
>     !(no_sfdp_flags & SPI_NOR_SKIP_SFDP]
>
> Any reason that we should not be able to easily convert existing
> depracted flash info specifications to the new SFDP with fallback to
> static config?

IMHO the only difference is that dual/quad read will imply the sfdp
fallback. So maybe the function can really be renamed accordingly
and the "deprecated handling" is just that dual/quad read will set
the sfdp fallback flag.

> Also I am wondering if anyone can remember or otherwise figure out why
> we are doing this memcpy() dance with nor->params in
> spi_nor_sfdp_init_params_deprecated()? Why not simply call
> spi_nor_parse_sfdp() before
> spi_nor_no_sfdp_init_params()/spi_nor_manufacturer_init_params()?

spi_nor_parse_sfdp() will overwrite the parameters set by the former.
So if you'll swap the order, the latter (static) ones will overwrite
the the ones from sfdp.

The correct fix would be to make spi_nor_parse_sfdp() not fiddle
around the parameters if it might fail. The quick fix here might be
to push that memcpy into the parse function. But I haven't looked at
the code right now.

-michael

Download attachment "signature.asc" of type "application/pgp-signature" (298 bytes)