[PATCH v4] mtd: block2mtd: defer device open out of param/sysfs write

[Chris Roy <iam@thechris.in>]

block2mtd_setup() opens the named block device while still under
param_lock, and on the sysfs write path under kernfs (and possibly a
splice pipe lock). That nests VFS locking the wrong way relative to
overlayfs and trips lockdep.

Drop param_lock and run setup on a dedicated ordered workqueue. Keep
the call synchronous with wait_for_completion(). Allocate the work on
the heap so DEBUG_OBJECTS_WORK stays quiet.

Reported-by: syzbot+7cab6a19619f1b8efc00@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7cab6a19619f1b8efc00
Tested-by: syzbot+7cab6a19619f1b8efc00@syzkaller.appspotmail.com
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Chris Roy <iam@thechris.in>
---
v4:
 - rename list_mutex / setup_wq to say what they are for
 - tidy new comments
 - fix Assisted-by tag format (checkpatch: AGENT_NAME:MODEL_VERSION)
 - drop redundant setup_wq check in block2mtd_setup_defer() (the sole
   caller already gates on it)
 - claim Tested-by from syzbot (granted on v2, v3, and this content)
v3:
 - rewrite the new comments to match the rest of the file
 - add Assisted-by
v2:
 - heap-allocated work (v1 tripped DEBUG_OBJECTS_WORK)
 - dedicated ordered workqueue instead of system_wq
 - module reference across the deferred open
 - flush/destroy the workqueue before exit teardown
 - serialize setup2 on the worker under list_mutex
 - early-boot paramline updates under that mutex

Not proposed for stable. block2mtd has no known production use (per
Richard Weinberger, testing is the only real use case), so there is
no backport trail worth chasing here.

 drivers/mtd/devices/block2mtd.c | 116 ++++++++++++++++++++++++++------
 1 file changed, 95 insertions(+), 21 deletions(-)

diff --git a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c
index 03e80b2..a540089 100644
--- a/drivers/mtd/devices/block2mtd.c
+++ b/drivers/mtd/devices/block2mtd.c
@@ -27,6 +27,8 @@
 #include <linux/init.h>
 #include <linux/mtd/mtd.h>
 #include <linux/mutex.h>
+#include <linux/workqueue.h>
+#include <linux/completion.h>
 #include <linux/mount.h>
 #include <linux/slab.h>
 #include <linux/major.h>
@@ -45,6 +47,9 @@ struct block2mtd_dev {
 
 /* Static info about the MTD, used in cleanup_module */
 static LIST_HEAD(blkmtd_device_list);
+/* Protects blkmtd_device_list and early-boot paramline updates */
+static DEFINE_MUTEX(list_mutex);
+static struct workqueue_struct *setup_wq;
 
 
 static struct page *page_read(struct address_space *mapping, pgoff_t index)
@@ -461,31 +466,85 @@ static int block2mtd_setup2(const char *val)
 	return 0;
 }
 
+struct block2mtd_setup_work {
+	struct work_struct work;
+	struct completion done;
+	char *val;
+	int ret;
+};
+
+static void block2mtd_setup_workfn(struct work_struct *work)
+{
+	struct block2mtd_setup_work *w =
+		container_of(work, struct block2mtd_setup_work, work);
+
+	mutex_lock(&list_mutex);
+	w->ret = block2mtd_setup2(w->val);
+	mutex_unlock(&list_mutex);
+	complete(&w->done);
+}
+
+/* Runs block2mtd_setup2() on setup_wq, blocking until it completes */
+static int block2mtd_setup_defer(const char *val)
+{
+	struct block2mtd_setup_work *w;
+	int ret;
+
+	w = kzalloc(sizeof(*w), GFP_KERNEL);
+	if (!w)
+		return -ENOMEM;
+
+	w->val = kstrdup(val, GFP_KERNEL);
+	if (!w->val) {
+		kfree(w);
+		return -ENOMEM;
+	}
+
+	init_completion(&w->done);
+	INIT_WORK(&w->work, block2mtd_setup_workfn);
+	queue_work(setup_wq, &w->work);
+	wait_for_completion(&w->done);
+
+	ret = w->ret;
+	kfree(w->val);
+	kfree(w);
+	return ret;
+}
 
 static int block2mtd_setup(const char *val, const struct kernel_param *kp)
 {
-#ifdef MODULE
-	return block2mtd_setup2(val);
-#else
-	/* If more parameters are later passed in via
-	   /sys/module/block2mtd/parameters/block2mtd
-	   and block2mtd_init() has already been called,
-	   we can parse the argument now. */
-
-	if (block2mtd_init_called)
-		return block2mtd_setup2(val);
-
-	/* During early boot stage, we only save the parameters
-	   here. We must parse them later: if the param passed
-	   from kernel boot command line, block2mtd_setup() is
-	   called so early that it is not possible to resolve
-	   the device (even kmalloc() fails). Deter that work to
-	   block2mtd_setup2(). */
-
-	strscpy(block2mtd_paramline, val, sizeof(block2mtd_paramline));
+	int ret = 0;
 
-	return 0;
+	if (!try_module_get(kp->mod))
+		return -ENODEV;
+
+	kernel_param_unlock(kp->mod);
+
+#ifndef MODULE
+	mutex_lock(&list_mutex);
+	if (!block2mtd_init_called) {
+		/* Cannot resolve block devices this early */
+		strscpy(block2mtd_paramline, val, sizeof(block2mtd_paramline));
+		mutex_unlock(&list_mutex);
+		kernel_param_lock(kp->mod);
+		module_put(kp->mod);
+		return 0;
+	}
+	mutex_unlock(&list_mutex);
 #endif
+
+	if (setup_wq) {
+		ret = block2mtd_setup_defer(val);
+	} else {
+		/* Not yet deferred to setup_wq; safe to call setup2 directly */
+		mutex_lock(&list_mutex);
+		ret = block2mtd_setup2(val);
+		mutex_unlock(&list_mutex);
+	}
+
+	kernel_param_lock(kp->mod);
+	module_put(kp->mod);
+	return ret;
 }
 
 
@@ -496,10 +555,17 @@ static int __init block2mtd_init(void)
 {
 	int ret = 0;
 
+	setup_wq = alloc_ordered_workqueue("block2mtd", 0);
+	if (!setup_wq)
+		return -ENOMEM;
+
 #ifndef MODULE
+	mutex_lock(&list_mutex);
 	if (strlen(block2mtd_paramline))
 		ret = block2mtd_setup2(block2mtd_paramline);
+	/* Avoid racing sysfs with the early paramline */
 	block2mtd_init_called = 1;
+	mutex_unlock(&list_mutex);
 #endif
 
 	return ret;
@@ -510,9 +576,16 @@ static void block2mtd_exit(void)
 {
 	struct list_head *pos, *next;
 
-	/* Remove the MTD devices */
+	if (setup_wq) {
+		flush_workqueue(setup_wq);
+		destroy_workqueue(setup_wq);
+		setup_wq = NULL;
+	}
+
+	mutex_lock(&list_mutex);
 	list_for_each_safe(pos, next, &blkmtd_device_list) {
 		struct block2mtd_dev *dev = list_entry(pos, typeof(*dev), list);
+
 		block2mtd_sync(&dev->mtd);
 		mtd_device_unregister(&dev->mtd);
 		mutex_destroy(&dev->write_mutex);
@@ -522,6 +595,7 @@ static void block2mtd_exit(void)
 		list_del(&dev->list);
 		block2mtd_free_device(dev);
 	}
+	mutex_unlock(&list_mutex);
 }
 
 late_initcall(block2mtd_init);

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