All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC] mtd: block2mtd: fix circular locking dependency in block2mtd_setup
@ 2026-05-10  1:38 syzbot
  2026-05-16 10:08 ` Dmitry Vyukov
  0 siblings, 1 reply; 2+ messages in thread
From: syzbot @ 2026-05-10  1:38 UTC (permalink / raw)
  To: syzkaller-upstream-moderation; +Cc: syzbot

mtd: block2mtd: fix circular locking dependency in block2mtd_setup

syzkaller reported a circular locking dependency involving the global
param_lock and VFS locks (i_rwsem). The block2mtd driver performs a
synchronous VFS path lookup (kern_path via bdev_file_open_by_path)
inside its module parameter set callback (block2mtd_setup). This
callback is invoked by the sysfs core with param_lock held.

The dependency chain is:
1. i_rwsem -> cgroup_mutex (via cgroup_rmdir)
2. cgroup_mutex -> rtnl_mutex (via cgroup_mkdir -> cgrp_css_online)
3. rtnl_mutex -> param_lock (via mac80211_hwsim_new_radio ->
   ieee80211_rate_control_ops_get)
4. param_lock -> i_rwsem (via block2mtd_setup -> kern_path)

To fix this without breaking existing userspace scripts that expect
synchronous device creation, temporarily drop the param_lock during
the device creation process. This breaks the circular locking
dependency while preserving the synchronous nature of the operation.

To implement this safely, introduce a local mutex (block2mtd_mutex)
to protect the module's internal state (blkmtd_device_list,
block2mtd_paramline, and block2mtd_init_called) from concurrent sysfs
writes, since param_lock will no longer provide this protection during
the critical section.

Summary of changes:
1. Introduce block2mtd_mutex to protect the global blkmtd_device_list
   and early-boot state variables.
2. Drop param_lock in block2mtd_setup using kernel_param_unlock(kp->mod)
   and kernel_param_lock(kp->mod). For built-in modules, kp->mod is
   NULL,
   and these functions correctly unlock/lock the global param_lock.
3. Protect block2mtd_setup with block2mtd_mutex to ensure that
concurrent
   sysfs writes are serialized and the device list is safely updated.
4. Protect block2mtd_init and block2mtd_exit with block2mtd_mutex to
   ensure safe access to blkmtd_device_list and block2mtd_paramline.

The lock ordering is now block2mtd_mutex -> VFS locks (i_rwsem) ->
mtd_table_mutex, which is strictly hierarchical and free of circular
dependencies.

Assisted-by: Gemini:gemini-3.1-pro-preview Gemini:gemini-3-flash-preview
To: <joern@lazybastard.org>
To: <linux-mtd@lists.infradead.org>
Cc: <linux-kernel@vger.kernel.org>
Cc: <miquel.raynal@bootlin.com>
Cc: <richard@nod.at>
Cc: <vigneshr@ti.com>

---
diff --git a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c
index 03e80b2c4..f813272da 100644
--- a/drivers/mtd/devices/block2mtd.c
+++ b/drivers/mtd/devices/block2mtd.c
@@ -45,7 +45,7 @@ struct block2mtd_dev {
 
 /* Static info about the MTD, used in cleanup_module */
 static LIST_HEAD(blkmtd_device_list);
-
+static DEFINE_MUTEX(block2mtd_mutex);
 
 static struct page *page_read(struct address_space *mapping, pgoff_t index)
 {
@@ -464,28 +464,43 @@ static int block2mtd_setup2(const char *val)
 
 static int block2mtd_setup(const char *val, const struct kernel_param *kp)
 {
+	int ret = 0;
+
+	/*
+	 * Drop the param_lock to avoid circular locking dependency
+	 * with VFS locks during device lookup.
+	 */
+	kernel_param_unlock(kp->mod);
+
+	mutex_lock(&block2mtd_mutex);
+
 #ifdef MODULE
-	return block2mtd_setup2(val);
+	ret = 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);
+	if (block2mtd_init_called) {
+		ret = block2mtd_setup2(val);
+	} else {
+		/* 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));
+	}
+#endif
 
-	/* 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(). */
+	mutex_unlock(&block2mtd_mutex);
 
-	strscpy(block2mtd_paramline, val, sizeof(block2mtd_paramline));
+	kernel_param_lock(kp->mod);
 
-	return 0;
-#endif
+	return ret;
 }
 
 
@@ -497,9 +512,11 @@ static int __init block2mtd_init(void)
 	int ret = 0;
 
 #ifndef MODULE
+	mutex_lock(&block2mtd_mutex);
 	if (strlen(block2mtd_paramline))
 		ret = block2mtd_setup2(block2mtd_paramline);
 	block2mtd_init_called = 1;
+	mutex_unlock(&block2mtd_mutex);
 #endif
 
 	return ret;
@@ -511,6 +528,7 @@ static void block2mtd_exit(void)
 	struct list_head *pos, *next;
 
 	/* Remove the MTD devices */
+	mutex_lock(&block2mtd_mutex);
 	list_for_each_safe(pos, next, &blkmtd_device_list) {
 		struct block2mtd_dev *dev = list_entry(pos, typeof(*dev), list);
 		block2mtd_sync(&dev->mtd);
@@ -522,6 +540,7 @@ static void block2mtd_exit(void)
 		list_del(&dev->list);
 		block2mtd_free_device(dev);
 	}
+	mutex_unlock(&block2mtd_mutex);
 }
 
 late_initcall(block2mtd_init);


base-commit: 7fd2df204f342fc17d1a0bfcd474b24232fb0f32
-- 
This is an AI-generated patch subject to moderation.
Reply with '#syz upstream' to send it to the mailing list.
Reply with '#syz reject' to reject it.

See  for more information.

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH RFC] mtd: block2mtd: fix circular locking dependency in block2mtd_setup
  2026-05-10  1:38 [PATCH RFC] mtd: block2mtd: fix circular locking dependency in block2mtd_setup syzbot
@ 2026-05-16 10:08 ` Dmitry Vyukov
  0 siblings, 0 replies; 2+ messages in thread
From: Dmitry Vyukov @ 2026-05-16 10:08 UTC (permalink / raw)
  To: syzbot; +Cc: syzkaller-upstream-moderation, syzbot

On Sun, 10 May 2026 at 03:38, 'syzbot' via
syzkaller-upstream-moderation
<syzkaller-upstream-moderation@googlegroups.com> wrote:
>
> mtd: block2mtd: fix circular locking dependency in block2mtd_setup
>
> syzkaller reported a circular locking dependency involving the global
> param_lock and VFS locks (i_rwsem). The block2mtd driver performs a
> synchronous VFS path lookup (kern_path via bdev_file_open_by_path)
> inside its module parameter set callback (block2mtd_setup). This
> callback is invoked by the sysfs core with param_lock held.
>
> The dependency chain is:
> 1. i_rwsem -> cgroup_mutex (via cgroup_rmdir)
> 2. cgroup_mutex -> rtnl_mutex (via cgroup_mkdir -> cgrp_css_online)
> 3. rtnl_mutex -> param_lock (via mac80211_hwsim_new_radio ->
>    ieee80211_rate_control_ops_get)
> 4. param_lock -> i_rwsem (via block2mtd_setup -> kern_path)
>
> To fix this without breaking existing userspace scripts that expect
> synchronous device creation, temporarily drop the param_lock during
> the device creation process. This breaks the circular locking
> dependency while preserving the synchronous nature of the operation.
>
> To implement this safely, introduce a local mutex (block2mtd_mutex)
> to protect the module's internal state (blkmtd_device_list,
> block2mtd_paramline, and block2mtd_init_called) from concurrent sysfs
> writes, since param_lock will no longer provide this protection during
> the critical section.
>
> Summary of changes:
> 1. Introduce block2mtd_mutex to protect the global blkmtd_device_list
>    and early-boot state variables.
> 2. Drop param_lock in block2mtd_setup using kernel_param_unlock(kp->mod)
>    and kernel_param_lock(kp->mod). For built-in modules, kp->mod is
>    NULL,
>    and these functions correctly unlock/lock the global param_lock.
> 3. Protect block2mtd_setup with block2mtd_mutex to ensure that
> concurrent
>    sysfs writes are serialized and the device list is safely updated.
> 4. Protect block2mtd_init and block2mtd_exit with block2mtd_mutex to
>    ensure safe access to blkmtd_device_list and block2mtd_paramline.
>
> The lock ordering is now block2mtd_mutex -> VFS locks (i_rwsem) ->
> mtd_table_mutex, which is strictly hierarchical and free of circular
> dependencies.
>
> Assisted-by: Gemini:gemini-3.1-pro-preview Gemini:gemini-3-flash-preview
> To: <joern@lazybastard.org>
> To: <linux-mtd@lists.infradead.org>
> Cc: <linux-kernel@vger.kernel.org>
> Cc: <miquel.raynal@bootlin.com>
> Cc: <richard@nod.at>
> Cc: <vigneshr@ti.com>
>
> ---
> diff --git a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c
> index 03e80b2c4..f813272da 100644
> --- a/drivers/mtd/devices/block2mtd.c
> +++ b/drivers/mtd/devices/block2mtd.c
> @@ -45,7 +45,7 @@ struct block2mtd_dev {
>
>  /* Static info about the MTD, used in cleanup_module */
>  static LIST_HEAD(blkmtd_device_list);
> -
> +static DEFINE_MUTEX(block2mtd_mutex);
>
>  static struct page *page_read(struct address_space *mapping, pgoff_t index)
>  {
> @@ -464,28 +464,43 @@ static int block2mtd_setup2(const char *val)
>
>  static int block2mtd_setup(const char *val, const struct kernel_param *kp)
>  {
> +       int ret = 0;
> +
> +       /*
> +        * Drop the param_lock to avoid circular locking dependency
> +        * with VFS locks during device lookup.
> +        */
> +       kernel_param_unlock(kp->mod);
> +
> +       mutex_lock(&block2mtd_mutex);
> +
>  #ifdef MODULE
> -       return block2mtd_setup2(val);
> +       ret = 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);
> +       if (block2mtd_init_called) {
> +               ret = block2mtd_setup2(val);
> +       } else {
> +               /* 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));
> +       }
> +#endif
>
> -       /* 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(). */
> +       mutex_unlock(&block2mtd_mutex);
>
> -       strscpy(block2mtd_paramline, val, sizeof(block2mtd_paramline));
> +       kernel_param_lock(kp->mod);
>
> -       return 0;
> -#endif
> +       return ret;
>  }
>
>
> @@ -497,9 +512,11 @@ static int __init block2mtd_init(void)
>         int ret = 0;
>
>  #ifndef MODULE
> +       mutex_lock(&block2mtd_mutex);

Why do we need to hold block2mtd_mutex here and in block2mtd_exit?
What possible executions does this prevent?

>         if (strlen(block2mtd_paramline))
>                 ret = block2mtd_setup2(block2mtd_paramline);
>         block2mtd_init_called = 1;
> +       mutex_unlock(&block2mtd_mutex);
>  #endif
>
>         return ret;
> @@ -511,6 +528,7 @@ static void block2mtd_exit(void)
>         struct list_head *pos, *next;
>
>         /* Remove the MTD devices */
> +       mutex_lock(&block2mtd_mutex);
>         list_for_each_safe(pos, next, &blkmtd_device_list) {
>                 struct block2mtd_dev *dev = list_entry(pos, typeof(*dev), list);
>                 block2mtd_sync(&dev->mtd);
> @@ -522,6 +540,7 @@ static void block2mtd_exit(void)
>                 list_del(&dev->list);
>                 block2mtd_free_device(dev);
>         }
> +       mutex_unlock(&block2mtd_mutex);
>  }
>
>  late_initcall(block2mtd_init);
>
>
> base-commit: 7fd2df204f342fc17d1a0bfcd474b24232fb0f32
> --
> This is an AI-generated patch subject to moderation.
> Reply with '#syz upstream' to send it to the mailing list.
> Reply with '#syz reject' to reject it.
>
> See  for more information.
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-upstream-moderation" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-upstream-moderation+unsubscribe@googlegroups.com.
> To view this discussion visit https://groups.google.com/d/msgid/syzkaller-upstream-moderation/9ceb3eb3-40e2-497b-8d68-257fd88a3946%40mail.kernel.org.

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-05-16 10:09 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-10  1:38 [PATCH RFC] mtd: block2mtd: fix circular locking dependency in block2mtd_setup syzbot
2026-05-16 10:08 ` Dmitry Vyukov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.