[PATCH 2/2] mtd: spi-nor: core: Fix use-after-free of spi_nor on unbind with open handles
Michael Walle
mwalle at kernel.org
Mon Sep 21 00:26:12 PDT 2026
On Thu Sep 3, 2026 at 5:14 AM CEST, tze.yee.ng wrote:
> From: Tze Yee Ng <tze.yee.ng at altera.com>
>
> The spi_nor is allocated with devm_kzalloc() on the SPI device, so a
> sysfs unbind frees it while /dev/mtdX is still open. A later close then
> oopses in spi_nor_put_device()->module_put() on the freed spi_nor.
> try_module_get() does not help: it blocks rmmod, not an unbind.
>
> Give the spi_nor (and its params and bouncebuf) a kref lifetime on the
> spi-mem probe path so it survives until the last MTD user is gone, and
> cache the controller module for the put path. The spimem and dirmaps
> stay owned by the SPI core, so spi_nor_remove() drains in-flight
> operations and sets nor->removed to fail later ones with -ENODEV.
> Legacy controllers are unchanged.
>
> Signed-off-by: Tze Yee Ng <tze.yee.ng at altera.com>
Sashiko had some remarks:
https://sashiko.dev/#/patchset/cover.1788404586.git.tze.yee.ng%40altera.com
Also how does the other mtd subsystems (spi-nand?) doing this? Do we
have the wrong dev for devres?
-michael
> ---
> drivers/mtd/spi-nor/core.c | 111 ++++++++++++++++++++++++++++++++----
> include/linux/mtd/spi-nor.h | 17 ++++++
> 2 files changed, 118 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index 8bc117b46e02..7fb61dde58f5 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -45,6 +45,9 @@
> #define SPI_NOR_SRST_SLEEP_MIN 200
> #define SPI_NOR_SRST_SLEEP_MAX 400
>
> +static void spi_nor_unlock_and_unprep_pe(struct spi_nor *nor, loff_t start, size_t len);
> +static void spi_nor_unlock_and_unprep_rd(struct spi_nor *nor, loff_t start, size_t len);
> +
> /**
> * spi_nor_get_cmd_ext() - Get the command opcode extension based on the
> * extension type.
> @@ -1345,8 +1348,15 @@ int spi_nor_prep_and_lock(struct spi_nor *nor)
> else
> ret = wait_event_killable(nor->rww.wait,
> spi_nor_rww_start_exclusive(nor));
> + if (ret)
> + return ret;
>
> - return ret;
> + if (nor->removed) {
> + spi_nor_unlock_and_unprep(nor);
> + return -ENODEV;
> + }
> +
> + return 0;
> }
>
> void spi_nor_unlock_and_unprep(struct spi_nor *nor)
> @@ -1416,8 +1426,15 @@ static int spi_nor_prep_and_lock_pe(struct spi_nor *nor, loff_t start, size_t le
> else
> ret = wait_event_killable(nor->rww.wait,
> spi_nor_rww_start_pe(nor, start, len));
> + if (ret)
> + return ret;
>
> - return ret;
> + if (nor->removed) {
> + spi_nor_unlock_and_unprep_pe(nor, start, len);
> + return -ENODEV;
> + }
> +
> + return 0;
> }
>
> static void spi_nor_unlock_and_unprep_pe(struct spi_nor *nor, loff_t start, size_t len)
> @@ -1489,8 +1506,15 @@ static int spi_nor_prep_and_lock_rd(struct spi_nor *nor, loff_t start, size_t le
> else
> ret = wait_event_killable(nor->rww.wait,
> spi_nor_rww_start_rd(nor, start, len));
> + if (ret)
> + return ret;
>
> - return ret;
> + if (nor->removed) {
> + spi_nor_unlock_and_unprep_rd(nor, start, len);
> + return -ENODEV;
> + }
> +
> + return 0;
> }
>
> static void spi_nor_unlock_and_unprep_rd(struct spi_nor *nor, loff_t start, size_t len)
> @@ -3186,7 +3210,12 @@ static int spi_nor_init_params(struct spi_nor *nor)
> {
> int ret;
>
> - nor->params = devm_kzalloc(nor->dev, sizeof(*nor->params), GFP_KERNEL);
> + /* Keep params on the kref lifetime so it survives unbind (see probe). */
> + if (nor->refcounted)
> + nor->params = kzalloc(sizeof(*nor->params), GFP_KERNEL);
> + else
> + nor->params = devm_kzalloc(nor->dev, sizeof(*nor->params),
> + GFP_KERNEL);
> if (!nor->params)
> return -ENOMEM;
>
> @@ -3420,6 +3449,22 @@ static void spi_nor_resume(struct mtd_info *mtd)
> dev_err(dev, "resume() failed\n");
> }
>
> +static void spi_nor_release(struct kref *kref)
> +{
> + struct spi_nor *nor = container_of(kref, struct spi_nor, refcount);
> +
> + kfree(nor->bouncebuf);
> + kfree(nor->params);
> + kfree(nor);
> +}
> +
> +static void spi_nor_release_device(void *data)
> +{
> + struct spi_nor *nor = data;
> +
> + kref_put(&nor->refcount, spi_nor_release);
> +}
> +
> static int spi_nor_get_device(struct mtd_info *mtd)
> {
> struct mtd_info *master = mtd_get_master(mtd);
> @@ -3434,6 +3479,12 @@ static int spi_nor_get_device(struct mtd_info *mtd)
> if (!try_module_get(dev->driver->owner))
> return -ENODEV;
>
> + if (nor->refcounted) {
> + /* Cache the module: the spimem/controller chain may be freed by put time. */
> + nor->controller_module = dev->driver->owner;
> + kref_get(&nor->refcount);
> + }
> +
> return 0;
> }
>
> @@ -3443,6 +3494,14 @@ static void spi_nor_put_device(struct mtd_info *mtd)
> struct spi_nor *nor = mtd_to_spi_nor(master);
> struct device *dev;
>
> + if (nor->refcounted) {
> + module_put(nor->controller_module);
> +
> + /* Must be last: this may free nor (and the embedded mtd). */
> + kref_put(&nor->refcount, spi_nor_release);
> + return;
> + }
> +
> if (nor->spimem)
> dev = nor->spimem->spi->controller->dev.parent;
> else
> @@ -3655,8 +3714,11 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> * than 1KB) after spi_nor_scan() returns.
> */
> nor->bouncebuf_size = PAGE_SIZE;
> - nor->bouncebuf = devm_kmalloc(dev, nor->bouncebuf_size,
> - GFP_KERNEL);
> + if (nor->refcounted)
> + nor->bouncebuf = kmalloc(nor->bouncebuf_size, GFP_KERNEL);
> + else
> + nor->bouncebuf = devm_kmalloc(dev, nor->bouncebuf_size,
> + GFP_KERNEL);
> if (!nor->bouncebuf)
> return -ENOMEM;
>
> @@ -3788,10 +3850,21 @@ static int spi_nor_probe(struct spi_mem *spimem)
> if (ret)
> return ret;
>
> - nor = devm_kzalloc(dev, sizeof(*nor), GFP_KERNEL);
> + /*
> + * An open /dev/mtdX handle can outlive unbind, so manage the spi_nor
> + * with a kref and drop the probe-time reference from a devres callback.
> + */
> + nor = kzalloc_obj(*nor, GFP_KERNEL);
> if (!nor)
> return -ENOMEM;
>
> + kref_init(&nor->refcount);
> + nor->refcounted = true;
> +
> + ret = devm_add_action_or_reset(dev, spi_nor_release_device, nor);
> + if (ret)
> + return ret;
> +
> nor->spimem = spimem;
> nor->dev = dev;
> spi_nor_set_flash_node(nor, dev->of_node);
> @@ -3830,9 +3903,8 @@ static int spi_nor_probe(struct spi_mem *spimem)
> */
> if (nor->params->page_size > PAGE_SIZE) {
> nor->bouncebuf_size = nor->params->page_size;
> - devm_kfree(dev, nor->bouncebuf);
> - nor->bouncebuf = devm_kmalloc(dev, nor->bouncebuf_size,
> - GFP_KERNEL);
> + kfree(nor->bouncebuf);
> + nor->bouncebuf = kmalloc(nor->bouncebuf_size, GFP_KERNEL);
> if (!nor->bouncebuf)
> return -ENOMEM;
> }
> @@ -3853,6 +3925,25 @@ static int spi_nor_remove(struct spi_mem *spimem)
> {
> struct spi_nor *nor = spi_mem_get_drvdata(spimem);
>
> + /*
> + * Drain in-flight operations and set nor->removed under the lock so
> + * later ones fail with -ENODEV before touching SPI-core state (spimem,
> + * dirmaps) freed after this returns. The wait is uninterruptible.
> + */
> + if (!spi_nor_use_parallel_locking(nor))
> + mutex_lock(&nor->lock);
> + else
> + wait_event(nor->rww.wait, spi_nor_rww_start_exclusive(nor));
> +
> + nor->removed = true;
> +
> + if (!spi_nor_use_parallel_locking(nor)) {
> + mutex_unlock(&nor->lock);
> + } else {
> + spi_nor_rww_end_exclusive(nor);
> + wake_up(&nor->rww.wait);
> + }
> +
> spi_nor_restore(nor);
>
> /* Clean up MTD stuff. */
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 4b92494827b1..f8d5e3ca8371 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -7,6 +7,7 @@
> #define __LINUX_MTD_SPI_NOR_H
>
> #include <linux/bitops.h>
> +#include <linux/kref.h>
> #include <linux/mtd/mtd.h>
> #include <linux/spi/spi-mem.h>
>
> @@ -352,6 +353,18 @@ struct spi_nor_flash_parameter;
> * @rww.used_banks: bitmap of the banks in use
> * @dev: pointer to an SPI device or an SPI NOR controller device
> * @spimem: pointer to the SPI memory device
> + * @refcount: reference count keeping the kzalloc()'d spi_nor alive
> + * past driver unbind until the last MTD user releases the
> + * device. Only valid when @refcounted is set.
> + * @controller_module: controller module pinned in spi_nor_get_device() so
> + * spi_nor_put_device() need not walk the possibly freed
> + * spimem/controller chain. Only valid when @refcounted is
> + * set.
> + * @refcounted: true when the spi_nor lifetime is kref-managed (the
> + * spi-mem spi_nor_probe() path). Legacy controllers that
> + * embed or devres-allocate spi_nor leave this clear.
> + * @removed: set on unbind to make subsequent MTD operations fail
> + * with -ENODEV instead of touching released resources.
> * @bouncebuf: bounce buffer used when the buffer passed by the MTD
> * layer is not DMA-able
> * @bouncebuf_size: size of the bounce buffer
> @@ -393,6 +406,10 @@ struct spi_nor {
> } rww;
> struct device *dev;
> struct spi_mem *spimem;
> + struct kref refcount;
> + struct module *controller_module;
> + bool refcounted;
> + bool removed;
> u8 *bouncebuf;
> size_t bouncebuf_size;
> u8 *id;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 297 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20260921/7f6eb4ef/attachment.sig>
More information about the linux-mtd
mailing list