Re: [PATCH v13 5/6] hwmon: add NXP MC33978/MC34978 driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Background polling worker unconditionally re-arms itself during teardown, leading to a Use-After-Free.
--

commit 120174fd926c55b417dcd675badea0f431859a73
Author: Oleksij Rempel <o.rempel@pengutronix.de>

hwmon: add NXP MC33978/MC34978 driver

This commit adds hardware monitoring support for the NXP MC33978/MC34978
Multiple Switch Detection Interface (MSDI). It implements an edge-triggered
interrupt handler paired with a polling delayed workqueue to manage persistent
hardware faults.

> diff --git a/drivers/hwmon/mc33978-hwmon.c b/drivers/hwmon/mc33978-hwmon.c
> new file mode 100644
> index 0000000000000..bd4dd25b3b20a
> --- /dev/null
> +++ b/drivers/hwmon/mc33978-hwmon.c
[ ... ]
> +static int mc33978_hwmon_update_faults(struct mc33978_hwmon_priv *priv)
> +{
[ ... ]
> +	ret = mc33978_hwmon_read_fault(priv, &faults);
> +	if (ret) {
> +		hwmon_unlock(priv->hwmon_dev);
> +		dev_err_ratelimited(priv->dev,
> +				    "failed to read fault register: %pe\n",
> +				    ERR_PTR(ret));
> +		/*
> +		 * Always retry on read failure. If we drop the heartbeat during
> +		 * the initial fault before caching it, the edge-triggered IRQ
> +		 * will never fire again and permanently stall fault monitoring.
> +		 */
> +		rearm = true;
> +		goto out_poll;
> +	}
[ ... ]
> +	rearm = !!alarm_faults;
> +
> +out_poll:
> +	/*
> +	 * If any alarms are currently active, the global FAULT_STAT bit remains
> +	 * asserted. The hardware will not generate a new rising edge interrupt
> +	 * if a secondary fault occurs, nor will it interrupt when faults clear.
> +	 * Schedule a poll to detect both clearing edges and secondary alarms.
> +	 */
> +	if (rearm)
> +		/* Use freezable polling to pause while the system is suspended. */
> +		mod_delayed_work(system_freezable_wq, &priv->poll_work,
> +				 msecs_to_jiffies(MC33978_FAULT_POLL_INTERVAL_MS));
> +
> +	return ret;
> +}
[ ... ]
> +static void mc33978_hwmon_action_cancel_work(void *data)
> +{
> +	struct mc33978_hwmon_priv *priv = data;
> +
> +	cancel_delayed_work_sync(&priv->poll_work);
> +}

During device removal, devres calls mc33978_hwmon_action_cancel_work(), which
uses cancel_delayed_work_sync(). If the work is concurrently executing,
cancel_delayed_work_sync() will wait for the worker to finish.

Inside the running worker, mc33978_hwmon_update_faults() attempts a
regmap_read(). Since the device is being removed, this read is likely to fail
or return persistent physical faults, setting rearm to true. The worker then
calls mod_delayed_work() and queues the underlying timer to fire again.

When cancel_delayed_work_sync() observes the completion of the execution, it
returns without canceling the newly re-armed timer. The teardown sequence then
continues, freeing the devm-allocated priv structure.

When the timer expires, could this cause a use-after-free when the kernel timer
subsystem dereferences the freed priv->poll_work.timer memory? Should there be
a state flag to prevent re-arming the work during teardown?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518100054.3552143-1-o.rempel@pengutronix.de?part=5