[PATCH v2 0/2] mtd: slram: fix failed registration cleanup

[PATCH v2 0/2] mtd: slram: fix failed registration cleanup

[Ruoyu Wang]

This splits the previous slram patch into a backportable fix and a
follow-up cleanup.

Patch 1 fixes the dangling list entry on register_device() failure and
carries Fixes and stable tags.

Patch 2 is a cleanup which uses local variables while initializing the
new entry and publishes the list entry only after mtd_device_register()
succeeds.

Changes in v2:
- Split the original patch into a fix first and a cleanup second.
- Add Fixes and Cc: stable tags to the fix.
- Link to v1: https://lore.kernel.org/all/20260608162939.6-1-ruoyuw560@gmail.com/

Ruoyu Wang (2):
  mtd: slram: remove failed entries from the device list
  mtd: slram: simplify register_device() cleanup

 drivers/mtd/devices/slram.c | 96 ++++++++++++++++++++-----------------
 1 file changed, 52 insertions(+), 44 deletions(-)

-- 
2.51.0

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

[PATCH v2 1/2] mtd: slram: remove failed entries from the device list

[Ruoyu Wang]

register_device() links a new slram_mtdlist entry before allocating all
of the state needed by the entry. If a later allocation, memremap(), or
mtd_device_register() fails, the partially initialized entry remains on
the global list. A later cleanup can then dereference or free invalid
state from that failed entry.

Unwind the partially initialized entry and clear the list tail on each
failure path after the entry has been linked.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
 drivers/mtd/devices/slram.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/devices/slram.c b/drivers/mtd/devices/slram.c
index 69cb63d99f573..48c2bc6b65eec 100644
--- a/drivers/mtd/devices/slram.c
+++ b/drivers/mtd/devices/slram.c
@@ -129,6 +129,7 @@ static int slram_write(struct mtd_info *mtd, loff_t to, size_t len,
 static int register_device(char *name, unsigned long start, unsigned long length)
 {
 	slram_mtd_list_t **curmtd;
+	int ret = -ENOMEM;
 
 	curmtd = &slram_mtdlist;
 	while (*curmtd) {
@@ -155,14 +156,15 @@ static int register_device(char *name, unsigned long start, unsigned long length
 
 	if (!(*curmtd)->mtdinfo) {
 		E("slram: Cannot allocate new MTD device.\n");
-		return(-ENOMEM);
+		goto err_free_list;
 	}
 
 	if (!(((slram_priv_t *)(*curmtd)->mtdinfo->priv)->start =
 		memremap(start, length,
 			 MEMREMAP_WB | MEMREMAP_WT | MEMREMAP_WC))) {
 		E("slram: memremap failed\n");
-		return -EIO;
+		ret = -EIO;
+		goto err_free_priv;
 	}
 	((slram_priv_t *)(*curmtd)->mtdinfo->priv)->end =
 		((slram_priv_t *)(*curmtd)->mtdinfo->priv)->start + length;
@@ -183,10 +185,8 @@ static int register_device(char *name, unsigned long start, unsigned long length
 
 	if (mtd_device_register((*curmtd)->mtdinfo, NULL, 0))	{
 		E("slram: Failed to register new device\n");
-		memunmap(((slram_priv_t *)(*curmtd)->mtdinfo->priv)->start);
-		kfree((*curmtd)->mtdinfo->priv);
-		kfree((*curmtd)->mtdinfo);
-		return(-EAGAIN);
+		ret = -EAGAIN;
+		goto err_unmap;
 	}
 	T("slram: Registered device %s from %luKiB to %luKiB\n", name,
 			(start / 1024), ((start + length) / 1024));
@@ -194,6 +194,16 @@ static int register_device(char *name, unsigned long start, unsigned long length
 			((slram_priv_t *)(*curmtd)->mtdinfo->priv)->start,
 			((slram_priv_t *)(*curmtd)->mtdinfo->priv)->end);
 	return(0);
+
+err_unmap:
+	memunmap(((slram_priv_t *)(*curmtd)->mtdinfo->priv)->start);
+err_free_priv:
+	kfree((*curmtd)->mtdinfo->priv);
+err_free_list:
+	kfree((*curmtd)->mtdinfo);
+	kfree(*curmtd);
+	*curmtd = NULL;
+	return ret;
 }
 
 static void unregister_devices(void)
-- 
2.51.0

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

[PATCH v2 2/2] mtd: slram: simplify register_device() cleanup

[Ruoyu Wang]

Use local variables for the list entry, mtd_info, and private data while
initializing a new device. This keeps the initialization path easier to
read and publishes the new list entry only after mtd_device_register()
has succeeded.

Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
 drivers/mtd/devices/slram.c | 86 ++++++++++++++++++-------------------
 1 file changed, 42 insertions(+), 44 deletions(-)

diff --git a/drivers/mtd/devices/slram.c b/drivers/mtd/devices/slram.c
index 48c2bc6b65eec..aa38ecdffc975 100644
--- a/drivers/mtd/devices/slram.c
+++ b/drivers/mtd/devices/slram.c
@@ -129,6 +129,9 @@ static int slram_write(struct mtd_info *mtd, loff_t to, size_t len,
 static int register_device(char *name, unsigned long start, unsigned long length)
 {
 	slram_mtd_list_t **curmtd;
+	slram_mtd_list_t *new_mtd;
+	struct mtd_info *mtdinfo;
+	slram_priv_t *priv;
 	int ret = -ENOMEM;
 
 	curmtd = &slram_mtdlist;
@@ -136,73 +139,68 @@ static int register_device(char *name, unsigned long start, unsigned long length
 		curmtd = &(*curmtd)->next;
 	}
 
-	*curmtd = kmalloc_obj(slram_mtd_list_t);
-	if (!(*curmtd)) {
+	new_mtd = kmalloc_obj(slram_mtd_list_t);
+	if (!new_mtd) {
 		E("slram: Cannot allocate new MTD device.\n");
 		return(-ENOMEM);
 	}
-	(*curmtd)->mtdinfo = kzalloc_obj(struct mtd_info);
-	(*curmtd)->next = NULL;
-
-	if ((*curmtd)->mtdinfo)	{
-		(*curmtd)->mtdinfo->priv =
-			kzalloc_obj(slram_priv_t);
+	new_mtd->next = NULL;
 
-		if (!(*curmtd)->mtdinfo->priv) {
-			kfree((*curmtd)->mtdinfo);
-			(*curmtd)->mtdinfo = NULL;
-		}
+	mtdinfo = kzalloc_obj(struct mtd_info);
+	if (!mtdinfo) {
+		E("slram: Cannot allocate new MTD device.\n");
+		goto err_free_list;
 	}
+	new_mtd->mtdinfo = mtdinfo;
 
-	if (!(*curmtd)->mtdinfo) {
+	priv = kzalloc_obj(slram_priv_t);
+	if (!priv) {
 		E("slram: Cannot allocate new MTD device.\n");
-		goto err_free_list;
+		goto err_free_mtdinfo;
 	}
+	mtdinfo->priv = priv;
 
-	if (!(((slram_priv_t *)(*curmtd)->mtdinfo->priv)->start =
-		memremap(start, length,
-			 MEMREMAP_WB | MEMREMAP_WT | MEMREMAP_WC))) {
+	priv->start = memremap(start, length,
+			       MEMREMAP_WB | MEMREMAP_WT | MEMREMAP_WC);
+	if (!priv->start) {
 		E("slram: memremap failed\n");
 		ret = -EIO;
 		goto err_free_priv;
 	}
-	((slram_priv_t *)(*curmtd)->mtdinfo->priv)->end =
-		((slram_priv_t *)(*curmtd)->mtdinfo->priv)->start + length;
-
-
-	(*curmtd)->mtdinfo->name = name;
-	(*curmtd)->mtdinfo->size = length;
-	(*curmtd)->mtdinfo->flags = MTD_CAP_RAM;
-	(*curmtd)->mtdinfo->_erase = slram_erase;
-	(*curmtd)->mtdinfo->_point = slram_point;
-	(*curmtd)->mtdinfo->_unpoint = slram_unpoint;
-	(*curmtd)->mtdinfo->_read = slram_read;
-	(*curmtd)->mtdinfo->_write = slram_write;
-	(*curmtd)->mtdinfo->owner = THIS_MODULE;
-	(*curmtd)->mtdinfo->type = MTD_RAM;
-	(*curmtd)->mtdinfo->erasesize = SLRAM_BLK_SZ;
-	(*curmtd)->mtdinfo->writesize = 1;
-
-	if (mtd_device_register((*curmtd)->mtdinfo, NULL, 0))	{
+	priv->end = priv->start + length;
+
+	mtdinfo->name = name;
+	mtdinfo->size = length;
+	mtdinfo->flags = MTD_CAP_RAM;
+	mtdinfo->_erase = slram_erase;
+	mtdinfo->_point = slram_point;
+	mtdinfo->_unpoint = slram_unpoint;
+	mtdinfo->_read = slram_read;
+	mtdinfo->_write = slram_write;
+	mtdinfo->owner = THIS_MODULE;
+	mtdinfo->type = MTD_RAM;
+	mtdinfo->erasesize = SLRAM_BLK_SZ;
+	mtdinfo->writesize = 1;
+
+	if (mtd_device_register(mtdinfo, NULL, 0)) {
 		E("slram: Failed to register new device\n");
 		ret = -EAGAIN;
 		goto err_unmap;
 	}
+	*curmtd = new_mtd;
 	T("slram: Registered device %s from %luKiB to %luKiB\n", name,
 			(start / 1024), ((start + length) / 1024));
-	T("slram: Mapped from 0x%p to 0x%p\n",
-			((slram_priv_t *)(*curmtd)->mtdinfo->priv)->start,
-			((slram_priv_t *)(*curmtd)->mtdinfo->priv)->end);
-	return(0);
+	T("slram: Mapped from 0x%p to 0x%p\n", priv->start, priv->end);
+	return 0;
 
 err_unmap:
-	memunmap(((slram_priv_t *)(*curmtd)->mtdinfo->priv)->start);
+	memunmap(priv->start);
 err_free_priv:
-	kfree((*curmtd)->mtdinfo->priv);
+	kfree(priv);
+err_free_mtdinfo:
+	kfree(mtdinfo);
 err_free_list:
-	kfree((*curmtd)->mtdinfo);
-	kfree(*curmtd);
-	*curmtd = NULL;
+	kfree(new_mtd);
 	return ret;
 }
 
-- 
2.51.0


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

Re: [PATCH v2 0/2] mtd: slram: fix failed registration cleanup

[Miquel Raynal]

On Tue, 09 Jun 2026 16:45:26 +0800, Ruoyu Wang wrote:
> This splits the previous slram patch into a backportable fix and a
> follow-up cleanup.
> 
> Patch 1 fixes the dangling list entry on register_device() failure and
> carries Fixes and stable tags.
> 
> Patch 2 is a cleanup which uses local variables while initializing the
> new entry and publishes the list entry only after mtd_device_register()
> succeeds.
> 
> [...]

Applied to mtd/next, thanks!

[1/2] mtd: slram: remove failed entries from the device list
      commit: 36f1648644d769c496a8e47e53603e863e358d73
[2/2] mtd: slram: simplify register_device() cleanup
      commit: 4fe97a54daddb221009964161fb362b2b930d657

Patche(s) should be available on mtd/linux.git and will be
part of the next PR (provided that no robot complains by then).

Kind regards,
Miquèl


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