[PATCH 0/2] mtd: spi-nor: Fix unbind deadlock and use-after-free

[PATCH 0/2] mtd: spi-nor: Fix unbind deadlock and use-after-free

[tze.yee.ng]

From: Tze Yee Ng <tze.yee.ng@altera.com>

Two fixes in the spi-nor core around locking and driver unbind:

Patch 1 fixes a self-deadlock in spi_nor_rww_start_exclusive(), which
took nor->lock while its end helper re-acquired it via guard(mutex).

Patch 2 fixes a use-after-free where a sysfs unbind frees the
devm-allocated spi_nor while /dev/mtdX is still open. Give it a kref
lifetime on the spi-mem probe path and fail operations with -ENODEV
after unbind. Legacy controllers are unchanged.

Tze Yee Ng (2):
  mtd: spi-nor: Fix deadlock in spi_nor_rww_start_exclusive()
  mtd: spi-nor: core: Fix use-after-free of spi_nor on unbind with open
    handles

 drivers/mtd/spi-nor/core.c  | 113 ++++++++++++++++++++++++++++++++----
 include/linux/mtd/spi-nor.h |  17 ++++++
 2 files changed, 119 insertions(+), 11 deletions(-)

-- 
2.43.7


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

[PATCH 1/2] mtd: spi-nor: Fix deadlock in spi_nor_rww_start_exclusive()

[tze.yee.ng]

From: Tze Yee Ng <tze.yee.ng@altera.com>

spi_nor_rww_start_exclusive() takes nor->lock with a bare mutex_lock()
and returns with it held, while its counterpart
spi_nor_rww_end_exclusive() re-acquires nor->lock via guard(mutex). Any
exclusive operation on a read-while-write (SNOR_F_RWW) part thus
deadlocks against itself.

The exclusive section is serialized by the rww ongoing_io/rd/pe flags,
so take nor->lock with guard(mutex) like the sibling start helpers.

Fixes: 03e7bb864d9a9 ("mtd: spi-nor: use scope-based mutex cleanup helpers")
Cc: stable@vger.kernel.org
Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
---
 drivers/mtd/spi-nor/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index ccf4396cdcd0..8bc117b46e02 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -1310,7 +1310,7 @@ static bool spi_nor_rww_start_exclusive(struct spi_nor *nor)
 {
 	struct spi_nor_rww *rww = &nor->rww;
 
-	mutex_lock(&nor->lock);
+	guard(mutex)(&nor->lock);
 
 	if (rww->ongoing_io || rww->ongoing_rd || rww->ongoing_pe)
 		return false;
-- 
2.43.7


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

Re: [PATCH 1/2] mtd: spi-nor: Fix deadlock in spi_nor_rww_start_exclusive()

[Michael Walle]

[-- Attachment #1.1: Type: text/plain, Size: 870 bytes --]

On Thu Sep 3, 2026 at 5:14 AM CEST, tze.yee.ng wrote:
> From: Tze Yee Ng <tze.yee.ng@altera.com>
>
> spi_nor_rww_start_exclusive() takes nor->lock with a bare mutex_lock()
> and returns with it held, while its counterpart
> spi_nor_rww_end_exclusive() re-acquires nor->lock via guard(mutex). Any
> exclusive operation on a read-while-write (SNOR_F_RWW) part thus
> deadlocks against itself.
>
> The exclusive section is serialized by the rww ongoing_io/rd/pe flags,
> so take nor->lock with guard(mutex) like the sibling start helpers.
>
> Fixes: 03e7bb864d9a9 ("mtd: spi-nor: use scope-based mutex cleanup helpers")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>

This was already fixed in:
https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/commit/?id=44b8a0bf5f96e8393312fc34b956766882341f16

-michael

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

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

Re: [PATCH 1/2] mtd: spi-nor: Fix deadlock in spi_nor_rww_start_exclusive()

[NG, TZE YEE]

On 21/9/2026 2:58 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@altera.com>
>>
>> spi_nor_rww_start_exclusive() takes nor->lock with a bare mutex_lock()
>> and returns with it held, while its counterpart
>> spi_nor_rww_end_exclusive() re-acquires nor->lock via guard(mutex). Any
>> exclusive operation on a read-while-write (SNOR_F_RWW) part thus
>> deadlocks against itself.
>>
>> The exclusive section is serialized by the rww ongoing_io/rd/pe flags,
>> so take nor->lock with guard(mutex) like the sibling start helpers.
>>
>> Fixes: 03e7bb864d9a9 ("mtd: spi-nor: use scope-based mutex cleanup helpers")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Tze Yee Ng <tze.yee.ng@altera.com>
> 
> This was already fixed in:
> https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/commit/?id=44b8a0bf5f96e8393312fc34b956766882341f16
> 
> -michael

Thanks Michael, you're right. I missed that commit. I'll drop this patch 
and resend the use-after-free fix (patch 2/2) standalone as v2.

Thanks,
Tze Yee

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

[PATCH 2/2] mtd: spi-nor: core: Fix use-after-free of spi_nor on unbind with open handles

[tze.yee.ng]

From: Tze Yee Ng <tze.yee.ng@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@altera.com>
---
 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;
-- 
2.43.7


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

Re: [PATCH 2/2] mtd: spi-nor: core: Fix use-after-free of spi_nor on unbind with open handles

[Michael Walle]

[-- Attachment #1.1: Type: text/plain, Size: 9494 bytes --]

On Thu Sep 3, 2026 at 5:14 AM CEST, tze.yee.ng wrote:
> From: Tze Yee Ng <tze.yee.ng@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@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;


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]

[-- Attachment #2: Type: text/plain, Size: 144 bytes --]

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

Re: [PATCH 2/2] mtd: spi-nor: core: Fix use-after-free of spi_nor on unbind with open handles

[NG, TZE YEE]

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@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@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;
> 


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

Re: Fw: [PATCH 0/2] mtd: spi-nor: Fix unbind deadlock and use-after-free

[NG, TZE YEE]

On 21/9/2026 2:27 pm, NG, TZE YEE wrote:
> 
> *From:* NG, TZE YEE
> *Sent:* Thursday, September 3, 2026 11:14 AM
> *To:* Pratyush Yadav <pratyush@kernel.org>; Michael Walle 
> <mwalle@kernel.org>; Takahiro Kuwano <takahiro.kuwano@infineon.com>; 
> Tudor Ambarus <tudor.ambarus@linaro.org>; linux-mtd@lists.infradead.org 
> <linux-mtd@lists.infradead.org>; linux-kernel@vger.kernel.org <linux- 
> kernel@vger.kernel.org>
> *Subject:* [PATCH 0/2] mtd: spi-nor: Fix unbind deadlock and use-after-free
> From: Tze Yee Ng <tze.yee.ng@altera.com>
> 
> Two fixes in the spi-nor core around locking and driver unbind:
> 
> Patch 1 fixes a self-deadlock in spi_nor_rww_start_exclusive(), which
> took nor->lock while its end helper re-acquired it via guard(mutex).
> 
> Patch 2 fixes a use-after-free where a sysfs unbind frees the
> devm-allocated spi_nor while /dev/mtdX is still open. Give it a kref
> lifetime on the spi-mem probe path and fail operations with -ENODEV
> after unbind. Legacy controllers are unchanged.
> 
> Tze Yee Ng (2):
>    mtd: spi-nor: Fix deadlock in spi_nor_rww_start_exclusive()
>    mtd: spi-nor: core: Fix use-after-free of spi_nor on unbind with open
>      handles
> 
>   drivers/mtd/spi-nor/core.c  | 113 ++++++++++++++++++++++++++++++++----
>   include/linux/mtd/spi-nor.h |  17 ++++++
>   2 files changed, 119 insertions(+), 11 deletions(-)
> 
> -- 
> 2.43.7
> 

Hi,

Gentle ping on this patch. It has been over 2 weeks with no feedback.

Happy to address any review comments.

Thanks,
Tze Yee


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