* [PATCH] arch_topology: Fix incorrect error check in topology_parse_cpu_capacity()
@ 2025-09-23 7:25 Kaushlendra Kumar
2025-09-23 7:46 ` Markus Elfring
2025-09-23 8:40 ` Sudeep Holla
0 siblings, 2 replies; 6+ messages in thread
From: Kaushlendra Kumar @ 2025-09-23 7:25 UTC (permalink / raw)
To: sudeep.holla, gregkh, dakr, rafael; +Cc: linux-kernel, Kaushlendra Kumar
Fix incorrect use of PTR_ERR_OR_ZERO() in topology_parse_cpu_capacity()
which causes the code to proceed with NULL clock pointers. The current
logic uses !PTR_ERR_OR_ZERO(cpu_clk) which evaluates to true for both
valid pointers and NULL, leading to potential NULL pointer dereference
in clk_get_rate().
PTR_ERR_OR_ZERO(cpu_clk) returns:
- 0 if cpu_clk is a valid pointer or NULL
- error code if cpu_clk is an error pointer
Therefore !PTR_ERR_OR_ZERO(cpu_clk) is true for both valid pointers and
NULL, causing the code to call clk_get_rate(NULL) when of_clk_get()
returns NULL. Replace with IS_ERR_OR_NULL() which correctly identifies
only valid pointers, ensuring clk_get_rate() is called only with valid
clock objects.
Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
---
drivers/base/arch_topology.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index 1037169abb45..e1eff05bea4a 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -292,7 +292,7 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu)
* frequency (by keeping the initial capacity_freq_ref value).
*/
cpu_clk = of_clk_get(cpu_node, 0);
- if (!PTR_ERR_OR_ZERO(cpu_clk)) {
+ if (!IS_ERR_OR_NULL(cpu_clk)) {
per_cpu(capacity_freq_ref, cpu) =
clk_get_rate(cpu_clk) / HZ_PER_KHZ;
clk_put(cpu_clk);
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] arch_topology: Fix incorrect error check in topology_parse_cpu_capacity()
2025-09-23 7:25 [PATCH] arch_topology: Fix incorrect error check in topology_parse_cpu_capacity() Kaushlendra Kumar
@ 2025-09-23 7:46 ` Markus Elfring
2025-09-23 8:50 ` Kumar, Kaushlendra
2025-09-23 8:40 ` Sudeep Holla
1 sibling, 1 reply; 6+ messages in thread
From: Markus Elfring @ 2025-09-23 7:46 UTC (permalink / raw)
To: Kaushlendra Kumar, kernel-janitors
Cc: LKML, Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Sudeep Holla
…
> returns NULL. Replace with IS_ERR_OR_NULL() which correctly identifies
> only valid pointers, ensuring clk_get_rate() is called only with valid
> clock objects.
* Can the change description be refined based on documented macro call properties?
https://elixir.bootlin.com/linux/v6.17-rc7/source/include/linux/err.h#L101-L123
* How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?
Regards,
Markus
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] arch_topology: Fix incorrect error check in topology_parse_cpu_capacity()
2025-09-23 7:46 ` Markus Elfring
@ 2025-09-23 8:50 ` Kumar, Kaushlendra
0 siblings, 0 replies; 6+ messages in thread
From: Kumar, Kaushlendra @ 2025-09-23 8:50 UTC (permalink / raw)
To: Markus Elfring, kernel-janitors@vger.kernel.org
Cc: LKML, Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Sudeep Holla
On [Date], Markus Elfring <Markus.Elfring@web.de> wrote:
> > returns NULL. Replace with IS_ERR_OR_NULL() which correctly identifies
> > only valid pointers, ensuring clk_get_rate() is called only with valid
> > clock objects.
>
> * Can the change description be refined based on documented macro call properties?
> https://elixir.bootlin.com/linux/v6.17-rc7/source/include/linux/err.h#L101-L123
>
> * How do you think about to add any tags (like "Fixes" and "Cc") accordingly?
Hi Markus,
Thank you for the valuable feedback and the documentation reference.
Created a v2 patch with the corrected description based on the documented
behavior.
Best regards,
Kaushlendra
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arch_topology: Fix incorrect error check in topology_parse_cpu_capacity()
2025-09-23 7:25 [PATCH] arch_topology: Fix incorrect error check in topology_parse_cpu_capacity() Kaushlendra Kumar
2025-09-23 7:46 ` Markus Elfring
@ 2025-09-23 8:40 ` Sudeep Holla
2025-09-23 10:24 ` Dan Carpenter
1 sibling, 1 reply; 6+ messages in thread
From: Sudeep Holla @ 2025-09-23 8:40 UTC (permalink / raw)
To: Kaushlendra Kumar; +Cc: gregkh, dakr, rafael, Sudeep Holla, linux-kernel
On Tue, Sep 23, 2025 at 12:55:20PM +0530, Kaushlendra Kumar wrote:
> Fix incorrect use of PTR_ERR_OR_ZERO() in topology_parse_cpu_capacity()
> which causes the code to proceed with NULL clock pointers. The current
> logic uses !PTR_ERR_OR_ZERO(cpu_clk) which evaluates to true for both
> valid pointers and NULL, leading to potential NULL pointer dereference
> in clk_get_rate().
>
> PTR_ERR_OR_ZERO(cpu_clk) returns:
> - 0 if cpu_clk is a valid pointer or NULL
> - error code if cpu_clk is an error pointer
>
> Therefore !PTR_ERR_OR_ZERO(cpu_clk) is true for both valid pointers and
> NULL, causing the code to call clk_get_rate(NULL) when of_clk_get()
> returns NULL. Replace with IS_ERR_OR_NULL() which correctly identifies
> only valid pointers, ensuring clk_get_rate() is called only with valid
> clock objects.
>
Nice catch, wonder how it survived so long unnoticed. I also checked
if PTR_ERR_OR_ZERO() is used in similar way anywhere else. This happens
to be the only occurrence.
I think it needs,
Fixes: b8fe128dad8f ("arch_topology: Adjust initial CPU capacities with current freq")
With that,
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> ---
> drivers/base/arch_topology.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
> index 1037169abb45..e1eff05bea4a 100644
> --- a/drivers/base/arch_topology.c
> +++ b/drivers/base/arch_topology.c
> @@ -292,7 +292,7 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu)
> * frequency (by keeping the initial capacity_freq_ref value).
> */
> cpu_clk = of_clk_get(cpu_node, 0);
> - if (!PTR_ERR_OR_ZERO(cpu_clk)) {
> + if (!IS_ERR_OR_NULL(cpu_clk)) {
> per_cpu(capacity_freq_ref, cpu) =
> clk_get_rate(cpu_clk) / HZ_PER_KHZ;
> clk_put(cpu_clk);
> --
> 2.34.1
>
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] arch_topology: Fix incorrect error check in topology_parse_cpu_capacity()
2025-09-23 8:40 ` Sudeep Holla
@ 2025-09-23 10:24 ` Dan Carpenter
2025-09-23 10:27 ` Sudeep Holla
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2025-09-23 10:24 UTC (permalink / raw)
To: Sudeep Holla; +Cc: Kaushlendra Kumar, gregkh, dakr, rafael, linux-kernel
On Tue, Sep 23, 2025 at 09:40:28AM +0100, Sudeep Holla wrote:
> On Tue, Sep 23, 2025 at 12:55:20PM +0530, Kaushlendra Kumar wrote:
> > Fix incorrect use of PTR_ERR_OR_ZERO() in topology_parse_cpu_capacity()
> > which causes the code to proceed with NULL clock pointers. The current
> > logic uses !PTR_ERR_OR_ZERO(cpu_clk) which evaluates to true for both
> > valid pointers and NULL, leading to potential NULL pointer dereference
> > in clk_get_rate().
> >
> > PTR_ERR_OR_ZERO(cpu_clk) returns:
> > - 0 if cpu_clk is a valid pointer or NULL
> > - error code if cpu_clk is an error pointer
> >
> > Therefore !PTR_ERR_OR_ZERO(cpu_clk) is true for both valid pointers and
> > NULL, causing the code to call clk_get_rate(NULL) when of_clk_get()
> > returns NULL. Replace with IS_ERR_OR_NULL() which correctly identifies
> > only valid pointers, ensuring clk_get_rate() is called only with valid
> > clock objects.
> >
>
> Nice catch, wonder how it survived so long unnoticed.
I don't think of_clk_get() can actually return NULL... It's still worth
fixing but I don't think it affects real life.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] arch_topology: Fix incorrect error check in topology_parse_cpu_capacity()
2025-09-23 10:24 ` Dan Carpenter
@ 2025-09-23 10:27 ` Sudeep Holla
0 siblings, 0 replies; 6+ messages in thread
From: Sudeep Holla @ 2025-09-23 10:27 UTC (permalink / raw)
To: Dan Carpenter
Cc: Kaushlendra Kumar, Sudeep Holla, gregkh, dakr, rafael,
linux-kernel
On Tue, Sep 23, 2025 at 01:24:41PM +0300, Dan Carpenter wrote:
> On Tue, Sep 23, 2025 at 09:40:28AM +0100, Sudeep Holla wrote:
> > On Tue, Sep 23, 2025 at 12:55:20PM +0530, Kaushlendra Kumar wrote:
> > > Fix incorrect use of PTR_ERR_OR_ZERO() in topology_parse_cpu_capacity()
> > > which causes the code to proceed with NULL clock pointers. The current
> > > logic uses !PTR_ERR_OR_ZERO(cpu_clk) which evaluates to true for both
> > > valid pointers and NULL, leading to potential NULL pointer dereference
> > > in clk_get_rate().
> > >
> > > PTR_ERR_OR_ZERO(cpu_clk) returns:
> > > - 0 if cpu_clk is a valid pointer or NULL
> > > - error code if cpu_clk is an error pointer
> > >
> > > Therefore !PTR_ERR_OR_ZERO(cpu_clk) is true for both valid pointers and
> > > NULL, causing the code to call clk_get_rate(NULL) when of_clk_get()
> > > returns NULL. Replace with IS_ERR_OR_NULL() which correctly identifies
> > > only valid pointers, ensuring clk_get_rate() is called only with valid
> > > clock objects.
> > >
> >
> > Nice catch, wonder how it survived so long unnoticed.
>
> I don't think of_clk_get() can actually return NULL... It's still worth
> fixing but I don't think it affects real life.
>
Ah that explains, just looked at PTR_ERR_OR_ZERO and wondered. Thanks
for the details.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-09-23 10:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-23 7:25 [PATCH] arch_topology: Fix incorrect error check in topology_parse_cpu_capacity() Kaushlendra Kumar
2025-09-23 7:46 ` Markus Elfring
2025-09-23 8:50 ` Kumar, Kaushlendra
2025-09-23 8:40 ` Sudeep Holla
2025-09-23 10:24 ` Dan Carpenter
2025-09-23 10:27 ` Sudeep Holla
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox