* [PATCH v4 1/6] phy: core: Fix that API devm_phy_put() fails to release the phy
2024-11-02 3:53 [PATCH v4 0/6] phy: core: Fix bugs for several APIs and simplify an API Zijun Hu
@ 2024-11-02 3:53 ` Zijun Hu
2024-11-05 17:13 ` Johan Hovold
2024-11-02 3:53 ` [PATCH v4 2/6] phy: core: Fix that API devm_of_phy_provider_unregister() fails to unregister the phy provider Zijun Hu
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Zijun Hu @ 2024-11-02 3:53 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Felipe Balbi,
Greg Kroah-Hartman, Rob Herring, Arnd Bergmann, Lee Jones
Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Christophe JAILLET, Johan Hovold, Zijun Hu, stable, linux-phy,
linux-kernel, Zijun Hu
From: Zijun Hu <quic_zijuhu@quicinc.com>
For devm_phy_put(), its comment says it needs to invoke phy_put() to
release the phy, but it will not actually invoke the function since
devres_destroy() does not call devm_phy_release(), and the missing
phy_put() call will cause:
- The phy fails to be released.
- devm_phy_put() can not fully undo what API devm_phy_get() does.
- Leak refcount of both the module and device for below typical usage:
devm_phy_get(); // or its variant
...
err = do_something();
if (err)
goto err_out;
...
err_out:
devm_phy_put(); // leak refcount here
The file(s) affected by this issue are shown below since they have such
typical usage.
drivers/pci/controller/cadence/pcie-cadence.c
drivers/net/ethernet/ti/am65-cpsw-nuss.c
Fixed by using devres_release() instead of devres_destroy() within the API.
Fixes: ff764963479a ("drivers: phy: add generic PHY framework")
Cc: stable@vger.kernel.org
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: "Krzysztof Wilczyński" <kw@linux.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Cc: Johan Hovold <johan@kernel.org>
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
drivers/phy/phy-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index f053b525ccff..f190d7126613 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -737,7 +737,7 @@ void devm_phy_put(struct device *dev, struct phy *phy)
if (!phy)
return;
- r = devres_destroy(dev, devm_phy_release, devm_phy_match, phy);
+ r = devres_release(dev, devm_phy_release, devm_phy_match, phy);
dev_WARN_ONCE(dev, r, "couldn't find PHY resource\n");
}
EXPORT_SYMBOL_GPL(devm_phy_put);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v4 1/6] phy: core: Fix that API devm_phy_put() fails to release the phy
2024-11-02 3:53 ` [PATCH v4 1/6] phy: core: Fix that API devm_phy_put() fails to release the phy Zijun Hu
@ 2024-11-05 17:13 ` Johan Hovold
0 siblings, 0 replies; 13+ messages in thread
From: Johan Hovold @ 2024-11-05 17:13 UTC (permalink / raw)
To: Zijun Hu
Cc: Vinod Koul, Kishon Vijay Abraham I, Felipe Balbi,
Greg Kroah-Hartman, Rob Herring, Arnd Bergmann, Lee Jones,
Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Christophe JAILLET, stable, linux-phy, linux-kernel, Zijun Hu
On Sat, Nov 02, 2024 at 11:53:43AM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
>
> For devm_phy_put(), its comment says it needs to invoke phy_put() to
> release the phy, but it will not actually invoke the function since
> devres_destroy() does not call devm_phy_release(), and the missing
> phy_put() call will cause:
I still think the above should have been split in two sentences, but
this also works.
> - The phy fails to be released.
> - devm_phy_put() can not fully undo what API devm_phy_get() does.
> - Leak refcount of both the module and device for below typical usage:
>
> devm_phy_get(); // or its variant
> ...
> err = do_something();
> if (err)
> goto err_out;
> ...
> err_out:
> devm_phy_put(); // leak refcount here
>
> The file(s) affected by this issue are shown below since they have such
> typical usage.
> drivers/pci/controller/cadence/pcie-cadence.c
> drivers/net/ethernet/ti/am65-cpsw-nuss.c
>
> Fixed by using devres_release() instead of devres_destroy() within the API.
Nit: in the future, try to use imperative mood in your commit messages
(e.g. "fix" instead of "fixed") as the process documentation suggests.
> Fixes: ff764963479a ("drivers: phy: add generic PHY framework")
> Cc: stable@vger.kernel.org
> Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
> Cc: "Krzysztof Wilczyński" <kw@linux.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: Johan Hovold <johan@kernel.org>
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 2/6] phy: core: Fix that API devm_of_phy_provider_unregister() fails to unregister the phy provider
2024-11-02 3:53 [PATCH v4 0/6] phy: core: Fix bugs for several APIs and simplify an API Zijun Hu
2024-11-02 3:53 ` [PATCH v4 1/6] phy: core: Fix that API devm_phy_put() fails to release the phy Zijun Hu
@ 2024-11-02 3:53 ` Zijun Hu
2024-11-05 17:15 ` Johan Hovold
2024-11-02 3:53 ` [PATCH v4 3/6] phy: core: Fix that API devm_phy_destroy() fails to destroy the phy Zijun Hu
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Zijun Hu @ 2024-11-02 3:53 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Felipe Balbi,
Greg Kroah-Hartman, Rob Herring, Arnd Bergmann, Lee Jones
Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Christophe JAILLET, Johan Hovold, Zijun Hu, stable, linux-phy,
linux-kernel, Zijun Hu
From: Zijun Hu <quic_zijuhu@quicinc.com>
For devm_of_phy_provider_unregister(), its comment says it needs to invoke
of_phy_provider_unregister() to unregister the phy provider, but it will
not actually invoke the function since devres_destroy() does not call
devm_phy_provider_release(), and the missing of_phy_provider_unregister()
call will case:
- The phy provider fails to be unregistered.
- Leak both memory and the OF node refcount.
Fortunately, the faulty API has not been used by current kernel tree.
Fixed by using devres_release() instead of devres_destroy() within the API.
Fixes: ff764963479a ("drivers: phy: add generic PHY framework")
Cc: stable@vger.kernel.org
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
Why to fix the API here instead of directly deleting it?
1) it is simpler, just one line change.
2) it may be used in future.
3) ensure this restored API right if need to restore it in future
after deleting.
Anyone may remove such APIs separately later if he/she cares.
---
drivers/phy/phy-core.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index f190d7126613..de07e1616b34 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -1259,12 +1259,12 @@ EXPORT_SYMBOL_GPL(of_phy_provider_unregister);
* of_phy_provider_unregister to unregister the phy provider.
*/
void devm_of_phy_provider_unregister(struct device *dev,
- struct phy_provider *phy_provider)
+ struct phy_provider *phy_provider)
{
int r;
- r = devres_destroy(dev, devm_phy_provider_release, devm_phy_match,
- phy_provider);
+ r = devres_release(dev, devm_phy_provider_release, devm_phy_match,
+ phy_provider);
dev_WARN_ONCE(dev, r, "couldn't find PHY provider device resource\n");
}
EXPORT_SYMBOL_GPL(devm_of_phy_provider_unregister);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v4 2/6] phy: core: Fix that API devm_of_phy_provider_unregister() fails to unregister the phy provider
2024-11-02 3:53 ` [PATCH v4 2/6] phy: core: Fix that API devm_of_phy_provider_unregister() fails to unregister the phy provider Zijun Hu
@ 2024-11-05 17:15 ` Johan Hovold
0 siblings, 0 replies; 13+ messages in thread
From: Johan Hovold @ 2024-11-05 17:15 UTC (permalink / raw)
To: Zijun Hu
Cc: Vinod Koul, Kishon Vijay Abraham I, Felipe Balbi,
Greg Kroah-Hartman, Rob Herring, Arnd Bergmann, Lee Jones,
Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Christophe JAILLET, stable, linux-phy, linux-kernel, Zijun Hu
On Sat, Nov 02, 2024 at 11:53:44AM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
>
> For devm_of_phy_provider_unregister(), its comment says it needs to invoke
> of_phy_provider_unregister() to unregister the phy provider, but it will
> not actually invoke the function since devres_destroy() does not call
> devm_phy_provider_release(), and the missing of_phy_provider_unregister()
> call will case:
>
> - The phy provider fails to be unregistered.
> - Leak both memory and the OF node refcount.
>
> Fortunately, the faulty API has not been used by current kernel tree.
> Fixed by using devres_release() instead of devres_destroy() within the API.
>
> Fixes: ff764963479a ("drivers: phy: add generic PHY framework")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 3/6] phy: core: Fix that API devm_phy_destroy() fails to destroy the phy
2024-11-02 3:53 [PATCH v4 0/6] phy: core: Fix bugs for several APIs and simplify an API Zijun Hu
2024-11-02 3:53 ` [PATCH v4 1/6] phy: core: Fix that API devm_phy_put() fails to release the phy Zijun Hu
2024-11-02 3:53 ` [PATCH v4 2/6] phy: core: Fix that API devm_of_phy_provider_unregister() fails to unregister the phy provider Zijun Hu
@ 2024-11-02 3:53 ` Zijun Hu
2024-11-05 17:17 ` Johan Hovold
2024-11-02 3:53 ` [PATCH v4 4/6] phy: core: Fix an OF node refcount leakage in _of_phy_get() Zijun Hu
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Zijun Hu @ 2024-11-02 3:53 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Felipe Balbi,
Greg Kroah-Hartman, Rob Herring, Arnd Bergmann, Lee Jones
Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Christophe JAILLET, Johan Hovold, Zijun Hu, stable, linux-phy,
linux-kernel, Zijun Hu
From: Zijun Hu <quic_zijuhu@quicinc.com>
For devm_phy_destroy(), its comment says it needs to invoke phy_destroy()
to destroy the phy, but it will not actually invoke the function since
devres_destroy() does not call devm_phy_consume(), and the missing
phy_destroy() call will case that the phy fails to be destroyed.
Fortunately, the faulty API has not been used by current kernel tree.
Fixed by using devres_release() instead of devres_destroy() within the API.
Fixes: ff764963479a ("drivers: phy: add generic PHY framework")
Cc: stable@vger.kernel.org
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
Why to fix the API here instead of directly deleting it?
1) it is simpler, just one line change.
2) it may be used in future.
3) ensure this restored API right if need to restore it in future
after deleting.
Anyone may remove such APIs separately later if he/she cares.
---
drivers/phy/phy-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index de07e1616b34..52ca590a58b9 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -1121,7 +1121,7 @@ void devm_phy_destroy(struct device *dev, struct phy *phy)
{
int r;
- r = devres_destroy(dev, devm_phy_consume, devm_phy_match, phy);
+ r = devres_release(dev, devm_phy_consume, devm_phy_match, phy);
dev_WARN_ONCE(dev, r, "couldn't find PHY resource\n");
}
EXPORT_SYMBOL_GPL(devm_phy_destroy);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v4 3/6] phy: core: Fix that API devm_phy_destroy() fails to destroy the phy
2024-11-02 3:53 ` [PATCH v4 3/6] phy: core: Fix that API devm_phy_destroy() fails to destroy the phy Zijun Hu
@ 2024-11-05 17:17 ` Johan Hovold
0 siblings, 0 replies; 13+ messages in thread
From: Johan Hovold @ 2024-11-05 17:17 UTC (permalink / raw)
To: Zijun Hu
Cc: Vinod Koul, Kishon Vijay Abraham I, Felipe Balbi,
Greg Kroah-Hartman, Rob Herring, Arnd Bergmann, Lee Jones,
Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Christophe JAILLET, stable, linux-phy, linux-kernel, Zijun Hu
On Sat, Nov 02, 2024 at 11:53:45AM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
>
> For devm_phy_destroy(), its comment says it needs to invoke phy_destroy()
> to destroy the phy, but it will not actually invoke the function since
> devres_destroy() does not call devm_phy_consume(), and the missing
> phy_destroy() call will case that the phy fails to be destroyed.
Should also have been split in two sentences, but this also works.
typo: s/case/cause/
> Fortunately, the faulty API has not been used by current kernel tree.
> Fixed by using devres_release() instead of devres_destroy() within the API.
>
> Fixes: ff764963479a ("drivers: phy: add generic PHY framework")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 4/6] phy: core: Fix an OF node refcount leakage in _of_phy_get()
2024-11-02 3:53 [PATCH v4 0/6] phy: core: Fix bugs for several APIs and simplify an API Zijun Hu
` (2 preceding siblings ...)
2024-11-02 3:53 ` [PATCH v4 3/6] phy: core: Fix that API devm_phy_destroy() fails to destroy the phy Zijun Hu
@ 2024-11-02 3:53 ` Zijun Hu
2024-11-05 17:20 ` Johan Hovold
2024-11-02 3:53 ` [PATCH v4 5/6] phy: core: Fix an OF node refcount leakage in of_phy_provider_lookup() Zijun Hu
2024-11-02 3:53 ` [PATCH v4 6/6] phy: core: Simplify API of_phy_simple_xlate() implementation Zijun Hu
5 siblings, 1 reply; 13+ messages in thread
From: Zijun Hu @ 2024-11-02 3:53 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Felipe Balbi,
Greg Kroah-Hartman, Rob Herring, Arnd Bergmann, Lee Jones
Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Christophe JAILLET, Johan Hovold, Zijun Hu, stable, linux-phy,
linux-kernel, Zijun Hu
From: Zijun Hu <quic_zijuhu@quicinc.com>
_of_phy_get() will directly return when suffers of_device_is_compatible()
error, but it forgets to decrease refcount of OF node @args.np before error
return, the refcount was increased by previous of_parse_phandle_with_args()
so causes the OF node's refcount leakage.
Fix by decreasing the refcount via of_node_put() before the error return.
Fixes: b7563e2796f8 ("phy: work around 'phys' references to usb-nop-xceiv devices")
Cc: stable@vger.kernel.org
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
drivers/phy/phy-core.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 52ca590a58b9..3127c5d9c637 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -624,13 +624,15 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
struct of_phandle_args args;
ret = of_parse_phandle_with_args(np, "phys", "#phy-cells",
- index, &args);
+ index, &args);
if (ret)
return ERR_PTR(-ENODEV);
/* This phy type handled by the usb-phy subsystem for now */
- if (of_device_is_compatible(args.np, "usb-nop-xceiv"))
- return ERR_PTR(-ENODEV);
+ if (of_device_is_compatible(args.np, "usb-nop-xceiv")) {
+ phy = ERR_PTR(-ENODEV);
+ goto out_put_node;
+ }
mutex_lock(&phy_provider_mutex);
phy_provider = of_phy_provider_lookup(args.np);
@@ -652,6 +654,7 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
out_unlock:
mutex_unlock(&phy_provider_mutex);
+out_put_node:
of_node_put(args.np);
return phy;
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v4 4/6] phy: core: Fix an OF node refcount leakage in _of_phy_get()
2024-11-02 3:53 ` [PATCH v4 4/6] phy: core: Fix an OF node refcount leakage in _of_phy_get() Zijun Hu
@ 2024-11-05 17:20 ` Johan Hovold
2024-11-06 15:28 ` Zijun Hu
0 siblings, 1 reply; 13+ messages in thread
From: Johan Hovold @ 2024-11-05 17:20 UTC (permalink / raw)
To: Zijun Hu
Cc: Vinod Koul, Kishon Vijay Abraham I, Felipe Balbi,
Greg Kroah-Hartman, Rob Herring, Arnd Bergmann, Lee Jones,
Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Christophe JAILLET, stable, linux-phy, linux-kernel, Zijun Hu
On Sat, Nov 02, 2024 at 11:53:46AM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
>
> _of_phy_get() will directly return when suffers of_device_is_compatible()
> error, but it forgets to decrease refcount of OF node @args.np before error
> return, the refcount was increased by previous of_parse_phandle_with_args()
> so causes the OF node's refcount leakage.
>
> Fix by decreasing the refcount via of_node_put() before the error return.
>
> Fixes: b7563e2796f8 ("phy: work around 'phys' references to usb-nop-xceiv devices")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> ---
> drivers/phy/phy-core.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
> index 52ca590a58b9..3127c5d9c637 100644
> --- a/drivers/phy/phy-core.c
> +++ b/drivers/phy/phy-core.c
> @@ -624,13 +624,15 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
> struct of_phandle_args args;
>
> ret = of_parse_phandle_with_args(np, "phys", "#phy-cells",
> - index, &args);
> + index, &args);
This is an unrelated change which do not belong in this patch (and even
more so as it is a fix that is marked for backporting).
> if (ret)
> return ERR_PTR(-ENODEV);
>
> /* This phy type handled by the usb-phy subsystem for now */
> - if (of_device_is_compatible(args.np, "usb-nop-xceiv"))
> - return ERR_PTR(-ENODEV);
> + if (of_device_is_compatible(args.np, "usb-nop-xceiv")) {
> + phy = ERR_PTR(-ENODEV);
> + goto out_put_node;
> + }
>
> mutex_lock(&phy_provider_mutex);
> phy_provider = of_phy_provider_lookup(args.np);
> @@ -652,6 +654,7 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
>
> out_unlock:
> mutex_unlock(&phy_provider_mutex);
> +out_put_node:
> of_node_put(args.np);
>
> return phy;A
With the above fixed:
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v4 4/6] phy: core: Fix an OF node refcount leakage in _of_phy_get()
2024-11-05 17:20 ` Johan Hovold
@ 2024-11-06 15:28 ` Zijun Hu
0 siblings, 0 replies; 13+ messages in thread
From: Zijun Hu @ 2024-11-06 15:28 UTC (permalink / raw)
To: Johan Hovold
Cc: Vinod Koul, Kishon Vijay Abraham I, Felipe Balbi,
Greg Kroah-Hartman, Rob Herring, Arnd Bergmann, Lee Jones,
Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Christophe JAILLET, stable, linux-phy, linux-kernel, Zijun Hu
On 2024/11/6 01:20, Johan Hovold wrote:
> On Sat, Nov 02, 2024 at 11:53:46AM +0800, Zijun Hu wrote:
>> From: Zijun Hu <quic_zijuhu@quicinc.com>
>>
>> _of_phy_get() will directly return when suffers of_device_is_compatible()
>> error, but it forgets to decrease refcount of OF node @args.np before error
>> return, the refcount was increased by previous of_parse_phandle_with_args()
>> so causes the OF node's refcount leakage.
>>
>> Fix by decreasing the refcount via of_node_put() before the error return.
>>
>> Fixes: b7563e2796f8 ("phy: work around 'phys' references to usb-nop-xceiv devices")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
>> ---
>> drivers/phy/phy-core.c | 9 ++++++---
>> 1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
>> index 52ca590a58b9..3127c5d9c637 100644
>> --- a/drivers/phy/phy-core.c
>> +++ b/drivers/phy/phy-core.c
>> @@ -624,13 +624,15 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
>> struct of_phandle_args args;
>>
>> ret = of_parse_phandle_with_args(np, "phys", "#phy-cells",
>> - index, &args);
>> + index, &args);
>
> This is an unrelated change which do not belong in this patch (and even
> more so as it is a fix that is marked for backporting).
>
make sense.
will remove it for next revision. (^^)
>> if (ret)
>> return ERR_PTR(-ENODEV);
>>
>> /* This phy type handled by the usb-phy subsystem for now */
>> - if (of_device_is_compatible(args.np, "usb-nop-xceiv"))
>> - return ERR_PTR(-ENODEV);
>> + if (of_device_is_compatible(args.np, "usb-nop-xceiv")) {
>> + phy = ERR_PTR(-ENODEV);
>> + goto out_put_node;
>> + }
>>
>> mutex_lock(&phy_provider_mutex);
>> phy_provider = of_phy_provider_lookup(args.np);
>> @@ -652,6 +654,7 @@ static struct phy *_of_phy_get(struct device_node *np, int index)
>>
>> out_unlock:
>> mutex_unlock(&phy_provider_mutex);
>> +out_put_node:
>> of_node_put(args.np);
>>
>> return phy;A
>
> With the above fixed:
>
> Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 5/6] phy: core: Fix an OF node refcount leakage in of_phy_provider_lookup()
2024-11-02 3:53 [PATCH v4 0/6] phy: core: Fix bugs for several APIs and simplify an API Zijun Hu
` (3 preceding siblings ...)
2024-11-02 3:53 ` [PATCH v4 4/6] phy: core: Fix an OF node refcount leakage in _of_phy_get() Zijun Hu
@ 2024-11-02 3:53 ` Zijun Hu
2024-11-02 3:53 ` [PATCH v4 6/6] phy: core: Simplify API of_phy_simple_xlate() implementation Zijun Hu
5 siblings, 0 replies; 13+ messages in thread
From: Zijun Hu @ 2024-11-02 3:53 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Felipe Balbi,
Greg Kroah-Hartman, Rob Herring, Arnd Bergmann, Lee Jones
Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Christophe JAILLET, Johan Hovold, Zijun Hu, stable, linux-phy,
linux-kernel, Zijun Hu, Johan Hovold
From: Zijun Hu <quic_zijuhu@quicinc.com>
For macro for_each_child_of_node(parent, child), refcount of @child has
been increased before entering its loop body, so normally needs to call
of_node_put(@child) before returning from the loop body to avoid refcount
leakage.
of_phy_provider_lookup() has such usage but does not call of_node_put()
before returning, so cause leakage of the OF node refcount.
Fixed by simply calling of_node_put() before returning from the loop body.
The APIs affected by this issue are shown below since they indirectly
invoke problematic of_phy_provider_lookup().
phy_get()
of_phy_get()
devm_phy_get()
devm_of_phy_get()
devm_of_phy_get_by_index()
Fixes: 2a4c37016ca9 ("phy: core: Fix of_phy_provider_lookup to return PHY provider for sub node")
Cc: stable@vger.kernel.org
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
The following kernel mainline commit fixes a similar issue:
Commit: b337cc3ce475 ("backlight: lm3509_bl: Fix early returns in for_each_child_of_node()")
---
drivers/phy/phy-core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 3127c5d9c637..9d4cc64a0865 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -145,8 +145,10 @@ static struct phy_provider *of_phy_provider_lookup(struct device_node *node)
return phy_provider;
for_each_child_of_node(phy_provider->children, child)
- if (child == node)
+ if (child == node) {
+ of_node_put(child);
return phy_provider;
+ }
}
return ERR_PTR(-EPROBE_DEFER);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH v4 6/6] phy: core: Simplify API of_phy_simple_xlate() implementation
2024-11-02 3:53 [PATCH v4 0/6] phy: core: Fix bugs for several APIs and simplify an API Zijun Hu
` (4 preceding siblings ...)
2024-11-02 3:53 ` [PATCH v4 5/6] phy: core: Fix an OF node refcount leakage in of_phy_provider_lookup() Zijun Hu
@ 2024-11-02 3:53 ` Zijun Hu
2024-11-02 3:57 ` kernel test robot
5 siblings, 1 reply; 13+ messages in thread
From: Zijun Hu @ 2024-11-02 3:53 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Felipe Balbi,
Greg Kroah-Hartman, Rob Herring, Arnd Bergmann, Lee Jones
Cc: Lorenzo Pieralisi, Krzysztof Wilczyński, Bjorn Helgaas,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Christophe JAILLET, Johan Hovold, Zijun Hu, stable, linux-phy,
linux-kernel, Zijun Hu
From: Zijun Hu <quic_zijuhu@quicinc.com>
Simplify of_phy_simple_xlate() implementation by API
class_find_device_by_of_node().
Also correct comments to mark its parameter @dev as unused instead of
@args in passing.
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
drivers/phy/phy-core.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 9d4cc64a0865..39476ca9e51c 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -749,8 +749,8 @@ EXPORT_SYMBOL_GPL(devm_phy_put);
/**
* of_phy_simple_xlate() - returns the phy instance from phy provider
- * @dev: the PHY provider device
- * @args: of_phandle_args (not used here)
+ * @dev: the PHY provider device (not used here)
+ * @args: of_phandle_args
*
* Intended to be used by phy provider for the common case where #phy-cells is
* 0. For other cases where #phy-cells is greater than '0', the phy provider
@@ -760,20 +760,14 @@ EXPORT_SYMBOL_GPL(devm_phy_put);
struct phy *of_phy_simple_xlate(struct device *dev,
const struct of_phandle_args *args)
{
- struct phy *phy;
- struct class_dev_iter iter;
-
- class_dev_iter_init(&iter, &phy_class, NULL, NULL);
- while ((dev = class_dev_iter_next(&iter))) {
- phy = to_phy(dev);
- if (args->np != phy->dev.of_node)
- continue;
+ struct device *target_dev;
- class_dev_iter_exit(&iter);
- return phy;
+ target_dev = class_find_device_by_of_node(&phy_class, args->np);
+ if (target_dev) {
+ put_device(target_dev);
+ return to_phy(target_dev);
}
- class_dev_iter_exit(&iter);
return ERR_PTR(-ENODEV);
}
EXPORT_SYMBOL_GPL(of_phy_simple_xlate);
--
2.34.1
^ permalink raw reply related [flat|nested] 13+ messages in thread