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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mtd: block2mtd: use-after-free and resource leak on init failure
- [High] mtd: block2mtd: sysfs lock inversion bypasses workqueue deferral
--

commit c3f3ebd86ecc63dd07e6a22d8527eadaeb761633
Author: Chris Roy <iam@thechris.in>

mtd: block2mtd: defer device open out of param/sysfs write

This patch defers block2mtd_setup() device opening to a dedicated ordered
workqueue to prevent lockdep-detected lock inversion between VFS and
kernfs/overlayfs during sysfs writes. The setup work is executed via a
dynamically allocated work item, preserving synchronous behavior with
wait_for_completion().

Closes: https://syzkaller.appspot.com/bug?extid=7cab6a19619f1b8efc00

> diff --git a/drivers/mtd/devices/block2mtd.c b/drivers/mtd/devices/block2mtd.c
> index 349fa07be314c..30ac9325bdd2b 100644
> --- a/drivers/mtd/devices/block2mtd.c
> +++ b/drivers/mtd/devices/block2mtd.c
[ ... ]
> @@ -461,31 +466,85 @@ static int block2mtd_setup2(const char *val)
[ ... ]
>  static int block2mtd_setup(const char *val, const struct kernel_param *kp)
>  {
[ ... ]
> +	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);
> +	}

[Severity: High]
Does evaluating setup_wq here bypass the workqueue deferral and re-introduce
the sysfs lock inversion?

Since mod_sysfs_setup() exposes module parameters to sysfs before
do_init_module() calls block2mtd_init(), setup_wq can be NULL during a
concurrent sysfs write.

This would cause the sysfs writer to execute block2mtd_setup2() inline under
the sysfs kernfs lock.

Additionally, since setup_wq is assigned in block2mtd_init() without memory
barriers (such as smp_store_release() and smp_load_acquire()), concurrent
sysfs writers might observe a stale NULL pointer even after initialization
completes.

> +
> +	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;

[Severity: High]
Can returning -ENOMEM here lead to a use-after-free and resource leak?

If parameters are provided during module load
(e.g., insmod block2mtd.ko block2mtd=...), block2mtd_setup() is executed
before block2mtd_init(). Because setup_wq is NULL at that time, it
synchronously registers the MTD device in the block2mtd_setup() else branch
shown above.

If alloc_ordered_workqueue() subsequently fails here, it returns -ENOMEM and
aborts the module load.

The kernel frees the module memory without calling block2mtd_exit(), leaving
the registered MTD device holding dangling function pointers to the unloaded
module text.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920185132.1266699-1-iam@thechris.in?part=1