PCI/pwrctrl: Fix device node leak in pci_pwrctrl_is_required()

[PATCH] PCI/pwrctrl: Fix device node leak in pci_pwrctrl_is_required()

Posted by Felix Gu 1 week, 4 days ago

The for_each_endpoint_of_node() macro requires calling of_node_put() on
the endpoint node when breaking out of the loop early.

Add of_node_put(endpoint) before the early return to properly release
the reference.

Fixes: cf3287fb2c1f ("PCI/pwrctrl: Ensure that remote endpoint node parent has supply requirement")
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
---
 drivers/pci/pwrctrl/core.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
index 7754baed67f2..97cff5b8ca88 100644
--- a/drivers/pci/pwrctrl/core.c
+++ b/drivers/pci/pwrctrl/core.c
@@ -299,8 +299,10 @@ static bool pci_pwrctrl_is_required(struct device_node *np)
 			struct device_node *remote __free(device_node) =
 				of_graph_get_remote_port_parent(endpoint);
 			if (remote) {
-				if (of_pci_supply_present(remote))
+				if (of_pci_supply_present(remote)) {
+					of_node_put(endpoint);
 					return true;
+				}
 			}
 		}
 	}

---
base-commit: 785f0eb2f85decbe7c1ef9ae922931f0194ffc2e
change-id: 20260323-pwctrl-fe840d733443

Best regards,
-- 
Felix Gu <ustc.gu@gmail.com>

Re: [PATCH] PCI/pwrctrl: Fix device node leak in pci_pwrctrl_is_required()

Posted by Bjorn Helgaas 1 week, 3 days ago

On Mon, Mar 23, 2026 at 07:05:22PM +0800, Felix Gu wrote:
> The for_each_endpoint_of_node() macro requires calling of_node_put() on
> the endpoint node when breaking out of the loop early.
> 
> Add of_node_put(endpoint) before the early return to properly release
> the reference.
> 
> Fixes: cf3287fb2c1f ("PCI/pwrctrl: Ensure that remote endpoint node parent has supply requirement")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>

cf3287fb2c1f appeared in v7.0-rc5, so I put this on pci/for-linus for
v7.0, thanks!

Would you also take a look at pwrseq_pcie_m2_match()?  It can
similarly return early from a for_each_endpoint_of_node() loop, so I'm
concerned it may have a similar issue.

> ---
>  drivers/pci/pwrctrl/core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
> index 7754baed67f2..97cff5b8ca88 100644
> --- a/drivers/pci/pwrctrl/core.c
> +++ b/drivers/pci/pwrctrl/core.c
> @@ -299,8 +299,10 @@ static bool pci_pwrctrl_is_required(struct device_node *np)
>  			struct device_node *remote __free(device_node) =
>  				of_graph_get_remote_port_parent(endpoint);
>  			if (remote) {
> -				if (of_pci_supply_present(remote))
> +				if (of_pci_supply_present(remote)) {
> +					of_node_put(endpoint);
>  					return true;
> +				}
>  			}
>  		}
>  	}
> 
> ---
> base-commit: 785f0eb2f85decbe7c1ef9ae922931f0194ffc2e
> change-id: 20260323-pwctrl-fe840d733443
> 
> Best regards,
> -- 
> Felix Gu <ustc.gu@gmail.com>
>

Re: [PATCH] PCI/pwrctrl: Fix device node leak in pci_pwrctrl_is_required()

Posted by Felix Gu 1 week, 3 days ago

On Tue, Mar 24, 2026 at 4:57 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Mon, Mar 23, 2026 at 07:05:22PM +0800, Felix Gu wrote:
> > The for_each_endpoint_of_node() macro requires calling of_node_put() on
> > the endpoint node when breaking out of the loop early.
> >
> > Add of_node_put(endpoint) before the early return to properly release
> > the reference.
> >
> > Fixes: cf3287fb2c1f ("PCI/pwrctrl: Ensure that remote endpoint node parent has supply requirement")
> > Signed-off-by: Felix Gu <ustc.gu@gmail.com>
>
> cf3287fb2c1f appeared in v7.0-rc5, so I put this on pci/for-linus for
> v7.0, thanks!
>
> Would you also take a look at pwrseq_pcie_m2_match()?  It can
> similarly return early from a for_each_endpoint_of_node() loop, so I'm
> concerned it may have a similar issue.
Hi Bjorn,
pwrseq_pcie_m2_match should be fine, endpoint is marked as __free(device_node),
it will be called of_node_put() automatically when returning early.

Best regards,
Felix

Re: [PATCH] PCI/pwrctrl: Fix device node leak in pci_pwrctrl_is_required()

Posted by Bjorn Helgaas 1 week, 3 days ago

On Tue, Mar 24, 2026 at 06:05:18PM +0800, Felix Gu wrote:
> On Tue, Mar 24, 2026 at 4:57 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > On Mon, Mar 23, 2026 at 07:05:22PM +0800, Felix Gu wrote:
> > > The for_each_endpoint_of_node() macro requires calling of_node_put() on
> > > the endpoint node when breaking out of the loop early.
> > >
> > > Add of_node_put(endpoint) before the early return to properly release
> > > the reference.
> > >
> > > Fixes: cf3287fb2c1f ("PCI/pwrctrl: Ensure that remote endpoint node parent has supply requirement")
> > > Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> >
> > cf3287fb2c1f appeared in v7.0-rc5, so I put this on pci/for-linus for
> > v7.0, thanks!
> >
> > Would you also take a look at pwrseq_pcie_m2_match()?  It can
> > similarly return early from a for_each_endpoint_of_node() loop, so I'm
> > concerned it may have a similar issue.
>
> pwrseq_pcie_m2_match should be fine, endpoint is marked as
> __free(device_node), it will be called of_node_put() automatically
> when returning early.

Thanks, Felix, I indeed missed the __free() on the declaration.

For anyone else following along (and me the next time I trip over
this, since the connection from __free() to of_node_put() is somewhat
obscure), here's the connection:

  pwrseq_pcie_m2_match(...)
  {
    struct device_node *endpoint __free(device_node) = NULL;
    for_each_endpoint_of_node(ctx->of_node, endpoint) {
      if (...)
        return PWRSEQ_MATCH_OK;
    }
  }

  $ git grep "DEFINE_FREE(device_node"
  include/linux/of.h:DEFINE_FREE(device_node, struct device_node *, if (_T) of_node_put(_T))

Re: [PATCH] PCI/pwrctrl: Fix device node leak in pci_pwrctrl_is_required()

Posted by Bartosz Golaszewski 1 week, 4 days ago

On Mon, 23 Mar 2026 12:05:22 +0100, Felix Gu <ustc.gu@gmail.com> said:
> The for_each_endpoint_of_node() macro requires calling of_node_put() on
> the endpoint node when breaking out of the loop early.
>
> Add of_node_put(endpoint) before the early return to properly release
> the reference.
>
> Fixes: cf3287fb2c1f ("PCI/pwrctrl: Ensure that remote endpoint node parent has supply requirement")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> ---
>  drivers/pci/pwrctrl/core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
> index 7754baed67f2..97cff5b8ca88 100644
> --- a/drivers/pci/pwrctrl/core.c
> +++ b/drivers/pci/pwrctrl/core.c
> @@ -299,8 +299,10 @@ static bool pci_pwrctrl_is_required(struct device_node *np)
>  			struct device_node *remote __free(device_node) =
>  				of_graph_get_remote_port_parent(endpoint);
>  			if (remote) {
> -				if (of_pci_supply_present(remote))
> +				if (of_pci_supply_present(remote)) {
> +					of_node_put(endpoint);
>  					return true;
> +				}
>  			}
>  		}
>  	}
>
> ---
> base-commit: 785f0eb2f85decbe7c1ef9ae922931f0194ffc2e
> change-id: 20260323-pwctrl-fe840d733443
>
> Best regards,
> --
> Felix Gu <ustc.gu@gmail.com>
>
>

Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>

Re: [PATCH] PCI/pwrctrl: Fix device node leak in pci_pwrctrl_is_required()

Posted by Manivannan Sadhasivam 1 week, 4 days ago

On Mon, Mar 23, 2026 at 07:05:22PM +0800, Felix Gu wrote:
> The for_each_endpoint_of_node() macro requires calling of_node_put() on
> the endpoint node when breaking out of the loop early.
> 
> Add of_node_put(endpoint) before the early return to properly release
> the reference.
> 
> Fixes: cf3287fb2c1f ("PCI/pwrctrl: Ensure that remote endpoint node parent has supply requirement")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>

Err...

Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>

- Mani

> ---
>  drivers/pci/pwrctrl/core.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
> index 7754baed67f2..97cff5b8ca88 100644
> --- a/drivers/pci/pwrctrl/core.c
> +++ b/drivers/pci/pwrctrl/core.c
> @@ -299,8 +299,10 @@ static bool pci_pwrctrl_is_required(struct device_node *np)
>  			struct device_node *remote __free(device_node) =
>  				of_graph_get_remote_port_parent(endpoint);
>  			if (remote) {
> -				if (of_pci_supply_present(remote))
> +				if (of_pci_supply_present(remote)) {
> +					of_node_put(endpoint);
>  					return true;
> +				}
>  			}
>  		}
>  	}
> 
> ---
> base-commit: 785f0eb2f85decbe7c1ef9ae922931f0194ffc2e
> change-id: 20260323-pwctrl-fe840d733443
> 
> Best regards,
> -- 
> Felix Gu <ustc.gu@gmail.com>
> 

-- 
மணிவண்ணன் சதாசிவம்