* [PATCH] PCI/pwrctrl: tc9563: take i2c adapter module reference
@ 2026-07-16 13:25 Johan Hovold
2026-07-16 13:41 ` sashiko-bot
2026-07-16 16:47 ` Manivannan Sadhasivam
0 siblings, 2 replies; 3+ messages in thread
From: Johan Hovold @ 2026-07-16 13:25 UTC (permalink / raw)
To: Bartosz Golaszewski, Manivannan Sadhasivam
Cc: Bjorn Helgaas, linux-pci, linux-kernel, Johan Hovold
The i2c subsystem currently blocks during adapter deregistration
whenever there are consumers holding a reference.
Switch to using of_get_i2c_adapter_by_node() which also takes a
reference to the adapter module so that an attempt to unload the module
while in use fails gracefully instead of blocking uninterruptibly.
Signed-off-by: Johan Hovold <johan@kernel.org>
---
drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
index 1555e8a9b3ca..e6a8abd9da8e 100644
--- a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
+++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
@@ -548,7 +548,7 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
return dev_err_probe(dev, ret, "Failed to read i2c-parent property\n");
i2c_node = of_parse_phandle(dev->of_node, "i2c-parent", 0);
- tc9563->adapter = of_find_i2c_adapter_by_node(i2c_node);
+ tc9563->adapter = of_get_i2c_adapter_by_node(i2c_node);
of_node_put(i2c_node);
if (!tc9563->adapter)
return dev_err_probe(dev, -EPROBE_DEFER, "Failed to find I2C adapter\n");
@@ -556,7 +556,7 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
tc9563->client = i2c_new_dummy_device(tc9563->adapter, addr);
if (IS_ERR(tc9563->client)) {
dev_err(dev, "Failed to create I2C client\n");
- put_device(&tc9563->adapter->dev);
+ i2c_put_adapter(tc9563->adapter);
return PTR_ERR(tc9563->client);
}
@@ -624,7 +624,7 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
tc9563_pwrctrl_power_off(&tc9563->pwrctrl);
remove_i2c:
i2c_unregister_device(tc9563->client);
- put_device(&tc9563->adapter->dev);
+ i2c_put_adapter(tc9563->adapter);
return ret;
}
@@ -636,7 +636,7 @@ static void tc9563_pwrctrl_remove(struct platform_device *pdev)
tc9563_pwrctrl_power_off(&tc9563->pwrctrl);
i2c_unregister_device(tc9563->client);
- put_device(&tc9563->adapter->dev);
+ i2c_put_adapter(tc9563->adapter);
}
static const struct of_device_id tc9563_pwrctrl_of_match[] = {
--
2.54.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] PCI/pwrctrl: tc9563: take i2c adapter module reference
2026-07-16 13:25 [PATCH] PCI/pwrctrl: tc9563: take i2c adapter module reference Johan Hovold
@ 2026-07-16 13:41 ` sashiko-bot
2026-07-16 16:47 ` Manivannan Sadhasivam
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-07-16 13:41 UTC (permalink / raw)
To: Johan Hovold; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing bounds check on `port` index allows out-of-bounds array access on malformed device tree.
- [High] Missing device link to I2C adapter leads to Use-After-Free if the adapter is unbound.
- [Medium] Unconditional calls to `tc9563_pwrctrl_power_off()` cause regulator usage count underflows.
--
--- Patch [1]: [PATCH] PCI/pwrctrl: tc9563: take i2c adapter module reference ---
commit ef55bbdfd2a346b4d8f7147c1d7a0c1ab090cb03
Author: Johan Hovold <johan@kernel.org>
Subject: PCI/pwrctrl: tc9563: take i2c adapter module reference
This commit switches to using of_get_i2c_adapter_by_node() to take a module
reference to the I2C adapter. This prevents the I2C subsystem from blocking
uninterruptibly during adapter deregistration when consumers are holding a
reference, allowing module unloads to fail gracefully instead.
> diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> index 488e1ec34a7fb..83ae9f256b69e 100644
> --- a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> +++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> @@ -549,7 +549,7 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
> return dev_err_probe(dev, ret, "Failed to read i2c-parent property\n");
>
> i2c_node = of_parse_phandle(dev->of_node, "i2c-parent", 0);
> - tc9563->adapter = of_find_i2c_adapter_by_node(i2c_node);
> + tc9563->adapter = of_get_i2c_adapter_by_node(i2c_node);
> of_node_put(i2c_node);
> if (!tc9563->adapter)
> return dev_err_probe(dev, -EPROBE_DEFER, "Failed to find I2C adapter\n");
> @@ -557,7 +557,7 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
> tc9563->client = i2c_new_dummy_device(tc9563->adapter, addr);
[Severity: High]
This is a pre-existing issue, but does this code leave the platform device
vulnerable to a use-after-free?
If a privileged user unbinds the I2C adapter via sysfs, the I2C core
unregisters and frees all child clients, including this dummy client. Since
no device link is created (such as with DL_FLAG_AUTOREMOVE_CONSUMER), the
platform device remains bound and unaware, holding a dangling pointer to the
freed client.
Could subsequent operations like device removal or PCI power callbacks
dereference this freed client?
[Severity: High]
This is also a pre-existing issue, but could a malformed device tree cause an
out-of-bounds access on the configuration array?
Looking further down in tc9563_pwrctrl_probe(), the port variable is
incremented for each child node without bounds checking:
for_each_child_of_node_scoped(node, child) {
port++;
ret = tc9563_pwrctrl_parse_device_dt(tc9563, child, port);
...
}
If the device tree provides more child nodes than expected, does the port
variable increment beyond TC9563_MAX?
When passed to tc9563_pwrctrl_parse_device_dt(), this unvalidated index is
directly used to access the statically sized array:
struct tc9563_pwrctrl_cfg *cfg = &tc9563->cfg[port];
Could this lead to memory corruption if the loop exceeds the array bounds?
> if (IS_ERR(tc9563->client)) {
> dev_err(dev, "Failed to create I2C client\n");
> - put_device(&tc9563->adapter->dev);
> + i2c_put_adapter(tc9563->adapter);
> return PTR_ERR(tc9563->client);
> }
[ ... ]
> @@ -625,7 +625,7 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
> tc9563_pwrctrl_power_off(&tc9563->pwrctrl);
[Severity: Medium]
This is a pre-existing issue, but could this unconditional call to power off
cause a regulator usage count underflow?
Because the regulators are only enabled lazily when the PCI core invokes
the .power_on callback, they may not have been enabled if
devm_pci_pwrctrl_device_set_ready() fails during probe.
> remove_i2c:
> i2c_unregister_device(tc9563->client);
> - put_device(&tc9563->adapter->dev);
> + i2c_put_adapter(tc9563->adapter);
> return ret;
> }
>
> @@ -637,7 +637,7 @@ static void tc9563_pwrctrl_remove(struct platform_device *pdev)
>
> tc9563_pwrctrl_power_off(&tc9563->pwrctrl);
[Severity: Medium]
Similarly, this is a pre-existing issue, but if the device is removed before
being explicitly powered on by the PCI core, will this unconditional power
off also trigger a regulator usage count underflow during removal?
> i2c_unregister_device(tc9563->client);
> - put_device(&tc9563->adapter->dev);
> + i2c_put_adapter(tc9563->adapter);
> }
>
> static const struct of_device_id tc9563_pwrctrl_of_match[] = {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260716132532.1565340-1-johan@kernel.org?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] PCI/pwrctrl: tc9563: take i2c adapter module reference
2026-07-16 13:25 [PATCH] PCI/pwrctrl: tc9563: take i2c adapter module reference Johan Hovold
2026-07-16 13:41 ` sashiko-bot
@ 2026-07-16 16:47 ` Manivannan Sadhasivam
1 sibling, 0 replies; 3+ messages in thread
From: Manivannan Sadhasivam @ 2026-07-16 16:47 UTC (permalink / raw)
To: Johan Hovold; +Cc: Bartosz Golaszewski, Bjorn Helgaas, linux-pci, linux-kernel
On Thu, Jul 16, 2026 at 03:25:32PM +0200, Johan Hovold wrote:
> The i2c subsystem currently blocks during adapter deregistration
> whenever there are consumers holding a reference.
>
> Switch to using of_get_i2c_adapter_by_node() which also takes a
> reference to the adapter module so that an attempt to unload the module
> while in use fails gracefully instead of blocking uninterruptibly.
>
> Signed-off-by: Johan Hovold <johan@kernel.org>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
- Mani
> ---
> drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> index 1555e8a9b3ca..e6a8abd9da8e 100644
> --- a/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> +++ b/drivers/pci/pwrctrl/pci-pwrctrl-tc9563.c
> @@ -548,7 +548,7 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
> return dev_err_probe(dev, ret, "Failed to read i2c-parent property\n");
>
> i2c_node = of_parse_phandle(dev->of_node, "i2c-parent", 0);
> - tc9563->adapter = of_find_i2c_adapter_by_node(i2c_node);
> + tc9563->adapter = of_get_i2c_adapter_by_node(i2c_node);
> of_node_put(i2c_node);
> if (!tc9563->adapter)
> return dev_err_probe(dev, -EPROBE_DEFER, "Failed to find I2C adapter\n");
> @@ -556,7 +556,7 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
> tc9563->client = i2c_new_dummy_device(tc9563->adapter, addr);
> if (IS_ERR(tc9563->client)) {
> dev_err(dev, "Failed to create I2C client\n");
> - put_device(&tc9563->adapter->dev);
> + i2c_put_adapter(tc9563->adapter);
> return PTR_ERR(tc9563->client);
> }
>
> @@ -624,7 +624,7 @@ static int tc9563_pwrctrl_probe(struct platform_device *pdev)
> tc9563_pwrctrl_power_off(&tc9563->pwrctrl);
> remove_i2c:
> i2c_unregister_device(tc9563->client);
> - put_device(&tc9563->adapter->dev);
> + i2c_put_adapter(tc9563->adapter);
> return ret;
> }
>
> @@ -636,7 +636,7 @@ static void tc9563_pwrctrl_remove(struct platform_device *pdev)
>
> tc9563_pwrctrl_power_off(&tc9563->pwrctrl);
> i2c_unregister_device(tc9563->client);
> - put_device(&tc9563->adapter->dev);
> + i2c_put_adapter(tc9563->adapter);
> }
>
> static const struct of_device_id tc9563_pwrctrl_of_match[] = {
> --
> 2.54.0
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-07-16 16:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-16 13:25 [PATCH] PCI/pwrctrl: tc9563: take i2c adapter module reference Johan Hovold
2026-07-16 13:41 ` sashiko-bot
2026-07-16 16:47 ` Manivannan Sadhasivam
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox