Re: [PATCH RFC v2] mtd: block2mtd: fix circular locking dependency in block2mtd_setup

On Sat, 16 May 2026 at 17:41, 'syzbot' via
syzkaller-upstream-moderation
<[email protected]> wrote:
>
> 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.
>
> Fixes: f9d8c3c4236ef2a1c15b01200b9d164e2c5a54c7 ("block2mtd: port device access to files")
> Assisted-by: Gemini:gemini-3.1-pro-preview Gemini:gemini-3-flash-preview
> Reported-by: [email protected]
> Link: https://syzkaller.appspot.com/bug?extid=65459fd3b61877d717a3
> Link: https://syzkaller.appspot.com/ai_job?id=c3c46711-9492-41e5-bcb4-4fb68a746c23
> To: <[email protected]>
> To: <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
> Cc: <[email protected]>
>
> ---
> v2:
> - Added comments in block2mtd_init() and block2mtd_exit() explaining the need for block2mtd_mutex to protect against concurrent sysfs writes.
>
> v1:
> https://lore.kernel.org/all/[email protected]/T/
> ---
> diff --git a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c
> index 03e80b2c4..42da6b9a5 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,15 @@ static int __init block2mtd_init(void)
>         int ret = 0;
>
>  #ifndef MODULE
> +       /*
> +        * block2mtd_mutex protects block2mtd_init_called and
> +        * block2mtd_paramline against concurrent sysfs writes.
> +        */
> +       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 +532,11 @@ static void block2mtd_exit(void)
>         struct list_head *pos, *next;
>
>         /* Remove the MTD devices */
> +       /*
> +        * block2mtd_mutex protects blkmtd_device_list against
> +        * concurrent sysfs writes.
> +        */
> +       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 +548,7 @@ static void block2mtd_exit(void)
>                 list_del(&dev->list);
>                 block2mtd_free_device(dev);
>         }
> +       mutex_unlock(&block2mtd_mutex);
>  }
>
>  late_initcall(block2mtd_init);

  The deadlock resolution is correct, but dropping  param_lock
exposes a critical Use-After-Free (UAF) race condition during module
unloading.

  Because sysfs writes do not hold a module reference,  delete_module
(rmmod) can proceed concurrently with an active sysfs parameter write.

  1.  rmmod  starts, sets module state to  GOING , and calls  block2mtd_exit() .
  2.  block2mtd_exit()  acquires  block2mtd_mutex , unregisters all
active devices, clears  blkmtd_device_list , and releases
block2mtd_mutex .
  3. A concurrent sysfs write (which dropped  param_lock  to avoid the
deadlock and was blocked on  block2mtd_mutex ) now acquires the mutex.
  4. It calls  block2mtd_setup2()  $\rightarrow$  add_device() ,
registers a new MTD device, and adds it to  blkmtd_device_list .
  5. The sysfs write releases  block2mtd_mutex  and completes.
  6.  delete_module  completes and frees the module memory.

  This leaves an active MTD device registered in the kernel pointing
to freed module code ( dev->mtd._read , etc.), leading to an immediate
kernel panic on next access.

  To prevent this, we must serialize the module exit state. Introduce
a  block2mtd_exiting  flag synchronized by  block2mtd_mutex  to reject
new registrations during unload.

    diff --git a/drivers/mtd/devices/block2mtd.c
b/drivers/mtd/devices/block2mtd.c
    index f813272dad88..b12a84cf0e12 100644
    --- a/drivers/mtd/devices/block2mtd.c
    +++ b/drivers/mtd/devices/block2mtd.c
    @@ -46,6 +46,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 bool block2mtd_exiting;

     static struct page *page_read(struct address_space *mapping, pgoff_t index)
     {
    @@ -477,6 +478,11 @@ static int block2mtd_setup(const char *val,
const struct kernel_param *kp)

         mutex_lock(&block2mtd_mutex);

    +    if (block2mtd_exiting) {
    +        ret = -ENODEV;
    +        goto out_unlock;
    +    }
    +
     #ifdef MODULE
         ret = block2mtd_setup2(val);
     #else
    @@ -498,6 +504,7 @@ static int block2mtd_setup(const char *val,
const struct kernel_param *kp)
         }
     #endif

    +out_unlock:
         mutex_unlock(&block2mtd_mutex);

         kernel_param_lock(kp->mod);
    @@ -529,6 +536,7 @@ static void block2mtd_exit(void)

         /* Remove the MTD devices */
         mutex_lock(&block2mtd_mutex);
    +    block2mtd_exiting = true;
         list_for_each_safe(pos, next, &blkmtd_device_list) {
             struct block2mtd_dev *dev = list_entry(pos, typeof(*dev), list);
             block2mtd_sync(&dev->mtd);