lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Message-ID: <20250325133954.3699535-1-liwei.song.lsong@gmail.com>
Date: Tue, 25 Mar 2025 21:39:52 +0800
From: Liwei Song <liwei.song.lsong@...il.com>
To: MiquelRaynal <miquel.raynal@...tlin.com>,
	RichardWeinberger <richard@....at>,
	VigneshRaghavendra <vigneshr@...com>
Cc: linux-mtd@...ts.infradead.org,
	linux-kernel@...r.kernel.org,
	liwei.song.lsong@...il.com
Subject: [PATCH] mtd: core: add sync between read/write and unbind device

When unbinding mtd device or qspi controller with a high frequency
reading to /dev/mtd0 device, there will be Calltrace as below:

$ while true; do cat /dev/mtd0 >/dev/null; done &
$ echo ff8d2000.spi  > /sys/bus/platform/drivers/cadence-qspi/unbind

Internal error: synchronous external abort: 0000000096000210 [#1] PREEMPT SMP
Modules linked in:
CPU: 3 UID: 0 PID: 466 Comm: cat Not tainted 6.14.0-rc7-yocto-standard+ #1
Hardware name: SoCFPGA Stratix 10 SoCDK (DT)
pc : cqspi_indirect_read_execute.isra.0+0x188/0x330
lr : cqspi_indirect_read_execute.isra.0+0x21c/0x330
Call trace:
 cqspi_indirect_read_execute.isra.0+0x188/0x330 (P)
 cqspi_exec_mem_op+0x8bc/0xe40
 spi_mem_exec_op+0x3e0/0x478
 spi_mem_no_dirmap_read+0xa8/0xc8
 spi_mem_dirmap_read+0xdc/0x150
 spi_nor_read_data+0x120/0x198
 spi_nor_read+0xf0/0x280
 mtd_read_oob_std+0x80/0x98
 mtd_read_oob+0x9c/0x168
 mtd_read+0x6c/0xd8
 mtdchar_read+0xdc/0x288
 vfs_read+0xc8/0x2f8
 ksys_read+0x70/0x110
 __arm64_sys_read+0x24/0x38
 invoke_syscall+0x5c/0x130
 el0_svc_common.constprop.0+0x48/0xf8
 do_el0_svc+0x28/0x40
 el0_svc+0x30/0xd0
 el0t_64_sync_handler+0x144/0x168
 el0t_64_sync+0x198/0x1a0
Code: 927e7442 aa1a03e0 8b020342 d503201f (b9400321)
---[ end trace 0000000000000000 ]---

Or:
$ while true; do cat /dev/mtd0 >/dev/null; done &
$ echo spi0.0 > /sys/class/mtd/mtd0/device/driver/unbind

Unable to handle kernel paging request at virtual address 00000000000012e8
Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
Modules linked in:
CPU: 2 UID: 0 PID: 459 Comm: cat Not tainted 6.14.0-rc7-yocto-standard+ #1
Hardware name: SoCFPGA Stratix 10 SoCDK (DT)
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : spi_mem_exec_op+0x3e8/0x478
lr : spi_mem_exec_op+0x3e0/0x478
Call trace:
 spi_mem_exec_op+0x3e8/0x478 (P)
 spi_mem_no_dirmap_read+0xa8/0xc8
 spi_mem_dirmap_read+0xdc/0x150
 spi_nor_read_data+0x120/0x198
 spi_nor_read+0xf0/0x280
 mtd_read_oob_std+0x80/0x98
 mtd_read_oob+0x9c/0x168
 mtd_read+0x6c/0xd8
 mtdchar_read+0xdc/0x288
 vfs_read+0xc8/0x2f8
 ksys_read+0x70/0x110
 __arm64_sys_read+0x24/0x38
 invoke_syscall+0x5c/0x130
 el0_svc_common.constprop.0+0x48/0xf8
 do_el0_svc+0x28/0x40
 el0_svc+0x30/0xd0
 el0t_64_sync_handler+0x144/0x168
 el0t_64_sync+0x198/0x1a0
Code: f9400842 d63f0040 2a0003f4 f94002a1 (f9417437)
---[ end trace 0000000000000000 ]---

when unbind is running, the memory allocated to qspi controller and
mtd device is freed during unbinding, but open/close and reading device
are still running, if the reading process get read lock and start
excuting, there will be above illegal memory access. This issue also
can be repruduced on many other platforms like ls1046 and nxpimx8 which
have qspi flash.

In this patch, register a spi bus notifier which will called before
unbind process free device memory, and add a new member mtd_event_remove
to block mtd open/read, then wait for the running task to be finished,
then free memory is safe to be run.

Signed-off-by: Liwei Song <liwei.song.lsong@...il.com>
---
 drivers/mtd/mtdcore.c   | 57 +++++++++++++++++++++++++++++++++++++++++
 include/linux/mtd/mtd.h |  2 ++
 2 files changed, 59 insertions(+)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index 724f917f91ba..d35c2ecd5d11 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -34,6 +34,10 @@
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
+#include <linux/spi/spi.h>
+#include <linux/spi/spi-mem.h>
+#include <linux/mtd/spi-nor.h>
+#include <linux/delay.h>
 
 #include "mtdcore.h"
 
@@ -422,6 +426,48 @@ static int mtd_reboot_notifier(struct notifier_block *n, unsigned long state,
 	return NOTIFY_DONE;
 }
 
+static int mtd_remove_notifier_call(struct notifier_block *nb,
+				    unsigned long event, void *data)
+{
+	struct device *dev = data;
+	struct spi_device *spi = to_spi_device(dev);
+	struct spi_mem *mem = spi_get_drvdata(spi);
+	struct spi_nor *nor = mem->drvpriv;
+	struct mtd_info *master = &nor->mtd;
+	struct mtd_info *child, *next;
+	int ret;
+	bool removed;
+
+	switch (event) {
+	case BUS_NOTIFY_BOUND_DRIVER:
+		master->mtd_event_remove = false;
+		break;
+	case BUS_NOTIFY_DEL_DEVICE:
+	case BUS_NOTIFY_UNBIND_DRIVER:
+		mutex_lock(&mtd_table_mutex);
+		master->mtd_event_remove = true;
+		mutex_unlock(&mtd_table_mutex);
+
+		while (1) {
+			removed = true;
+			list_for_each_entry_safe(child, next, &master->partitions, part.node) {
+				mutex_lock(&mtd_table_mutex);
+				ret = kref_read(&child->refcnt);
+				if (ret > 1)
+					removed = false;
+				mutex_unlock(&mtd_table_mutex);
+			}
+			msleep(500);
+			if (removed)
+				break;
+		}
+
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
 /**
  * mtd_wunit_to_pairing_info - get pairing information of a wunit
  * @mtd: pointer to new MTD device info structure
@@ -1099,6 +1145,11 @@ int mtd_device_parse_register(struct mtd_info *mtd, const char * const *types,
 		register_reboot_notifier(&mtd->reboot_notifier);
 	}
 
+	if (!mtd->mtd_remove_nb.notifier_call) {
+		mtd->mtd_remove_nb.notifier_call = mtd_remove_notifier_call;
+		bus_register_notifier(&spi_bus_type, &mtd->mtd_remove_nb);
+	}
+
 out:
 	if (ret) {
 		nvmem_unregister(mtd->otp_user_nvmem);
@@ -1127,6 +1178,9 @@ int mtd_device_unregister(struct mtd_info *master)
 		memset(&master->reboot_notifier, 0, sizeof(master->reboot_notifier));
 	}
 
+	bus_unregister_notifier(&spi_bus_type, &master->mtd_remove_nb);
+	memset(&master->mtd_remove_nb, 0, sizeof(master->mtd_remove_nb));
+
 	nvmem_unregister(master->otp_user_nvmem);
 	nvmem_unregister(master->otp_factory_nvmem);
 
@@ -1243,6 +1297,9 @@ int __get_mtd_device(struct mtd_info *mtd)
 	struct mtd_info *master = mtd_get_master(mtd);
 	int err;
 
+	if (master->mtd_event_remove)
+		return -ENODEV;
+
 	if (master->_get_device) {
 		err = master->_get_device(mtd);
 		if (err)
diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 8d10d9d2e830..74a17eb207ad 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -290,6 +290,7 @@ struct mtd_info {
 	/* Kernel-only stuff starts here. */
 	const char *name;
 	int index;
+	bool mtd_event_remove;
 
 	/* OOB layout description */
 	const struct mtd_ooblayout_ops *ooblayout;
@@ -369,6 +370,7 @@ struct mtd_info {
 	bool oops_panic_write;
 
 	struct notifier_block reboot_notifier;  /* default mode before reboot */
+	struct notifier_block mtd_remove_nb;
 
 	/* ECC status information */
 	struct mtd_ecc_stats ecc_stats;
-- 
2.40.0


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ