Re: [PATCH v2 2/7] mtd: spi-nor: Wait for NOR ready with poll_timeout_us() - sashiko-bot

From: sashiko-bot@kernel.org
To: "Paul Cercueil" <paul.cercueil@bootlin.com>,
	"Pratyush Yadav" <pratyush@kernel.org>,
	"Michael Walle" <mwalle@kernel.org>,
	"Takahiro Kuwano" <takahiro.kuwano@infineon.com>,
	"Miquel Raynal" <miquel.raynal@bootlin.com>,
	"Richard Weinberger" <richard@nod.at>,
	"Vignesh Raghavendra" <vigneshr@ti.com>,
	"Mark Brown" <broonie@kernel.org>,
	"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>
Cc: "Kees Cook" <kees@kernel.org>, "Tony Luck" <tony.luck@intel.com>,
	"Guilherme G . Piccoli" <gpiccoli@igalia.com>,
	linux-mtd@lists.infradead.org,
	"Bharat Bhushan" <bbhushan2@marvell.com>,
	"Sunil Kovvuri Goutham" <sgoutham@marvell.com>,
	"Tanmay Jagdale" <tanmay@marvell.com>
Subject: Re: [PATCH v2 2/7] mtd: spi-nor: Wait for NOR ready with poll_timeout_us()
Date: Tue, 13 Oct 2026 14:30:15 +0000	[thread overview]
Message-ID: <sashiko-outbox-169912@kernel.org> (raw)
In-Reply-To: <20261013141350.189996-3-paul.cercueil@bootlin.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mtd: spi-nor: 32-bit integer overflow in timeout_ms * 1000
--

commit d88f329aca4b57db5e4ba0a0fc748ee4d7440aba
Author: Tanmay Jagdale <tanmay@marvell.com>

mtd: spi-nor: Wait for NOR ready with poll_timeout_us()

Instead of polling for the spi_nor_ready() with a custom loop, use
poll_timeout_us(). The sleep period between two polls is set to 100 us
which I think should be fine.

> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 8bc117b46e028..2509fb983d4f2 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
[ ... ]
> @@ -712,29 +713,21 @@ static int spi_nor_ready(struct spi_nor *nor)
>   * Return: 0 on success, -errno otherwise.
>   */
>  static int spi_nor_wait_till_ready_with_timeout(struct spi_nor *nor,
> -						unsigned long timeout_jiffies)
> +						unsigned long timeout_ms)
>  {
> -	unsigned long deadline;
> -	int timeout = 0, ret;
> +	int ret, status;
>  
> -	deadline = jiffies + timeout_jiffies;
> -
> -	while (!timeout) {
> -		if (time_after_eq(jiffies, deadline))
> -			timeout = 1;
> -
> -		ret = spi_nor_ready(nor);
> -		if (ret < 0)
> -			return ret;
> -		if (ret)
> -			return 0;
> -
> -		cond_resched();
> +	ret = poll_timeout_us(status = spi_nor_ready(nor), status,
> +			      100, timeout_ms * 1000, false);
                                   ^^^^^^^^^^^^^^^^^^^
[Severity: High]
Can this multiplication overflow on 32-bit systems?

In spi_nor_erase_dice(), the timeout scales with the flash size. For a 256MB
flash, timeout_ms evaluates to 40000UL * 128 = 5,120,000.

Because both operands are 32-bit types on 32-bit platforms, the arithmetic
could overflow ULONG_MAX (4,294,967,295), wrapping to 825,032,704 before being
implicitly promoted to u64 inside the poll_timeout_us macro.

This truncates the wait timeout from ~85 minutes to ~13.7 minutes, which could
cause legitimately slow erase operations on large flashes to prematurely abort
with -ETIMEDOUT.

> +	if (ret < 0) {
> -	dev_dbg(nor->dev, "flash operation timed out\n");
> +		dev_dbg(nor->dev, "flash operation timed out\n");
> +		return ret;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013141350.189996-1-paul.cercueil@bootlin.com?part=2

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