linux-kernel - Re: [PATCH] mtd: spi-nor: winbond: add Zetta ZD25Q128C support

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: <D3E0BKGQIZ9R.1ULZZ93E18VGM@kernel.org>
Date: Mon, 12 Aug 2024 16:36:11 +0200
From: "Michael Walle" <mwalle@...nel.org>
To: "Pratyush Yadav" <pratyush@...nel.org>
Cc: "Tudor Ambarus" <tudor.ambarus@...aro.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>
Subject: Re: [PATCH] mtd: spi-nor: winbond: add Zetta ZD25Q128C support

Hi,

> > Zetta normally uses BAh as its vendor ID. But for the ZD25Q128C they
> > took the one from Winbond and messed up the size parameters in SFDP.
> > Most functions seem compatible with the W25Q128, we just have to fix up
> > the size.
> >
> > Link: http://www.zettadevice.com/upload/file/20150821/DS_Zetta_25Q128_RevA.pdf
> > Link: https://www.lcsc.com/datasheet/lcsc_datasheet_2312081757_Zetta-ZD25Q128CSIGT_C19626875.pdf
> > Signed-off-by: Michael Walle <mwalle@...nel.org>
> > ---
> [...]
> > diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
> > index e065e4fd42a3..9f7ce5763e71 100644
> > --- a/drivers/mtd/spi-nor/winbond.c
> > +++ b/drivers/mtd/spi-nor/winbond.c
> > @@ -17,6 +17,31 @@
> >  		   SPI_MEM_OP_NO_DUMMY,					\
> >  		   SPI_MEM_OP_DATA_OUT(1, buf, 0))
> >  
> > +static int
> > +w25q128_post_bfpt_fixups(struct spi_nor *nor,
> > +			 const struct sfdp_parameter_header *bfpt_header,
> > +			 const struct sfdp_bfpt *bfpt)
> > +{
> > +	/*
> > +	 * Zetta ZD25Q128C is a clone of the Winbond device. But the encoded
> > +	 * size is really wrong. It seems that they confused Mbit with MiB.
> > +	 * Thus the flash is discovered as a 2MiB device.
> > +	 */
> > +	if (bfpt_header->major == SFDP_JESD216_MAJOR &&
> > +	    bfpt_header->minor == SFDP_JESD216_MINOR &&
> > +	    nor->params->size == SZ_2M &&
> > +	    nor->params->erase_map.regions[0].size == SZ_2M) {
> > +		nor->params->size = SZ_16M;
> > +		nor->params->erase_map.regions[0].size = SZ_16M;
> > +	}
>
> Since the size is 16 MiB for both Zetta and Winbond, why do you have
> these conditions here? Why not just do it unconditionally? What
> situation do you want to protect against?

Two things, I want to make sure, the values I'll overwrite (which
were set indirectly by the SFDP data I check here) are really the
"before" value I expect them to be. And secondly, I want to
fingerprint the SFDP as accurately as possible. I mean you wouldn't
need the SFDP version either. Think of a preparation for a possible
.match() callback in zetta.c. That would probably look like this,
i.e. check the flash id and these four conditions.

What are your concerns?

-michael

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