Re: [PATCH 1/7] spi: spi-qpic-snand: use the common qcom_nandc_check_flash_errors() function

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] spi: spi-qpic: OOB data corruption in qcom_spi_read_last_cw()
  [1]
--

commit e9db2cf0f483f41efa5a2350f20a79b1f0a4fe75
Author: Gabor Juhos <j4g8y7@gmail.com>

spi: spi-qpic-snand: use the common qcom_nandc_check_flash_errors() function

This commit replaces the local qcom_spi_check_raw_flash_errors() helper
with the common qcom_nandc_check_flash_errors() function from qpic_common.

> diff --git a/drivers/spi/spi-qpic-snand.c b/drivers/spi/spi-qpic-snand.c
> index 41685a286e381..981e16221b59e 100644
> --- a/drivers/spi/spi-qpic-snand.c
> +++ b/drivers/spi/spi-qpic-snand.c

[ ... ]

> @@ -651,7 +635,7 @@ static int qcom_spi_read_last_cw(struct qcom_nand_controller *snandc,
>  		return ret;
>  	}
>  
> -	ret = qcom_spi_check_raw_flash_errors(snandc, 1);
> +	ret = qcom_nandc_check_flash_errors(snandc, 1);
>  	if (ret)
>  		return ret;

[Severity: Medium]
This problem wasn't introduced by this patch, but can qcom_spi_read_last_cw()
return corrupted OOB data [1] when a raw OOB read spans beyond the bad
block marker?

qcom_spi_read_page() routes all raw OOB-only reads (snandc->qspi->oob_rw &&
snandc->qspi->raw_rw) to qcom_spi_read_last_cw():

drivers/spi/spi-qpic-snand.c:qcom_spi_read_page() {
	...
	if (snandc->qspi->oob_rw && snandc->qspi->raw_rw)
		return qcom_spi_read_last_cw(snandc, op);
	...
}

However, qcom_spi_read_last_cw() only reads the final codeword into
snandc->data_buffer and copies op->data.nbytes starting from bbpos:

drivers/spi/spi-qpic-snand.c:qcom_spi_read_last_cw() {
	...
	bbpos = mtd->writesize - ecc_cfg->cw_size * (num_cw - 1);
	...
	memcpy(op->data.buf.in, snandc->data_buffer + bbpos, op->data.nbytes);
}

Because physical OOB bytes are distributed across all codewords in a
multi-codeword page, a full raw OOB read (such as via nanddump -n --oob)
misses the OOB data from earlier codewords and returns shifted data from
the final codeword instead.

[1] https://sashiko.dev/#/bug/linux-d2e2b583-de83-4f91-a87f-a7c888dd45c9

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109-qpic-snand-use-common-functions-v1-0-1e8601e2db84@gmail.com?part=1

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