Re: [PATCH] PCI: meson: Propagate devm_add_action_or_reset() failure

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing `.remove` callback or `.suppress_bind_attrs = true` leaves PCI subsystem and PHY active after driver unbind.
--

commit fde9e1cd9d05730f72b074fa8edf3a1233d11384
Author: Shuvam Pandey <shuvampandey1@gmail.com>

PCI: meson: Propagate devm_add_action_or_reset() failure

This commit updates meson_pcie_probe_clock() to check the return value
of devm_add_action_or_reset(). If it fails, the error is returned so
that the probe error path can unwind normally.

> diff --git a/drivers/pci/controller/dwc/pci-meson.c b/drivers/pci/controller/dwc/pci-meson.c
> index 0694084f612b7..8d495bcc3a41a 100644
> --- a/drivers/pci/controller/dwc/pci-meson.c
> +++ b/drivers/pci/controller/dwc/pci-meson.c
> @@ -204,7 +204,9 @@ static inline struct clk *meson_pcie_probe_clock(struct device *dev,
>  		return ERR_PTR(ret);
>  	}
>  
> -	devm_add_action_or_reset(dev, meson_pcie_disable_clock, clk);
> +	ret = devm_add_action_or_reset(dev, meson_pcie_disable_clock, clk);
> +	if (ret)
> +		return ERR_PTR(ret);

This isn't a bug introduced by this patch, but could this driver be unbound
via sysfs, triggering a use-after-free?

The meson_pcie_probe() function registers the PCI host controller via
dw_pcie_host_init(&pci->pp) and powers on the PHY via meson_pcie_power_on().

However, the driver does not implement a .remove callback to call
dw_pcie_host_deinit() and meson_pcie_power_off(), nor does it set
.suppress_bind_attrs = true in its platform_driver struct.

If the driver is unbound via /sys/bus/platform/drivers/meson-pcie/unbind,
the device core will trigger devres cleanup. This frees the mp structure,
unmaps IO memory, disables clocks, and releases resets.

Because the PCI subsystem is never explicitly unregistered via
dw_pcie_host_deinit(), the PCI core retains references to the destroyed root
bus and freed structures.

Can a subsequent access by the PCI subsystem or userspace cause a
use-after-free and system crash?

Additionally, the PHY is left permanently powered on without its required
clocks. Should the driver either implement a proper teardown in a .remove
hook or prevent unbinding by setting .suppress_bind_attrs = true?

>  
>  	return clk;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/177909148011.9588.6639767953842842291@gmail.com?part=1