All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Shuvam Pandey" <shuvampandey1@gmail.com>
Cc: neil.armstrong@linaro.org, linux-amlogic@lists.infradead.org,
	linux-pci@vger.kernel.org
Subject: Re: [PATCH] PCI: meson: Propagate devm_add_action_or_reset() failure
Date: Mon, 18 May 2026 08:20:22 +0000	[thread overview]
Message-ID: <20260518082022.F4229C2BCB7@smtp.kernel.org> (raw)
In-Reply-To: <177909148011.9588.6639767953842842291@gmail.com>

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

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

WARNING: multiple messages have this Message-ID (diff)
From: sashiko-bot@kernel.org
To: "Shuvam Pandey" <shuvampandey1@gmail.com>
Cc: neil.armstrong@linaro.org, linux-amlogic@lists.infradead.org,
	linux-pci@vger.kernel.org
Subject: Re: [PATCH] PCI: meson: Propagate devm_add_action_or_reset() failure
Date: Mon, 18 May 2026 08:20:22 +0000	[thread overview]
Message-ID: <20260518082022.F4229C2BCB7@smtp.kernel.org> (raw)
In-Reply-To: <177909148011.9588.6639767953842842291@gmail.com>

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

  reply	other threads:[~2026-05-18  8:20 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-18  8:04 [PATCH] PCI: meson: Propagate devm_add_action_or_reset() failure Shuvam Pandey
2026-05-18  8:20 ` sashiko-bot [this message]
2026-05-18  8:20   ` sashiko-bot
2026-05-18 12:56   ` neil.armstrong
2026-05-18 12:56     ` neil.armstrong
2026-05-18 12:56 ` Neil Armstrong
2026-05-18 12:56   ` Neil Armstrong
2026-06-09 16:25 ` Manivannan Sadhasivam
2026-06-09 16:25   ` Manivannan Sadhasivam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260518082022.F4229C2BCB7@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=neil.armstrong@linaro.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=shuvampandey1@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.