Re: [PATCH v2 1/4] power: supply: Add registration init callback

[sashiko-bot@kernel.org]

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

New issues:
- [High] Driver hardware resources tied to the parent device will outlive psy on registration failure, causing dangling pointers.

Pre-existing issues:
- [High] Missing cancel_work_sync(&psy->changed_work) on the __power_supply_register error path leads to a Use-After-Free if changed_work is queued.
--

--- Patch [1]: [PATCH v2 1/4] power: supply: Add registration init callback ---
commit 7157b5979312471cbd162dd14b7a0dbf059d5a5a
Author: Vincent Cloutier <vincent@cloutier.co>

power: supply: Add registration init callback
    
Some battery drivers need to consume monitored-battery data before their
power_supply is visible. That lets them prepare hardware configuration from
parsed battery information without racing userspace exposure.

> diff --git a/drivers/power/supply/power_supply_core.c b/drivers/power/supply/power_supply_core.c
> --- a/drivers/power/supply/power_supply_core.c
> +++ b/drivers/power/supply/power_supply_core.c
> @@ -1624,6 +1624,14 @@ __power_supply_register(struct device *parent,
>  	init_rwsem(&psy->extensions_sem);
>  	INIT_LIST_HEAD(&psy->extensions);
>  
> +	if (desc->init) {
> +		rc = desc->init(psy);

[Severity: High]
If a driver implements desc->init() to configure hardware interrupts or
timers that receive the psy pointer as their context, how are those resources
cleaned up if a subsequent step like device_add() fails?

Because there is no desc->exit() or similar teardown callback, the driver
has no hook to synchronously stop its asynchronous resources before the core
subsystem frees psy via put_device() on the error path. If an interrupt
fires before the driver's probe error handling runs, will it dereference the
freed psy pointer?

[Severity: High]
This isn't a bug introduced by this patch, but this change might make it
reachable. If a driver's desc->init() triggers an interrupt that legitimately
calls power_supply_changed(psy), it queues psy->changed_work.

If a subsequent registration step such as device_add() fails,
__power_supply_register() jumps to the error path and unconditionally frees
psy:

__power_supply_register() {
    ...
    device_add_failed:
    check_supplies_failed:
    dev_set_name_failed:
        put_device(dev);
        return ERR_PTR(rc);
}

The error path doesn't cancel psy->changed_work. When the workqueue executes
the pending work, will it access the freed psy memory? Could a
cancel_work_sync(&psy->changed_work) be added to the error path?

> +		if (WARN_ON_ONCE(rc > 0))
> +			rc = -EINVAL;
> +		if (rc)
> +			goto check_supplies_failed;
> +	}
> +
>  	rc = device_add(dev);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720011144.1280219-1-vincent.cloutier@icloud.com?part=1