Re: [PATCH 2/2] PCI/pwrctrl: tc9563: use devm-managed I2C dummy device allocation

[email protected]
Newsgroups org.kernel.vger.linux-pci
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Use-After-Free due to teardown order inversion between i2c_put_adapter() and devm-managed dummy client release.
--

--- Patch [2]: [PATCH 2/2] PCI/pwrctrl: tc9563: use devm-managed I2C dummy device allocation ---
commit f54ecfd3f2485cf33e559cd8f58fa24de337138b
Author: Lorenzo Bianconi <[email protected]>

PCI/pwrctrl: tc9563: use devm-managed I2C dummy device allocation

This patch converts the driver to use devm_i2c_new_dummy_device to manage
the I2C dummy client. It removes the manual i2c_unregister_device calls
from the probe error path and the remove function.

> diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> index 8bccf3abd1892..5b05e76f6c80d 100644
> --- a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> +++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
[ ... ]
> @@ -607,7 +606,6 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
>  power_off:
>  	tc9563_pwrctrl_power_off(&tc9563->pwrctrl);
>  remove_i2c:
> -	i2c_unregister_device(tc9563->client);
>  	i2c_put_adapter(tc9563->adapter);
>  	return ret;
>  }

[Severity: High]
Will this cause a use-after-free by changing the teardown order?

By switching to devm_i2c_new_dummy_device, the dummy client is now
unregistered by devres cleanup, which executes after tc9563_pwrctrl_probe
returns. 

Because i2c_put_adapter is still called manually before returning, the
adapter reference is dropped before devres unregisters the dummy client. If
dropping this reference allows the adapter to be freed concurrently, it would
forcibly unregister all children. When devres later attempts to unregister
the client, it would operate on an already-freed pointer.

Could devm_add_action_or_reset be used to manage the i2c_put_adapter call
so the teardown sequence happens in the correct reverse order?

[ ... ]
> @@ -619,7 +617,6 @@ static void tc9563_pwrctrl_remove(struct platform_device *pdev)
>  					struct tc9563_pwrctrl, pwrctrl);
>  
>  	tc9563_pwrctrl_power_off(&tc9563->pwrctrl);
> -	i2c_unregister_device(tc9563->client);
>  	i2c_put_adapter(tc9563->adapter);
>  }

[Severity: High]
Does this also expose driver unbind to the same teardown order inversion?

Since tc9563_pwrctrl_remove executes before devres cleanup, dropping the
adapter reference here deterministically happens before the dummy client is
unregistered.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-pci-pwrctrl-tc956x-regmap-v1-0-20ce61a3ce12@oss.qualcomm.com?part=2
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.