* [PATCH] mtd: core: avoid overflow when checking data bounds
@ 2026-11-05 4:03 ` James Hilliard
0 siblings, 0 replies; 3+ messages in thread
From: James Hilliard @ 2026-11-05 4:03 UTC (permalink / raw)
To: Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Boris Brezillon
Cc: linux-mtd, linux-kernel, stable, James Hilliard
mtd_check_oob_ops() adds the starting offset to the data length before
comparing the result with the device size. On 64-bit systems, a request
with offs == 1 and ops->len == SIZE_MAX wraps that sum to zero and passes
the range check. The public read and write wrappers can then pass an
out-of-range request to the driver.
Validate the starting offset first, then compare the data length with
the remaining capacity. Preserve NULL-buffer length normalization and
empty requests at the end of the device.
Fixes: 5cdd929da53d ("mtd: Add sanity checks in mtd_write/read_oob()")
Cc: stable@vger.kernel.org
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
drivers/mtd/mtdcore.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 491a27d0315f..dda976ef52c9 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -1670,7 +1670,7 @@ static int mtd_check_oob_ops(struct mtd_info *mtd, loff_t offs,
if (!ops->oobbuf)
ops->ooblen = 0;
- if (offs < 0 || offs + ops->len > mtd->size)
+ if (offs < 0 || offs > mtd->size || ops->len > mtd->size - offs)
return -EINVAL;
if (ops->ooblen) {
---
base-commit: 1c1a342aceec79528b3ba51f376eca2be5928fe7
change-id: 20261104-submit-mtd-oob-bounds-v1-2c5cf82f2767
Best regards,
--
James Hilliard <james.hilliard1@gmail.com>
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] mtd: core: avoid overflow when checking data bounds
2026-11-05 4:03 ` James Hilliard
(?)
@ 2026-11-05 4:15 ` sashiko-bot
-1 siblings, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-11-05 4:15 UTC (permalink / raw)
To: James Hilliard, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, Boris Brezillon
Cc: linux-mtd
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mtd: core: mtd_check_oob_ops() allows offs == mtd->size and OOB underflow
--
commit c61f670d83feb0a2a97da57f9f9695ed9acc74db
Author: James Hilliard <james.hilliard1@gmail.com>
mtd: core: avoid overflow when checking data bounds
This commit updates mtd_check_oob_ops() to validate the starting offset
against the device size before comparing ops->len with the remaining
capacity, avoiding an integer overflow in offs + ops->len.
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 491a27d0315fa..dda976ef52c9b 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -1670,7 +1670,7 @@ static int mtd_check_oob_ops(struct mtd_info *mtd, loff_t offs,
> if (!ops->oobbuf)
> ops->ooblen = 0;
>
> - if (offs < 0 || offs + ops->len > mtd->size)
> + if (offs < 0 || offs > mtd->size || ops->len > mtd->size - offs)
> return -EINVAL;
[Severity: High]
Does allowing offs == mtd->size when ops->len == 0 here leave two
issues when requests are made at the exact end of the device?
First, for an OOB-only request (ops->datbuf == NULL, ops->len == 0,
ops->ooblen > 0) at offs == mtd->size, execution enters the if (ops->ooblen)
block in mtd_check_oob_ops():
if (ops->ooblen) {
size_t maxooblen;
if (ops->ooboffs >= mtd_oobavail(mtd, ops))
return -EINVAL;
maxooblen = ((size_t)(mtd_div_by_ws(mtd->size, mtd) -
mtd_div_by_ws(offs, mtd)) *
mtd_oobavail(mtd, ops)) - ops->ooboffs;
if (ops->ooblen > maxooblen)
return -EINVAL;
}
When offs == mtd->size, mtd_div_by_ws(mtd->size, mtd) - mtd_div_by_ws(offs, mtd)
is 0. If 0 < ops->ooboffs < mtd_oobavail(mtd, ops), subtracting ops->ooboffs
from 0 wraps the unsigned size_t maxooblen to SIZE_MAX - ops->ooboffs + 1.
The ops->ooblen > maxooblen check then fails to reject the request, and
mtd_read_oob() or mtd_write_oob() forwards an out-of-range OOB operation at
offs == mtd->size to the driver. This can be triggered from userspace via
mtdchar_readoob() or mtdchar_writeoob() when start == mtd->size + 1 and
length == 1:
ops.ooblen = length;
ops.ooboffs = start & (mtd->writesize - 1);
ops.datbuf = NULL;
...
start &= ~((uint64_t)mtd->writesize - 1);
ret = mtd_write_oob(mtd, start, &ops);
Second, even when both ops->len == 0 and ops->ooblen == 0 at offs == mtd->size,
neither mtd_check_oob_ops() nor mtd_read_oob() / mtd_write_oob() short-circuits
zero-length requests before calling into the driver via mtd_read_oob_std() or
mtd_write_oob_std(). Drivers such as nand_do_read_ops() and nand_do_read_oob()
in drivers/mtd/nand/raw/nand_base.c immediately compute the target chip from
the offset before checking the length:
chipnr = (int)(from >> chip->chip_shift);
nand_select_target(chip, chipnr);
When from == mtd->size on the master device, chipnr equals
nanddev_ntargets(&chip->base), triggering the WARN_ON in nand_select_target()
and issuing an out-of-bounds page operation.
Should mtd_check_oob_ops() reject offs >= mtd->size (or short-circuit empty
operations in mtd_read_oob() and mtd_write_oob() while rejecting
offs >= mtd->size for any non-empty data or OOB request), similar to how
mtd_point(), mtd_unpoint(), and mtd_panic_write() check offs >= mtd->size?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-submit-mtd-oob-bounds-v1-v1-1-05209fce5a3e@gmail.com?part=1
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 3+ messages in thread