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

[sashiko-bot@kernel.org]

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

commit 2ec6f1cdbda7137d56bda90c1d422ad8f955d1df
Author: Weibin Liu <liuwb@xiaopeng.com>

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

This commit adds a bounds check on the decoded bit error index in
jz4725b_correct() to prevent writing past the end of the data buffer when the
BCH controller reports an index >= params->size * 8, returning -EBADMSG
instead.

> diff --git a/drivers/mtd/nand/raw/ingenic/jz4725b_bch.c b/drivers/mtd/nand/raw/ingenic/jz4725b_bch.c
> index 2d0e0a2192ae4..727f135f381e4 100644
> --- a/drivers/mtd/nand/raw/ingenic/jz4725b_bch.c
> +++ b/drivers/mtd/nand/raw/ingenic/jz4725b_bch.c
> @@ -259,6 +259,11 @@ static int jz4725b_correct(struct ingenic_ecc *bch,
>  			bit = (reg & BCH_BHERR_INDEX0_MASK) >> BCH_BHERR_INDEX0_SHIFT;
>  		}
>  
> +		if (bit >= params->size * 8) {
> +			ret = -EBADMSG;
> +			goto out;
> +		}

[Severity: High]
Does returning -EBADMSG when bit >= params->size * 8 cause valid pages with a
correctable bitflip in the OOB ECC parity bytes to fail with an uncorrectable
ECC error?

In jz4725b_bch_reset(), the hardware decode count is programmed to cover both
the data area and the ECC parity area:

drivers/mtd/nand/raw/ingenic/jz4725b_bch.c:jz4725b_bch_reset() {
	...
	reg = params->size << BCH_BHCNT_ENC_COUNT_SHIFT;
	reg |= (params->size + params->bytes) << BCH_BHCNT_DEC_COUNT_SHIFT;
	writel(reg, bch->base + BCH_BHCNT);
	...
}

And jz4725b_correct() feeds both buf and ecc_code into the BCH engine as a
single codeword of (params->size + params->bytes) * 8 bits:

drivers/mtd/nand/raw/ingenic/jz4725b_bch.c:jz4725b_correct() {
	...
	jz4725b_bch_write_data(bch, buf, params->size);
	jz4725b_bch_write_data(bch, ecc_code, params->bytes);
	...
}

When a normal, correctable bitflip occurs on flash within the ECC parity bytes
themselves, the hardware reports a bit index in the range
[params->size * 8, (params->size + params->bytes) * 8) without setting
BCH_BHINT_UNCOR.

With this check, a bitflip in the parity region causes jz4725b_correct() to
abort the correction loop early (potentially leaving remaining data bitflips
in later BCH_BHERRn entries uncorrected) and return -EBADMSG to
ingenic_nand_ecc_correct(), incrementing mtd->ecc_stats.failed.

Should bit indices in the parity region simply skip modifying buf (while still
counting the bitflip or continuing the loop, similar to jz4740_ecc_correct()),
or only return -EBADMSG if bit >= (params->size + params->bytes) * 8?

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

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