Re: [PATCH 06/15] mtd: spi-nor: Fix WRSR with ODTR chips

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mtd: spi-nor: state leak in spi_nor_write_sr_ll() due to missing WEL cleanup
- [High] mtd: spi-nor: 16-bit status register writes erroneously rejected in 8-8-8 DTR mode
--

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

mtd: spi-nor: Fix WRSR with ODTR chips

The ODTR interface requires an even number of bytes in each section of
the operation. spi_nor_spimem_setup_op() takes care of the opcode phase,
but we have to adapt the rest of the operation ourself.

> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index cf23094021daa..381bbc13f82ff 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -791,6 +791,17 @@ static int spi_nor_write_sr_ll(struct spi_nor *nor, u8 opcode, const u8 *sr,
>  	if (nor->spimem) {
>  		struct spi_mem_op op = SPI_NOR_WRSR_OP(opcode,
>  						       nor->bouncebuf, len);
> +		if (nor->reg_proto == SNOR_PROTO_8_8_8_DTR) {
> +			if (len != 1)
> +				return -EOPNOTSUPP;

[Severity: High]
Does this early return leak the Write Enable Latch state?

In spi_nor_write_sr_ll(), spi_nor_write_enable() is called before this code
is reached, which transmits the WREN opcode to the flash chip and sets its
internal Write Enable Latch.

If len is not 1, this early return bypasses the execution of the intended WRSR
command and does not call spi_nor_write_disable() to manually clear the latch
bit.

Could this leave the flash globally write-enabled and vulnerable to accidental
data corruption?

[Severity: High]
Does this condition unintentionally reject 16-bit status register writes
in 8-8-8 DTR mode?

Callers like spi_nor_write_sr1_and_sr2() natively invoke spi_nor_write_sr_ll()
with len = 2 to write 16-bit status registers:

drivers/mtd/spi-nor/core.c:spi_nor_write_sr1_and_sr2() {
    ...
	if (params->opcodes.write_sr1_and_sr2)
		return spi_nor_write_sr_ll(nor,
					   params->opcodes.write_sr1_and_sr2,
					   sr, 2);
    ...
}

Returning -EOPNOTSUPP here blocks these valid operations without a fallback.

> +
> +			/*
> +			 * We don't want to write only one byte in DTR mode. So,
> +			 * duplicate the buffer and write 2.
> +			 */
> +			op.data.nbytes = 2;
> +			nor->bouncebuf[1] = nor->bouncebuf[0];
> +		}
>  
>  		spi_nor_spimem_setup_op(nor, &op, nor->reg_proto);
>

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