[PATCH] mtd: core: reject OOB transfers starting at the device end

[sashiko-bot at kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mtd: core: maxooblen underflow in mtd_check_oob_ops() on unaligned size
--

commit f1617f89facd985e5ed9b314a9af3e4693ab3800
Author: James Hilliard <james.hilliard1 at gmail.com>

mtd: core: reject OOB transfers starting at the device end

This commit updates mtd_check_oob_ops() to reject nonempty OOB transfers
starting at or beyond mtd->size so that maxooblen does not underflow when
ops->ooboffs is nonzero.

> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 491a27d0315fa..b82fd7ccd111d 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -1676,6 +1676,9 @@ static int mtd_check_oob_ops(struct mtd_info *mtd, loff_t offs,
>  	if (ops->ooblen) {
>  		size_t maxooblen;
>  
> +		if (offs >= mtd->size)
> +			return -EINVAL;

[Severity: Medium]
Can maxooblen still underflow in mtd_check_oob_ops() when mtd->size is not
aligned to mtd->writesize?

In drivers/mtd/mtdpart.c, allocate_partition() allows registering read-only
partitions whose size is not a multiple of writesize:

drivers/mtd/mtdpart.c:allocate_partition() {
	...
	tmp = mtd_get_master_ofs(child, 0) + child->part.size;
	remainder = do_div(tmp, wr_alignment);
	if ((child->flags & MTD_WRITEABLE) && remainder) {
		child->flags &= ~MTD_WRITEABLE;
		printk(KERN_WARNING"mtd: partition \"%s\" doesn't end on an erase/write block -- force read-only\n",
			part->name);
	}

	child->size = child->part.size;
	...
}

And mtd_check_oob_ops() computes maxooblen using floor division via
mtd_div_by_ws():

drivers/mtd/mtdcore.c:mtd_check_oob_ops() {
	...
		if (offs >= mtd->size)
			return -EINVAL;

		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;
	...
}

When a request arrives via mtdchar_readoob() -> mtd_read_oob() ->
mtd_check_oob_ops() with an offset in the trailing partial page
[mtd_div_by_ws(mtd->size, mtd) * mtd->writesize, mtd->size) and a nonzero
ops->ooboffs, offs < mtd->size holds, but mtd_div_by_ws(offs, mtd) equals
mtd_div_by_ws(mtd->size, mtd). Subtracting ops->ooboffs from zero then
underflows maxooblen and allows reading OOB data from the page extending
past the partition end.

Would checking mtd_div_by_ws(offs, mtd) >= mtd_div_by_ws(mtd->size, mtd)
instead of offs >= mtd->size avoid this underflow for unaligned partition
sizes as well?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104-submit-mtd-oob-end-v1-v1-1-5cb8ad921cb0@gmail.com?part=1