Re: [PATCH v2 3/3] mtd: rawnand: sunxi: select the packed H6/H616 OOB layout

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The free OOB layout for steps > 0 incorrectly overlaps with hardware ECC bytes, leading to uncorrectable data corruption.
- [High] Division by zero kernel panic when NAND_ECC_MAXIMIZE_STRENGTH is configured on 512-byte NAND pages.
--

commit 250f35cb46a45ae57aae72a174fbcad8b8b7a7e5
Author: James Hilliard <james.hilliard1@gmail.com>
Subject: mtd: rawnand: sunxi: select the packed H6/H616 OOB layout

This commit updates the sunxi NAND driver to support the randomized OOB
layout used by the H6/H616 controllers. It configures a separate protected
user-data length for each ECC step and allocates up to 16 bytes for step zero.

> diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> index c1246a9268ca5..1265d900fc276 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand.c
[ ... ]
> @@ -2111,6 +2126,10 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand,
>  				bytes -= 2;
>  
>  			bytes -= total_user_data_sz;
> +		} else if (sunxi_nand->randomized_oob) {
> +			total_user_data_sz =
> +				sunxi_nfc_h6_user_data_sz(nsectors);
> +			bytes -= total_user_data_sz;
>  		} else {
>  			/*
>  			 * User-data lengths are encoded in four-byte units. Reserve

[Severity: High]
This is a pre-existing issue, but I noticed a potential division by zero
kernel panic in sunxi_nand_hw_ecc_ctrl_init() when NAND_ECC_MAXIMIZE_STRENGTH
is configured on 512-byte NAND pages.

If a NAND device with a 512-byte page size is configured with the
nand-ecc-maximize property in the device tree, mtd->writesize will be 512.
This function forces ecc->size = 1024 and calculates:

nsectors = mtd->writesize / ecc->size;

This resolves to 0, and a few lines down the code executes:

bytes /= nsectors;

Would it be safer to gracefully fail (e.g., returning -EINVAL) instead of
allowing a fatal division by zero during device probe?

[Severity: Critical]
This is also a pre-existing issue, but I wanted to ask about the free OOB
layout defined in sunxi_nand_ooblayout_free(), which isn't modified by
this patch.

For sections greater than 0, the driver does:

oobregion->offset = sunxi_get_ecc_offset(sunxi_nand, ecc, section);
oobregion->length = user_data_sz;

This appears to incorrectly overlap with hardware ECC bytes, defining the MTD
free region exactly over the hardware ECC bytes instead of the user data
bytes.

Could this lead to unrecoverable data corruption if standard filesystems
(like JFFS2 or UBIFS) write metadata to the free OOB area, unknowingly
overwriting the ECC codes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904-submit-sunxi-nand-vendor-oob-layout-v1-v2-0-b12074f4aca7@gmail.com?part=3