Re: [PATCH v2 1/3] regulator: core: use system_freezable_wq for init complete work

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] regulator_unregister() uses flush_work() on a delayed_work, failing to cancel pending timers and leading to Use-After-Free.
- [High] Delayed work 'oc_work' is never canceled on unbind in mt6363-regulator, risking timer list corruption or Use-After-Free.
- [High] Delayed works 'sc_recovery_work' and 'ocp_recovery_work' are never canceled on unbind in qcom-labibb-regulator.
--

--- Patch [1]: [PATCH v2 1/3] regulator: core: use system_freezable_wq for init complete work ---
commit 7f543933219647c14311f3aa93aefaa15b1b5acf
Author: Joy Zou <joy.zou@oss.nxp.com>

regulator: core: use system_freezable_wq for init complete work

This patch fixes a race condition between the delayed regulator init
completion work and system suspend. By switching from system_wq to
system_freezable_wq, the init work is now frozen during suspend,
preventing it from running when the I2C adapter is already suspended.

> diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
> index 2e61606fc1d05..6a4008f387b5e 100644
> --- a/drivers/regulator/core.c
> +++ b/drivers/regulator/core.c
[ ... ]
> @@ -6899,8 +6900,9 @@ static int __init regulator_init_complete(void)
>  	 * we'd only do this on systems that need it, and a kernel
>  	 * command line option might be useful.
>  	 */
> -	schedule_delayed_work(&regulator_init_complete_work,
> -			      msecs_to_jiffies(30000));
> +	queue_delayed_work(system_freezable_wq,
> +			   &regulator_init_complete_work,
> +			   msecs_to_jiffies(30000));
>  
>  	return 0;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the use of
workqueues and timers in the regulator subsystem, I noticed that
regulator_unregister() uses flush_work() on a delayed_work instead of
cancel_delayed_work_sync():

drivers/regulator/core.c:regulator_unregister() {
...
	if (rdev->supply) {
		regulator_unregister_notifier(rdev->supply,
					      &rdev->supply_fwd_nb);

		while (rdev->use_count--)
			regulator_disable(rdev->supply);
		regulator_put(rdev->supply);
	}

	flush_work(&rdev->disable_work.work);
...
}

Because flush_work() returns immediately if the delayed_work timer is
pending but not yet queued on a workqueue, this fails to cancel the pending
timer.

When the timer eventually expires, it dereferences the freed rdev memory.

Can this lead to a use-after-free during hardware removal or module unloading?

[Severity: High]
This isn't a bug introduced by this patch, but I also noticed missing
teardown for delayed work in the mt6363-regulator driver:

drivers/regulator/mt6363-regulator.c:mt6363_regulator_probe() {
...
		config.driver_data = info;
		INIT_DELAYED_WORK(&info->oc_work, mt6363_oc_irq_enable_work);

		rdev = devm_regulator_register(dev, &info->desc, &config);
...
}

The delayed work 'oc_work' is initialized on a static array but is never
canceled on unbind.

If an overcurrent interrupt fires and schedules oc_work, and the device is
unbound before the timer expires, the timer remains pending.

If unbound and re-bound quickly, could the subsequent INIT_DELAYED_WORK()
overwrite the active timer's list node and corrupt the timer tree?

Or, if the module is unloaded, could the timer fire and execute code in
unmapped memory, causing a use-after-free?

[Severity: High]
This isn't a bug introduced by this patch, but there is a similar missing
teardown issue in the qcom-labibb-regulator driver:

drivers/regulator/qcom-labibb-regulator.c:qcom_labibb_regulator_probe() {
...
		INIT_DELAYED_WORK(&vreg->sc_recovery_work,
				  qcom_labibb_sc_recovery_worker);

		if (vreg->ocp_irq > 0)
			INIT_DELAYED_WORK(&vreg->ocp_recovery_work,
					  qcom_labibb_ocp_recovery_worker);
...
}

The delayed works 'sc_recovery_work' and 'ocp_recovery_work' point to a
devm-allocated structure.

Since there is no devm action or explicit teardown to cancel these delayed
works on unbind, devres will automatically free the vreg structure.

If the device unbinds while these timers are active, will the timers expire
and dereference the freed vreg pointer, causing a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-b4-regulator-pf01-v2-0-a406c8737fdb@oss.nxp.com?part=1