[PATCH] mtd: rawnand: sunxi: reject invalid ECC step sizes

[PATCH] mtd: rawnand: sunxi: reject invalid ECC step sizes

[James Hilliard]

ECC maximization forces a 1024-byte ECC step and divides the
available OOB bytes by the number of steps. A NAND with a smaller
page therefore produces zero steps and a division by zero.

An explicitly configured ECC step which is larger than, or does not
divide, the page also produces an unusable step count before the NAND
core can diagnose the configuration.

Validate the step size at both points where the driver derives the
number of sectors and reject invalid geometries.

Fixes: 4796d8655915 ("mtd: nand: sunxi: Support ECC maximization")
Cc: stable@vger.kernel.org
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/mtd/nand/raw/sunxi_nand.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
index 45ccbce91551..ad314c0e30b2 100644
--- a/drivers/mtd/nand/raw/sunxi_nand.c
+++ b/drivers/mtd/nand/raw/sunxi_nand.c
@@ -2032,6 +2032,15 @@ static void sunxi_nand_detach_chip(struct nand_chip *nand)
 	sunxi_nand->user_data_bytes = NULL;
 }
 
+static int sunxi_nfc_ecc_steps(struct mtd_info *mtd, unsigned int step_size)
+{
+	if (!step_size || mtd->writesize < step_size ||
+	    mtd->writesize % step_size)
+		return -EINVAL;
+
+	return mtd->writesize / step_size;
+}
+
 static int sunxi_nfc_maximize_user_data(struct nand_chip *nand, uint32_t oobsize,
 					int ecc_bytes, int nsectors)
 {
@@ -2078,7 +2087,9 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand,
 		int bytes = mtd->oobsize;
 
 		ecc->size = 1024;
-		nsectors = mtd->writesize / ecc->size;
+		nsectors = sunxi_nfc_ecc_steps(mtd, ecc->size);
+		if (nsectors < 0)
+			return nsectors;
 
 		if (!nfc->caps->reg_user_data_len) {
 			/*
@@ -2163,7 +2174,9 @@ static int sunxi_nand_hw_ecc_ctrl_init(struct nand_chip *nand,
 	/* HW ECC always work with even numbers of ECC bytes */
 	ecc->bytes = ALIGN(ecc->bytes, 2);
 
-	nsectors = mtd->writesize / ecc->size;
+	nsectors = sunxi_nfc_ecc_steps(mtd, ecc->size);
+	if (nsectors < 0)
+		return nsectors;
 
 	/*
 	 * The rationale for variable data length is to prioritize maximum ECC
-- 
2.53.0

Re: [PATCH] mtd: rawnand: sunxi: reject invalid ECC step sizes

[Miquel Raynal]

On 11/08/2026 at 00:06:48 -06, James Hilliard <james.hilliard1@gmail.com> wrote:

> ECC maximization forces a 1024-byte ECC step and divides the
> available OOB bytes by the number of steps. A NAND with a smaller
> page therefore produces zero steps and a division by zero.

No, steps cannot be < 1. If they are, it's a bug that must be fixed.

> An explicitly configured ECC step which is larger than, or does not
> divide, the page also produces an unusable step count before the NAND
> core can diagnose the configuration.
>
> Validate the step size at both points where the driver derives the
> number of sectors and reject invalid geometries.
>
> Fixes: 4796d8655915 ("mtd: nand: sunxi: Support ECC maximization")
> Cc: stable@vger.kernel.org
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
>  drivers/mtd/nand/raw/sunxi_nand.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> index 45ccbce91551..ad314c0e30b2 100644
> --- a/drivers/mtd/nand/raw/sunxi_nand.c
> +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> @@ -2032,6 +2032,15 @@ static void sunxi_nand_detach_chip(struct nand_chip *nand)
>  	sunxi_nand->user_data_bytes = NULL;
>  }
>  
> +static int sunxi_nfc_ecc_steps(struct mtd_info *mtd, unsigned int step_size)
> +{
> +	if (!step_size || mtd->writesize < step_size ||
> +	    mtd->writesize % step_size)

I don't get it, mtd->writesize < step_size is conceptually
impossible. If that happens, it must be fixed earlier than that.

> +		return -EINVAL;
> +
> +	return mtd->writesize / step_size;
> +}
> +
>  static int sunxi_nfc_maximize_user_data(struct nand_chip *nand, uint32_t oobsize,
>  					int ecc_bytes, int nsectors)
>  {

Thanks,
Miquèl

Re: [PATCH] mtd: rawnand: sunxi: reject invalid ECC step sizes

[James Hilliard]

On Fri, Sep 4, 2026 at 9:25 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
>
> On 11/08/2026 at 00:06:48 -06, James Hilliard <james.hilliard1@gmail.com> wrote:
>
> > ECC maximization forces a 1024-byte ECC step and divides the
> > available OOB bytes by the number of steps. A NAND with a smaller
> > page therefore produces zero steps and a division by zero.
>
> No, steps cannot be < 1. If they are, it's a bug that must be fixed.

The zero step count is created by the Sunxi maximization path itself.
sunxi_nand_hw_ecc_ctrl_init() unconditionally sets ecc->size to 1024,
including for a 512-byte-page NAND, before calculating nsectors.

The controller attach callback runs before nand_scan_tail(), so this
division happens before the generic ECC handling can reject the
configuration or fall back to software ECC.

For this case, would you expect the Sunxi maximization path to retain a
512-byte ECC step on 512-byte-page NANDs, rather than selecting 1024 and
then validating the resulting step count?

> > An explicitly configured ECC step which is larger than, or does not
> > divide, the page also produces an unusable step count before the NAND
> > core can diagnose the configuration.
> >
> > Validate the step size at both points where the driver derives the
> > number of sectors and reject invalid geometries.
> >
> > Fixes: 4796d8655915 ("mtd: nand: sunxi: Support ECC maximization")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> > ---
> >  drivers/mtd/nand/raw/sunxi_nand.c | 17 +++++++++++++++--
> >  1 file changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/sunxi_nand.c b/drivers/mtd/nand/raw/sunxi_nand.c
> > index 45ccbce91551..ad314c0e30b2 100644
> > --- a/drivers/mtd/nand/raw/sunxi_nand.c
> > +++ b/drivers/mtd/nand/raw/sunxi_nand.c
> > @@ -2032,6 +2032,15 @@ static void sunxi_nand_detach_chip(struct nand_chip *nand)
> >       sunxi_nand->user_data_bytes = NULL;
> >  }
> >
> > +static int sunxi_nfc_ecc_steps(struct mtd_info *mtd, unsigned int step_size)
> > +{
> > +     if (!step_size || mtd->writesize < step_size ||
> > +         mtd->writesize % step_size)
>
> I don't get it, mtd->writesize < step_size is conceptually
> impossible. If that happens, it must be fixed earlier than that.

An explicitly configured step reaches the controller attach callback
after the NAND geometry has been detected but before nand_scan_tail().
The Sunxi binding permits step sizes of 512 and 1024 without constraining
them against the detected page size.

Could you clarify where you expect this invariant to be established:
in the raw NAND core before ->attach_chip(), or in the Sunxi driver when
it selects or consumes the ECC configuration?

> > +             return -EINVAL;
> > +
> > +     return mtd->writesize / step_size;
> > +}
> > +
> >  static int sunxi_nfc_maximize_user_data(struct nand_chip *nand, uint32_t oobsize,
> >                                       int ecc_bytes, int nsectors)
> >  {
>
> Thanks,
> Miquèl