Re: [PATCH 2/2] mtd: rawnand: ingenic: validate BCH error index in jz4780_correct()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mtd: rawnand: ingenic: false -EBADMSG on parity bitflips in jz4780_correct()
--

commit 8de94e9615fc4f597d06591e322c73a02ac18b50
Author: Weibin Liu <liuwb@xiaopeng.com>

mtd: rawnand: ingenic: validate BCH error index in jz4780_correct()

This commit adds a bounds check on the BCH error index in jz4780_correct()
and returns -EBADMSG if the reported byte-pair index exceeds params->size.

> diff --git a/drivers/mtd/nand/raw/ingenic/jz4780_bch.c b/drivers/mtd/nand/raw/ingenic/jz4780_bch.c
> index 12b5b0484fe97..204bc853c1b42 100644
> --- a/drivers/mtd/nand/raw/ingenic/jz4780_bch.c
> +++ b/drivers/mtd/nand/raw/ingenic/jz4780_bch.c
> @@ -216,6 +216,11 @@ static int jz4780_correct(struct ingenic_ecc *bch,
>  						BCH_BHERR_MASK_SHIFT;
>  			index = (reg & BCH_BHERR_INDEX_MASK) >>
>  						BCH_BHERR_INDEX_SHIFT;
> +			if ((index * 2) + 1 >= params->size) {
> +				ret = -EBADMSG;
> +				goto out;
> +			}

[Severity: High]
Won't this introduce a regression when a correctable bitflip occurs in the
OOB ECC parity bytes?

In drivers/mtd/nand/raw/ingenic/jz4780_bch.c, jz4780_correct() writes both
the data buffer and the read ECC parity bytes to the BCH controller so it
decodes the entire codeword of params->size + params->bytes bytes:

	jz4780_bch_reset(bch, params, false);
	jz4780_bch_write_data(bch, buf, params->size);
	jz4780_bch_write_data(bch, ecc_code, params->bytes);

When a bitflip occurs on flash within the ECC parity bytes (and total
codeword bitflips are within params->strength), the hardware reports a
correctable error (BCH_BHINT_ERR set, BCH_BHINT_UNCOR clear) with index
pointing into the parity region [params->size, params->size + params->bytes),
such as index == 256 when params->size == 512.

With this check returning -EBADMSG and jumping to out whenever
(index * 2) + 1 >= params->size, any correctable bitflip in the parity area
aborts the loop (leaving any subsequent data-area error entries in
BCH_BHERRn uncorrected) and fails the page read as an uncorrectable ECC
error.

By comparison, the sibling driver in
drivers/mtd/nand/raw/ingenic/jz4740_ecc.c:jz4740_ecc_correct() skips
modifying buf when the index falls outside the data area without failing
the read:

		for (i = 0; i < error_count; ++i) {
			error = readl(ecc->base + JZ_REG_NAND_ERR(i));
			index = ((error >> 16) & 0x1ff) - 1;
			if (index >= 0 && index < params->size)
				jz_nand_correct_data(buf, index, error & 0x1ff);
		}

Should jz4780_correct() skip the buf XOR when index falls in the parity
region and only return -EBADMSG if the reported location lies beyond the
full codeword (params->size + params->bytes)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105040153.66597-1-zhugl3@xiaopeng.com?part=2

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