[PATCH 2/2] mtd: spi-nor: core: Fix use-after-free of spi_nor on unbind with open handles
NG, TZE YEE
tze.yee.ng at altera.com
Mon Oct 5 19:13:51 PDT 2026
On 21/9/2026 3:26 pm, Michael Walle wrote:
> 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
>
Thanks. I went through the Sashiko remarks.
The prep leak on signal and the legacy-controller path were
pre-existing, so I left them alone.
The rest were real holes in v1: eraseregions was still
devm-allocated, get_device() walked nor->spimem after unbind, and
suspend/resume had no removed check. Those convinced me the
spi-nor-local kref + nor->removed fence was the wrong place to fix
this. It duplicated mtd->refcnt and still left every other driver to
reimplement the same dance.
On the second question: I don't think we have the wrong device for
devres. spi-nand does the same thing (devm on &mem->spi->dev). An open
/dev/mtdX can outlive unbind, so anything allocated on the SPI device is
already gone.
In v2, I will moves the lifetime into the MTD core: optional mtd->_free
from the existing kref release, and mtd->removed so the core returns
-ENODEV for hw ops, new openers, and suspend/resume. spi-nor uses
that on the spi-mem path; legacy controllers are unchanged. spi-nand
is not converted in this series.
I'll send a v2 shortly.
Thanks,
Tze Yee
>> ---
>> 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;
>
More information about the linux-mtd
mailing list