linux-kernel - Re: [PATCH v0] mtd: rawnand: marvell: fix 8bit ECC layouts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87qzuvrr74.fsf@bootlin.com>
Date: Wed, 22 Oct 2025 15:39:27 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Aryan Srivastava <aryan.srivastava@...iedtelesis.co.nz>
Cc: Richard Weinberger <richard@....at>,  Vignesh Raghavendra
 <vigneshr@...com>,  Elad Nachman <enachman@...vell.com>,
  linux-mtd@...ts.infradead.org,  linux-kernel@...r.kernel.org
Subject: Re: [PATCH v0] mtd: rawnand: marvell: fix 8bit ECC layouts

On 22/10/2025 at 14:37:52 +13, Aryan Srivastava <aryan.srivastava@...iedtelesis.co.nz> wrote:

> These were modified to appease a check in nand_scan_tail. This change led
> to the last spare bytes never being written to or read from.
>
> Modify the fix by restoring the layouts and setting ecc->steps to the
> full_chunk_cnt value in the 8 bit ECC cases (4k and 8k page size). This
> allows the driver to continue reading/writing all chunks while also
> passing the check in nand_scan_tail.
>
> Fixes: e6a30d0c48a1 ("mtd: rawnand: marvell: fix layouts")
> Signed-off-by: Aryan Srivastava <aryan.srivastava@...iedtelesis.co.nz>
> ---
>  drivers/mtd/nand/raw/marvell_nand.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/marvell_nand.c b/drivers/mtd/nand/raw/marvell_nand.c
> index 303b3016a070..c20feacbaca8 100644
> --- a/drivers/mtd/nand/raw/marvell_nand.c
> +++ b/drivers/mtd/nand/raw/marvell_nand.c
> @@ -292,10 +292,10 @@ static const struct marvell_hw_ecc_layout marvell_nfc_layouts[] = {
>  	MARVELL_LAYOUT( 2048,   512,  8,  2,  1, 1024,  0, 30,1024,64, 30),
>  	MARVELL_LAYOUT( 2048,   512,  16, 4,  4, 512,   0, 30,  0, 32, 30),
>  	MARVELL_LAYOUT( 4096,   512,  4,  2,  2, 2048, 32, 30,  0,  0,  0),
> -	MARVELL_LAYOUT( 4096,   512,  8,  4,  4, 1024,  0, 30,  0, 64, 30),
> +	MARVELL_LAYOUT( 4096,   512,  8,  5,  4, 1024,  0, 30,  0, 64, 30),
>  	MARVELL_LAYOUT( 4096,   512,  16, 8,  8, 512,   0, 30,  0, 32, 30),
>  	MARVELL_LAYOUT( 8192,   512,  4,  4,  4, 2048,  0, 30,  0,  0,  0),
> -	MARVELL_LAYOUT( 8192,   512,  8,  8,  8, 1024,  0, 30,  0, 160, 30),
> +	MARVELL_LAYOUT( 8192,   512,  8,  9,  8, 1024,  0, 30,  0, 160, 30),
>  	MARVELL_LAYOUT( 8192,   512,  16, 16, 16, 512,  0, 30,  0,  32,
> 30),
>  };
>  
> @@ -2286,7 +2286,16 @@ static int marvell_nand_hw_ecc_controller_init(struct mtd_info *mtd,
>  	}
>  
>  	mtd_set_ooblayout(mtd, &marvell_nand_ooblayout_ops);
> -	ecc->steps = l->nchunks;
> +
> +	/* Validity checks in nand_scan_tail assume even sized chunks, but in the case of 8bit
> +	 * ECC with 4k/8k page size the last chunk is spare data, which is not sized to the data
> +	 * chunks. Overwrite the ecc->steps to pass this validity check, while maintaining the
> +	 * correct number of chunks in-driver.
> +	 */

I am not a big fan of this approach, I wonder whether we should relax
the check in nand_scan_tail() or not. Maybe we could avoid the error in
the core by just printing a warning, this way the drivers would not need
to lie to the core.

What I would suggest, now that I properly understood the problem, is to:
1. Revert Elad's patchset *entirely* (including the revert of
   existing/valid but with little use layouts).
2. Relax the check in the core.

Thanks,
Miquèl