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

[sashiko-bot at kernel.org]

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 at 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