[PATCH v2] mtd: Fix refcounting with MTD_PARTITIONED_MASTER

Miquel Raynal miquel.raynal at bootlin.com
Mon Jul 31 02:09:03 PDT 2023


The logic is way too convoluted, let's clean the kref_get/put section to
clarify what this block does, hopefully solving the refcounting issue
when using CONFIG_MTD_PARTITIONED_MASTER at the same time:
- Iterate through all the parent mtd devices
- Grab a reference over them all but the master
- Only grab the master whith CONFIG_MTD_PARTITIONED_MASTER
Same logic must apply in the put path, otherwise it would be broken.

Cc: Tomas Winkler <tomas.winkler at intel.com>
Cc: Alexander Usyskin <alexander.usyskin at intel.com>
Cc: Zhang Xiaoxu <zhangxiaoxu5 at huawei.com>
Fixes: 19bfa9ebebb5 ("mtd: use refcount to prevent corruption")
Signed-off-by: Miquel Raynal <miquel.raynal at bootlin.com>

---

Hello,

Alexander, Zhang, please test this version, it is close to what Zhang
produced while not strictly identical. This compile-tested only, please
check on you side whether it fixes the refcounting issue with and
without PARTITIONED_MASTER, as well as with Kasan.

Alexander, maybe there is something else to fix, in all cases the logic
here was broken so let's start by this one and see if we need something
else on top.

Changes in v2:
* Grab the parent before calling kref_put() to avoid accessing a freed
  mtd pointer.

Thanks,
Miquèl
---
 drivers/mtd/mtdcore.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 2466ea466466..9e8ff3a999de 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -1241,14 +1241,15 @@ int __get_mtd_device(struct mtd_info *mtd)
 		return -ENODEV;
 	}
 
-	kref_get(&mtd->refcnt);
-
-	while (mtd->parent) {
-		if (IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER) || mtd->parent != master)
-			kref_get(&mtd->parent->refcnt);
+	while (mtd) {
+		if (mtd != master)
+			kref_get(&mtd->refcnt);
 		mtd = mtd->parent;
 	}
 
+	if (IS_ENABLED(CONFIG_MTD_PARTITIONED_MASTER))
+		kref_get(&master->refcnt);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(__get_mtd_device);
@@ -1332,10 +1333,12 @@ void __put_mtd_device(struct mtd_info *mtd)
 {
 	struct mtd_info *master = mtd_get_master(mtd);
 
-	while (mtd != master) {
+	while (mtd) {
+		/* kref_put() can relese mtd, so keep a reference mtd->parent */
 		struct mtd_info *parent = mtd->parent;
 
-		kref_put(&mtd->refcnt, mtd_device_release);
+		if (mtd != master)
+			kref_put(&mtd->refcnt, mtd_device_release);
 		mtd = parent;
 	}
 
-- 
2.34.1




More information about the linux-mtd mailing list