Re: [PATCH 1/2] mtd: spi-nor: take the flash lock in spi_nor_shutdown()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Unsafe Killable Wait in Shutdown/Remove skips hardware restoration

Pre-existing issues:
- [Critical] Race condition leading to data corruption after shutdown restore
- [High] Resource leak in locking helpers on fatal signal interruption
--

commit 7d7b032d63ce9b4009f7d886e821261d29fb7c2a
Author: Itai Handler <itai.handler@gmail.com>

mtd: spi-nor: take the flash lock in spi_nor_shutdown()

This commit modifies spi_nor_shutdown() to acquire the device lock before
restoring the flash addressing mode. This serializes the restore operation
against operations already in flight during system shutdown.

> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index ccf4396cdcd04..96dd6ae6d6191 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -3862,8 +3862,21 @@ static int spi_nor_remove(struct spi_mem *spimem)
>  static void spi_nor_shutdown(struct spi_mem *spimem)
>  {
>  	struct spi_nor *nor = spi_mem_get_drvdata(spimem);
> +	int ret;
> +
> +	/*
> +	 * Wait for an operation started by another thread to finish.
> +	 * device_shutdown() runs with MTD users still active: a busy flash
> +	 * ignores the commands spi_nor_restore() issues, leaving it in
> +	 * 4-byte address mode, and a restore landing mid-read changes the
> +	 * chip's address width under the transfer.
> +	 */
> +	ret = spi_nor_prep_and_lock(nor);

[Severity: High]
This is a pre-existing issue, but does spi_nor_prep_and_lock() leak device
preparation resources if interrupted by a fatal signal?

If a process is killed while waiting on wait_event_killable() inside
spi_nor_prep_and_lock(), it returns -ERESTARTSYS but fails to invoke
spi_nor_unprep() to roll back the earlier spi_nor_prep() call. Does this
also apply to the other locking helpers like spi_nor_prep_and_lock_pe() and
spi_nor_prep_and_lock_rd()?

> +	if (ret)
> +		return;

[Severity: High]
If a fatal signal is pending during the reboot process, such as from the OOM
killer or a manual SIGKILL, wait_event_killable() in spi_nor_prep_and_lock()
will immediately return -ERESTARTSYS.

Will this early return skip the spi_nor_restore() hardware state reset? Since
this skips restoring the 3-byte addressing mode, could the bootloader read
garbage on the next boot, leaving the device unbootable until a power cycle?

>  
>  	spi_nor_restore(nor);
> +	spi_nor_unlock_and_unprep(nor);

[Severity: Critical]
This is a pre-existing issue, but could releasing the lock while the MTD
device remains active expose the flash's modified 3-byte addressing state
to concurrent operations?

If a concurrent thread acquires the lock immediately after spi_nor_shutdown()
finishes and issues a write command, the driver will assume 4-byte mode
and send 4 address bytes. Could the hardware misinterpret the 4th address
byte as data, resulting in permanent data corruption by shifting the
destination address?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910184452.895485-1-itai.handler@gmail.com?part=1

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