Skip to contentSkip to conversation

mtd: phram: Prevent divide by zero bug in phram_setup()

PATCH Applied Dan Carpenter 2 participants 3 messages Jan 21, 2022 – Jan 23, 2022

Subsystems (3)
Kernel janitors Memory technology devices (MTD) Phram MTD driver
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH v2 1/1 mtd: phram: Prevent divide by zero bug in phram_setup()

The problem is that "erasesize" is a uint64_t type so it might be non-zero but the lower 32 bits are zero so when it's truncated, "(uint32_t)erasesize", then that value is zero. This leads to a divide by zero bug.

Avoid the bug by delaying the divide until after we have validated that "erasesize" is non-zero and within the uint32_t range.

Fixes dc2b3e5cbc80 ("mtd: phram: use div_u64_rem to stop overwrite len in phram_setup")
Signed-off-by Dan Carpenter <[email protected]>

---
v2: edit the commit message

 drivers/mtd/devices/phram.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
drivers/mtd/devices/phram.c +8 -4
index 6ed6c51fac69..d503821a3e60 100644
@@ -264,16 +264,20 @@ static int phram_setup(const char *val)
 		}
 	}
 
-	if (erasesize)
-		div_u64_rem(len, (uint32_t)erasesize, &rem);
-
 	if (len == 0 || erasesize == 0 || erasesize > len
-	    || erasesize > UINT_MAX || rem) {
+	    || erasesize > UINT_MAX) {
 		parse_err("illegal erasesize or len\n");
 		ret = -EINVAL;
 		goto error;
 	}
 
+	div_u64_rem(len, (uint32_t)erasesize, &rem);
+	if (rem) {
+		parse_err("len is not multiple of erasesize\n");
+		ret = -EINVAL;
+		goto error;
+	}
+
 	ret = register_device(name, start, len, (uint32_t)erasesize);
 	if (ret)
 		goto error;
Signature · 1 line
2.20.1
maintainer Applied
#1 lore raw
Quoted 10 lines · Dan Carpenter

The problem is that "erasesize" is a uint64_t type so it might be non-zero but the lower 32 bits are zero so when it's truncated, "(uint32_t)erasesize", then that value is zero. This leads to a divide by zero bug.

Avoid the bug by delaying the divide until after we have validated that "erasesize" is non-zero and within the uint32_t range.

Fixes dc2b3e5cbc80 ("mtd: phram: use div_u64_rem to stop overwrite len in phram_setup")
Signed-off-by Dan Carpenter <[email protected]>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.

Miquel

maintainer
#2 lore raw

[email protected] wrote on Sun, 23 Jan 2022 16:22:56 +0100:

Quoted 6 lines · Dan Carpenter
Quoted 5 lines

The problem is that "erasesize" is a uint64_t type so it might be non-zero but the lower 32 bits are zero so when it's truncated, "(uint32_t)erasesize", then that value is zero. This leads to a divide by zero bug.

Quoted 7 lines
Quoted 5 lines

Avoid the bug by delaying the divide until after we have validated that "erasesize" is non-zero and within the uint32_t range.

Fixes dc2b3e5cbc80 ("mtd: phram: use div_u64_rem to stop overwrite len in phram_setup")
Signed-off-by Dan Carpenter <[email protected]>

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git mtd/next, thanks.

Pushed on mtd/fixes, actually.