Re: [PATCH v2] mtd: spi-nor: Fix quad-enable for flashes with QER bit in SR1

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] spi_nor_read_sr1_and_sr2() leaves sr[1] uninitialized when read_sr2 is unsupported, leading to uninitialized stack memory use in callers and spurious -EIO errors.
--

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

mtd: spi-nor: Fix quad-enable for flashes with QER bit in SR1

Some flashes do set BFPT_DWORD15_QER_SR1_BIT6, which means they do not
have an SR2 to read from/write to. The generic QE helper has forgotten
about the Macronix case, which causes booting to fail. This fix makes
sure SR2 reads just return 0 if unsupported to accommodate this situation.

Link: https://lore.kernel.org/linux-mtd/178876719232.3543902.14451625037676421254.b4-ty@b4/T/#m5bc4ba6776436f2870ced0eb5789d229037ad840

> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index e2b6efafdd8d0..7bb197df5585b 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -864,7 +865,10 @@ int spi_nor_read_sr1_and_sr2(struct spi_nor *nor, u8 *sr)
>  	if (ret)
>  		return ret;
>  
> -	return spi_nor_read_sr2(nor, &sr[1]);
> +	if (nor->params->opcodes.read_sr2)
> +		ret = spi_nor_read_sr2(nor, &sr[1]);
> +
> +	return ret;
>  }

[Severity: High]
Does this code leave sr[1] uninitialized when read_sr2 is unsupported?

In spi_nor_write_sr1_and_sr2_and_check(), an uninitialized u8 tmp[2] is
passed to spi_nor_read_sr1_and_sr2():

drivers/mtd/spi-nor/core.c:spi_nor_write_sr1_and_sr2_and_check() {
	u8 tmp[2];
	...
	ret = spi_nor_read_sr1_and_sr2(nor, tmp);
	if (ret)
		return ret;

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

For chips without SR2, tmp[1] remains uninitialized stack garbage and the
comparison sr[1] != tmp[1] can evaluate to true, returning a spurious -EIO
error. This causes quad-enable and device probing to fail for the exact
flashes the patch is attempting to fix.

Also, does this affect spi_nor_otp_lock_sr2()?

drivers/mtd/spi-nor/otp.c:spi_nor_otp_lock_sr2() {
	u8 sr[2];
	...
	ret = spi_nor_read_sr1_and_sr2(nor, sr);
	if (ret)
		return ret;

	/* no need to write the register if region is already locked */
	if (sr[1] & lock_bit)
		...
}

Here, sr[1] contains garbage, which is evaluated in the lock condition
check and potentially modified and written back to the device.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-perso-fix-spi-nor-qe-mxic-v2-1-70c324e9f30e@bootlin.com?part=1

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