Re: [PATCH v2 3/3] MIPS: Octeon: flash: use the simple map accessors without a shared eMMC - sashiko-bot

From: sashiko-bot@kernel.org
To: miquel.raynal@bootlin.com, "Orgad Shaneh" <orgads@gmail.com>,
	tsbogend@alpha.franken.de, richard@nod.at, vigneshr@ti.com
Cc: kaloz@openwrt.org, corbet@lwn.net, dwmw2@infradead.org,
	john@phrozen.org, linux-mtd@lists.infradead.org,
	linux-doc@vger.kernel.org, linux-mips@vger.kernel.org,
	linusw@kernel.org, linux-kernel@vger.kernel.org,
	nico@fluxnic.net, ulli.kroll@googlemail.com
Subject: Re: [PATCH v2 3/3] MIPS: Octeon: flash: use the simple map accessors without a shared eMMC
Date: Sat, 10 Oct 2026 18:18:29 +0000	[thread overview]
Message-ID: <sashiko-outbox-166800@kernel.org> (raw)
In-Reply-To: <20261010180840.2152492-4-orgads@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mips: cavium-octeon: static map_info overwrite and memory leak in octeon_flash_probe
--

commit 2d9aae5bb3f634f71b28d3ae93087e12e701fbe6
Author: Orgad Shaneh <orgads@gmail.com>

MIPS: Octeon: flash: use the simple map accessors without a shared eMMC

This commit updates the Octeon flash driver to use simple_map_init()
when the boot bus isn't shared with an eMMC host, reducing mount times
significantly. It also checks for supported bank widths earlier in the
probe function to avoid hitting BUG calls on unsupported widths.

> diff --git a/arch/mips/cavium-octeon/flash_setup.c b/arch/mips/cavium-octeon/flash_setup.c
> index 3395acde4d60a..5448d5139c640 100644
> --- a/arch/mips/cavium-octeon/flash_setup.c
> +++ b/arch/mips/cavium-octeon/flash_setup.c

[ ... ]

> @@ -85,6 +96,12 @@ static int octeon_flash_probe(struct platform_device *pdev)
[ ... ]
>  		flash_map.size = 0x1fc00000 - flash_map.phys;
>  		/* 8-bit bus (0 + 1) or 16-bit bus (1 + 1) */
>  		flash_map.bankwidth = region_cfg.s.width + 1;
>  		flash_map.virt = ioremap(flash_map.phys, flash_map.size);

[Severity: Medium]
If multiple flash devices are probed, does this unconditionally overwrite
the static flash_map.virt with a new ioremap() mapping?

If an earlier probe successfully registered an MTD device, overwriting
the singleton flash_map could corrupt the previous device's mapping.

>  		pr_notice("Bootbus flash: Setting flash for %luMB flash at "
>  			  "0x%08llx\n", flash_map.size >> 20, flash_map.phys);

[ ... ]

>  		mymtd = do_map_probe("cfi_probe", &flash_map);
>  		if (mymtd) {
>  			mymtd->owner = THIS_MODULE;
>  			mtd_device_parse_register(mymtd, part_probe_types,
>  						  NULL, NULL, 0);
>  		} else {
>  			pr_err("Failed to register MTD device for flash\n");
>  		}

[Severity: Medium]
Is an iounmap(flash_map.virt) needed here when do_map_probe() fails?

Without it, this error path appears to leak the virtual mapping.

The v2 changelog mentions omitting iounmap() because it could unmap a
mapping that an earlier probe registered. However, since the static
flash_map.virt is unconditionally overwritten by ioremap() earlier in
this function, wouldn't the singleton already be corrupted if a prior
probe existed?

Skipping iounmap() in this path appears to just leak memory rather than
safely preserving any previous mapping.

>  	}
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261010180840.2152492-1-orgads@gmail.com?part=3

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