[PATCH v3] arch_topology: Fix incorrect error check in topology_parse_cpu_capacity()

[PATCH v3] arch_topology: Fix incorrect error check in topology_parse_cpu_capacity()

[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().

Per include/linux/err.h documentation, PTR_ERR_OR_ZERO(ptr) returns:
"The error code within @ptr if it is an error pointer; 0 otherwise."

This means PTR_ERR_OR_ZERO() returns 0 for both valid pointers AND NULL
pointers. Therefore !PTR_ERR_OR_ZERO(cpu_clk) evaluates to true (proceed)
when cpu_clk is either valid or NULL, causing clk_get_rate(NULL) to be
called when of_clk_get() returns NULL.

Replace with !IS_ERR_OR_NULL(cpu_clk) which only proceeds for valid
pointers, preventing potential NULL pointer dereference in clk_get_rate().

Fixes: b8fe128dad8f ("arch_topology: Adjust initial CPU capacities with current freq")
Cc: stable@vger.kernel.org

Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
---
Changes in v3:
- Used accurate "function call properties" terminology in commit description
  (suggested by Markus Elfring)
- Added stable backport justification
- Removed duplicate marker line per kernel documentation

Changes in v2:
- Refined description based on documented macro properties (suggested by Markus Elfring)
- Added proper Fixes

 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

Re: [PATCH v3] arch_topology: Fix incorrect error check in topology_parse_cpu_capacity()

[Sudeep Holla]

On Tue, Sep 23, 2025 at 03:15:14PM +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().
> 
> Per include/linux/err.h documentation, PTR_ERR_OR_ZERO(ptr) returns:
> "The error code within @ptr if it is an error pointer; 0 otherwise."
> 
> This means PTR_ERR_OR_ZERO() returns 0 for both valid pointers AND NULL
> pointers. Therefore !PTR_ERR_OR_ZERO(cpu_clk) evaluates to true (proceed)
> when cpu_clk is either valid or NULL, causing clk_get_rate(NULL) to be
> called when of_clk_get() returns NULL.
> 
> Replace with !IS_ERR_OR_NULL(cpu_clk) which only proceeds for valid
> pointers, preventing potential NULL pointer dereference in clk_get_rate().
> 
> Fixes: b8fe128dad8f ("arch_topology: Adjust initial CPU capacities with current freq")
> Cc: stable@vger.kernel.org
> 

I wonder if you missed my response on v1[1] before you sent v2/v3 so quickly.
The reviewed by tag still stands, just for sake of tools:

Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>

-- 
Regards,
Sudeep

[1] https://lore.kernel.org/all/20250923-spectral-rich-shellfish-3ab26c@sudeepholla/

RE: [PATCH v3] arch_topology: Fix incorrect error check in topology_parse_cpu_capacity()

[Kumar, Kaushlendra]

On Tue, Sep 23, 2025 at 3:30 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> On Tue, Sep 23, 2025 at 03:15:14PM +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().
> > 
> > Per include/linux/err.h documentation, PTR_ERR_OR_ZERO(ptr) returns:
> > "The error code within @ptr if it is an error pointer; 0 otherwise."
> > 
> > This means PTR_ERR_OR_ZERO() returns 0 for both valid pointers AND 
> > NULL pointers. Therefore !PTR_ERR_OR_ZERO(cpu_clk) evaluates to true 
> > (proceed) when cpu_clk is either valid or NULL, causing 
> > clk_get_rate(NULL) to be called when of_clk_get() returns NULL.
> > 
> > Replace with !IS_ERR_OR_NULL(cpu_clk) which only proceeds for valid 
> > pointers, preventing potential NULL pointer dereference in clk_get_rate().
> > 
> > Fixes: b8fe128dad8f ("arch_topology: Adjust initial CPU capacities 
> > with current freq")
> > Cc: stable@vger.kernel.org
> > 
> 
> I wonder if you missed my response on v1[1] before you sent v2/v3 so quickly.
> The reviewed by tag still stands, just for sake of tools:
> 
> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> 
> --
> Regards,
> Sudeep
> 
> [1] https://lore.kernel.org/all/20250923-spectral-rich-shellfish-3ab26c@sudeepholla/

Hi Sudeep,

Thank you for the clarification and for providing the Reviewed-by tag!

You're absolutely right - I apologize for missing your v1 response before 
sending v2/v3. I was focused on addressing the feedback from other reviewers 
(particularly Markus Elfring's suggestions about commit message improvements 
and documentation compliance) and didn't properly check for your response first.

I really appreciate you maintaining the Reviewed-by tag through the versions, 
and I'll make sure to check all responses more carefully before sending 
subsequent versions in the future.

If possible you can ignore the later version of patch.

Thank you for the review and for pointing out this process oversight.

Best regards,
Kaushlendra

Re: [PATCH v3] arch_topology: Fix incorrect error check in topology_parse_cpu_capacity()

[Sudeep Holla]

On Tue, Sep 23, 2025 at 06:03:08PM +0000, Kumar, Kaushlendra wrote:
> On Tue, Sep 23, 2025 at 3:30 PM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > On Tue, Sep 23, 2025 at 03:15:14PM +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().
> > > 
> > > Per include/linux/err.h documentation, PTR_ERR_OR_ZERO(ptr) returns:
> > > "The error code within @ptr if it is an error pointer; 0 otherwise."
> > > 
> > > This means PTR_ERR_OR_ZERO() returns 0 for both valid pointers AND 
> > > NULL pointers. Therefore !PTR_ERR_OR_ZERO(cpu_clk) evaluates to true 
> > > (proceed) when cpu_clk is either valid or NULL, causing 
> > > clk_get_rate(NULL) to be called when of_clk_get() returns NULL.
> > > 
> > > Replace with !IS_ERR_OR_NULL(cpu_clk) which only proceeds for valid 
> > > pointers, preventing potential NULL pointer dereference in clk_get_rate().
> > > 
> > > Fixes: b8fe128dad8f ("arch_topology: Adjust initial CPU capacities 
> > > with current freq")
> > > Cc: stable@vger.kernel.org
> > > 
> > 
> > I wonder if you missed my response on v1[1] before you sent v2/v3 so quickly.
> > The reviewed by tag still stands, just for sake of tools:
> > 
> > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
> > 
> > --
> > Regards,
> > Sudeep
> > 
> > [1] https://lore.kernel.org/all/20250923-spectral-rich-shellfish-3ab26c@sudeepholla/
> 
> Hi Sudeep,
> 
> Thank you for the clarification and for providing the Reviewed-by tag!
> 

You are welcome.

> You're absolutely right - I apologize for missing your v1 response before 
> sending v2/v3. I was focused on addressing the feedback from other reviewers 
> (particularly Markus Elfring's suggestions about commit message improvements 
> and documentation compliance) and didn't properly check for your response first.
> 

Please take a look at these
https://lkml.org/lkml/2020/6/28/157
https://lkml.org/lkml/2024/6/25/1026
https://lkml.org/lkml/2024/1/30/1116
https://lkml.org/lkml/2025/9/2/812

> I really appreciate you maintaining the Reviewed-by tag through the versions, 
> and I'll make sure to check all responses more carefully before sending 
> subsequent versions in the future.
> 

Thanks.

> If possible you can ignore the later version of patch.
> 

Hmm, I see you have managed to send v4 before seeing my response on v1 and v3
and hence didn't add my review tag 🙁.

-- 
Regards,
Sudeep