Re: [syzbot] [fs?] possible deadlock in ovl_create_object (2)
From: Chris Roy
Date: Sat Sep 19 2026 - 04:52:48 EST
On Sat, Sep 19, 2026, Jörn Engel wrote:
> This is just awful taste. You are inside a file called "block2mtd".
> The prefix to the mutex an workqueue add absolutely nothing.
> [...]
> Pick a name that gives us some information like that, please!
Agreed. v4 renames to list_mutex and setup_wq. Additionally, I fixed
tag formatting and removed a redundant if-guard.
On Sat, Sep 19, 2026, Richard Weinberger wrote:
> While we're here, maybe it's time to add a decent configfs interface
> to this driver instead of configuring through module parameters.
I will look at a configfs interface as a follow up once this
lockdep fix lands.
#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
Regards,
Chris
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Chris Roy <iam@xxxxxxxxxxx>
Date: Sat, 19 Sep 2026 00:00:00 +0000
Subject: [PATCH v4] mtd: block2mtd: defer device open out of param/sysfs write
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.
Changes since v3:
- 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)
Changes since v2:
- rewrite the new comments to match the rest of the file
- add Assisted-by
Changes since v1:
- 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
Reported-by: syzbot+7cab6a19619f1b8efc00@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=7cab6a19619f1b8efc00
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Chris Roy <iam@xxxxxxxxxxx>
---
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(). */
+ int ret = 0;
- strscpy(block2mtd_paramline, val, sizeof(block2mtd_paramline));
+ if (!try_module_get(kp->mod))
+ return -ENODEV;
- return 0;
+ 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);
--
2.43.0