linux-kernel - Re: [PATCH] mtd: spi-nor: winbond: fix w25q128 regression

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: <76f8be4e-3050-4ae6-93b4-9524a0689022@linaro.org>
Date: Tue, 18 Jun 2024 11:33:19 +0100
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: Michael Walle <mwalle@...nel.org>, Pratyush Yadav <pratyush@...nel.org>,
 Miquel Raynal <miquel.raynal@...tlin.com>,
 Richard Weinberger <richard@....at>, Vignesh Raghavendra <vigneshr@...com>,
 Linus Walleij <linus.walleij@...aro.org>
Cc: linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
 e9hack <e9hack@...il.com>
Subject: Re: [PATCH] mtd: spi-nor: winbond: fix w25q128 regression



On 6/10/24 8:48 AM, Michael Walle wrote:
> Commit 83e824a4a595 ("mtd: spi-nor: Correct flags for Winbond w25q128")

That commit did:
-       { "w25q128", INFO(0xef4018, 0, 64 * 1024, 256)
-               NO_SFDP_FLAGS(SECT_4K) },
+       { "w25q128", INFO(0xef4018, 0, 0, 0)
+               PARSE_SFDP
+               FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB) },

> removed the flags for non-SFDP devices. It was assumed that it wasn't in
> use anymore. This turned out to be wrong. Add the no_sfdp_flags as
> well as the size again.
> 
> Reported-by: e9hack <e9hack@...il.com>
> Fixes: 83e824a4a595 ("mtd: spi-nor: Correct flags for Winbond w25q128")
> Signed-off-by: Michael Walle <mwalle@...nel.org>
> ---
> Hartmut, Linus, could you please test it on your boards? Also, do
> you have a real name we should put in the Reported-by tag?
> 
> This will also need a manual backport to the stable kernels due to
> the new syntax. But that should be straight forward.
> ---
>  drivers/mtd/spi-nor/winbond.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
> index ca67bf2c46c3..6b6dec6f8faf 100644
> --- a/drivers/mtd/spi-nor/winbond.c
> +++ b/drivers/mtd/spi-nor/winbond.c
> @@ -105,7 +105,9 @@ static const struct flash_info winbond_nor_parts[] = {
>  	}, {
>  		.id = SNOR_ID(0xef, 0x40, 0x18),
>  		.name = "w25q128",
> +		.size = SZ_16M,
>  		.flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB,
> +		.no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,

and here you add dual and quad to trigger SFDP parsing I guess. All fine
if the old flash supports dual and quad read. But please update the
commit message describing the intention. With that ACK. Would be good to
have this merged soon.

>  	}, {
>  		.id = SNOR_ID(0xef, 0x40, 0x19),
>  		.name = "w25q256",