[PATCH] mtd: rawnand: nandsim: validate overridesize before shifting

[PATCH] mtd: rawnand: nandsim: validate overridesize before shifting

[James Hilliard]

The size overflow check shifts by overridesize before validating it, so
values at least 64 already cause an invalid shift. Smaller values can
pass that check but exceed the width of the later eraseblock-count shift.

Use check_shl_overflow() to calculate the unsigned eraseblock count and
reject values that cannot be represented. Derive the byte size by
multiplying the two 32-bit quantities in 64-bit arithmetic.

Fixes: a5ac8aeb2900 ("[MTD] nandsim: enhance nandsim to allow arbitrary NAND size")
Cc: stable@vger.kernel.org
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/mtd/nand/raw/nandsim.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/raw/nandsim.c b/drivers/mtd/nand/raw/nandsim.c
index fe968037f75a..6716eafb4b00 100644
--- a/drivers/mtd/nand/raw/nandsim.c
+++ b/drivers/mtd/nand/raw/nandsim.c
@@ -16,6 +16,7 @@
 #include <linux/types.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
+#include <linux/overflow.h>
 #include <linux/vmalloc.h>
 #include <linux/math64.h>
 #include <linux/slab.h>
@@ -2341,21 +2342,21 @@ static int __init ns_init_module(void)
 	}
 
 	if (overridesize) {
-		uint64_t new_size = (uint64_t)nsmtd->erasesize << overridesize;
 		struct nand_memory_organization *memorg;
+		unsigned int eraseblocks;
 		u64 targetsize;
 
 		memorg = nanddev_get_memorg(&chip->base);
 
-		if (new_size >> overridesize != nsmtd->erasesize) {
+		if (check_shl_overflow(1U, overridesize, &eraseblocks)) {
 			NS_ERR("overridesize is too big\n");
 			ret = -EINVAL;
 			goto cleanup_nand;
 		}
 
 		/* N.B. This relies on nand_scan not doing anything with the size before we change it */
-		nsmtd->size = new_size;
-		memorg->eraseblocks_per_lun = 1 << overridesize;
+		nsmtd->size = (u64)nsmtd->erasesize * eraseblocks;
+		memorg->eraseblocks_per_lun = eraseblocks;
 		targetsize = nanddev_target_size(&chip->base);
 		chip->chip_shift = ffs(nsmtd->erasesize) + overridesize - 1;
 		chip->pagemask = (targetsize >> chip->page_shift) - 1;

---
base-commit: 30daaf3e7897b4a8d9b72e5b0c86dd6e2a4d7d85
change-id: 20261107-submit-nandsim-overridesize-bound-v1-1c83cbe80469

Best regards,
--  
James Hilliard <james.hilliard1@gmail.com>


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

Re: [PATCH] mtd: rawnand: nandsim: validate overridesize before shifting

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] mtd: rawnand: nandsim: unsigned integer overflow in
  ns_init() [1]
--

commit 172aea198c2ee0259ce74734f4b5dab1d08fa195
Author: James Hilliard <james.hilliard1@gmail.com>

mtd: rawnand: nandsim: validate overridesize before shifting

This commit validates the overridesize shift count using
check_shl_overflow() before computing the eraseblock count and total MTD
device size.

> diff --git a/drivers/mtd/nand/raw/nandsim.c b/drivers/mtd/nand/raw/nandsim.c
> index fe968037f75a2..6716eafb4b004 100644
> --- a/drivers/mtd/nand/raw/nandsim.c
> +++ b/drivers/mtd/nand/raw/nandsim.c

[ ... ]

> @@ -2341,21 +2342,21 @@ static int __init ns_init_module(void)
>  	}
>  
>  	if (overridesize) {
> -		uint64_t new_size = (uint64_t)nsmtd->erasesize << overridesize;
>  		struct nand_memory_organization *memorg;
> +		unsigned int eraseblocks;
>  		u64 targetsize;
>  
>  		memorg = nanddev_get_memorg(&chip->base);
>  
> -		if (new_size >> overridesize != nsmtd->erasesize) {
> +		if (check_shl_overflow(1U, overridesize, &eraseblocks)) {
>  			NS_ERR("overridesize is too big\n");
>  			ret = -EINVAL;
>  			goto cleanup_nand;
>  		}
>  
>  		/* N.B. This relies on nand_scan not doing anything with the size before we change it */
> -		nsmtd->size = new_size;
> -		memorg->eraseblocks_per_lun = 1 << overridesize;
> +		nsmtd->size = (u64)nsmtd->erasesize * eraseblocks;
> +		memorg->eraseblocks_per_lun = eraseblocks;

[Severity: Critical]
This problem wasn't introduced by this patch, but can a large
overridesize that passes check_shl_overflow() still overflow the 32-bit
ns->geom.pgnum in ns_init() [1]?

drivers/mtd/nand/raw/nandsim.c:ns_init() {
	...
	ns->geom.totsz    = mtd->size;
	ns->geom.pgsz     = mtd->writesize;
	...
	ns->geom.pgnum    = div_u64(ns->geom.totsz, ns->geom.pgsz);
	...
}

Because each eraseblock contains multiple pages, an eraseblock count that
fits in unsigned int can still produce a total page count exceeding
UINT_MAX.  Truncating ns->geom.pgnum causes ns_init() to allocate a
smaller page array than mtd->size requires, leading to out-of-bounds
accesses during simulated flash operations.

[1] https://sashiko.dev/#/bug/linux-3c379a73-65df-4b5f-8179-95ceb2f44717

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261108-submit-nandsim-overridesize-bound-v1-v1-1-9cde7657a69b@gmail.com?part=1

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