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: <D233KUGR81P5.1BJ8JSACE7C6A@kernel.org>
Date: Tue, 18 Jun 2024 13:14:03 +0200
From: "Michael Walle" <mwalle@...nel.org>
To: "Tudor Ambarus" <tudor.ambarus@...aro.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

Hi Tudor,

On Tue Jun 18, 2024 at 12:33 PM CEST, Tudor Ambarus wrote:
> 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.

Right. It's not because it will trigger the SFDP parsing, but
because that what was tested by Esben. We're lucky that this will
trigger the SFDP parsing ;) I'll explain that in more detail and add
a Link: to the bug report mail.

-michael

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


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