Re: [PATCH 13/15] mtd: spi-nor: winbond: Add support for W35T51NW-C/E

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mtd: spi-nor: uninitialized memory read in write_sr1_and_sr2_and_check_careful()
--

commit db0463242c1b895f1a8729e983049e5010571c6d
Author: Miquel Raynal <miquel.raynal@bootlin.com>

mtd: spi-nor: winbond: Add support for W35T51NW-C/E

This commit adds support for the Winbond W35T51NW-C/E SPI-NOR flash
devices. It defines their JEDEC ID and configures features like block
protection and locking by setting SPI_NOR_HAS_LOCK and related flags.

> diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
> index 05a80af3a6b93..0e350dcfbb755 100644
> --- a/drivers/mtd/spi-nor/winbond.c
> +++ b/drivers/mtd/spi-nor/winbond.c
[ ... ]
> @@ -633,6 +633,11 @@ static const struct flash_info winbond_nor_parts[] = {
>  		.id = SNOR_ID(0xef, 0x5b, 0x19),
>  		.flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6 |
>  			 SPI_NOR_4BIT_BP,
> +	}, {
> +		/* W35T51NW-C/E */
> +		.id = SNOR_ID(0xef, 0x5b, 0x1a),
> +		.flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_TB_SR_BIT6 |
> +			 SPI_NOR_4BIT_BP,

[Severity: High]
Does enabling SPI_NOR_HAS_LOCK on this chip lead to an uninitialized memory
read and random failures during lock/unlock operations?

Since W35T chips disable reading SR2 via winbond_w35txxnw_nor_late_init()
by setting nor->params->opcodes.read_sr2 = 0, lock operations eventually reach
spi_nor_write_sr1_and_sr2_and_check_careful() with an uninitialized buffer:

drivers/mtd/spi-nor/swp.c:spi_nor_write_sr1_and_sr2_and_check_careful() {
	u8 tmp[2];
	int ret;
	...
	ret = spi_nor_read_sr1_and_sr2_careful(nor, tmp);
	if (ret)
		return ret;

	if (sr[0] != tmp[0] || sr[1] != tmp[1])
		return -EIO;
}

Because read_sr2 is 0, spi_nor_read_sr2_careful() safely returns 0 but leaves
tmp[1] completely uninitialized.

When the code unconditionally evaluates if (sr[1] != tmp[1]), tmp[1] contains
stack garbage, which will randomly not match sr[1]. Can this cause spurious
-EIO failures for lock and unlock functionality on these chips?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002-winbond-master-spi-nor-w35t-v1-0-a22a2f95c26e@bootlin.com?part=13

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/