[PATCH v1 1/4] firmware: arm_scpi: fix device_node leak in scpi_dev_domain_id

[PATCH v1 1/4] firmware: arm_scpi: fix device_node leak in scpi_dev_domain_id

[Xixin Liu]

of_parse_phandle_with_args() takes a reference on clkspec.np that must be
released with of_node_put(). scpi_dev_domain_id() returned clkspec.args[0]
without dropping that reference, so every domain lookup leaked a device
node. Paths such as scpi_dvfs_info() / cpufreq init call this per CPU, so
the leak accumulates over time.

Save the domain id, of_node_put(clkspec.np), then return the saved value.

Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 drivers/firmware/arm_scpi.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
index 87c323de17b9..6056754ef48c 100644
--- a/drivers/firmware/arm_scpi.c
+++ b/drivers/firmware/arm_scpi.c
@@ -660,12 +660,15 @@ static struct scpi_dvfs_info *scpi_dvfs_get_info(u8 domain)
 static int scpi_dev_domain_id(struct device *dev)
 {
 	struct of_phandle_args clkspec;
+	int domain;
 
 	if (of_parse_phandle_with_args(dev->of_node, "clocks", "#clock-cells",
 				       0, &clkspec))
 		return -EINVAL;
 
-	return clkspec.args[0];
+	domain = clkspec.args[0];
+	of_node_put(clkspec.np);
+	return domain;
 }
 
 static struct scpi_dvfs_info *scpi_dvfs_info(struct device *dev)
-- 
2.53.0

[PATCH v1 3/4] clk: scpi: bound-check DVFS index in scpi_dvfs_recalc_rate

[Xixin Liu]

dvfs_get_idx() may return an out-of-range index if the SCP firmware is
buggy or returns a stale value. Only negative indexes were rejected, so a
large index walked past info->opps and could treat garbage as a clock rate
(KASAN OOB / wrong frequency to consumers).

Treat indexes >= opp count as invalid and return 0, same as idx < 0.

Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 drivers/clk/clk-scpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-scpi.c b/drivers/clk/clk-scpi.c
index 19d530d52e64..4e22ba12b157 100644
--- a/drivers/clk/clk-scpi.c
+++ b/drivers/clk/clk-scpi.c
@@ -85,7 +85,7 @@ static unsigned long scpi_dvfs_recalc_rate(struct clk_hw *hw,
 	int idx = clk->scpi_ops->dvfs_get_idx(clk->id);
 	const struct scpi_opp *opp;
 
-	if (idx < 0)
+	if (idx < 0 || idx >= clk->info->count)
 		return 0;
 
 	opp = clk->info->opps + idx;
-- 
2.53.0

[PATCH v1 2/4] firmware: arm_scpi: reject DVFS OPP count above MAX_DVFS_OPPS

[Xixin Liu]

scpi_dvfs_get_info() already rejected a zero opp_count, but still trusted
any larger value from the SCP firmware. The shared-memory reply only holds
MAX_DVFS_OPPS entries in buf.opps[]; a bigger count over-reads that array
and then sizes the allocated OPP table incorrectly (garbage OPPs / OOB).

Reject zero and out-of-range counts in one check and return -EINVAL.

Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 drivers/firmware/arm_scpi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
index 6056754ef48c..5db58bdfc947 100644
--- a/drivers/firmware/arm_scpi.c
+++ b/drivers/firmware/arm_scpi.c
@@ -630,8 +630,8 @@ static struct scpi_dvfs_info *scpi_dvfs_get_info(u8 domain)
 	if (ret)
 		return ERR_PTR(ret);
 
-	if (!buf.opp_count)
-		return ERR_PTR(-ENOENT);
+	if (!buf.opp_count || buf.opp_count > MAX_DVFS_OPPS)
+		return ERR_PTR(-EINVAL);
 
 	info = kmalloc(sizeof(*info), GFP_KERNEL);
 	if (!info)
-- 
2.53.0

[PATCH v1 4/4] clk: scpi: register scpi-cpufreq once and clear on failure

[Xixin Liu]

scpi_clk_probe() walks clock children and, for each DVFS provider, calls
platform_device_register_simple("scpi-cpufreq"). Two related bugs:

1) Multiple DVFS children each spawned another virtual cpufreq device.
   If cpufreq_dev is already set, continue and skip a second register.

2) On IS_ERR, the pointer still held ERR_PTR (e.g. -ENOMEM). remove()
   treats any non-NULL cpufreq_dev as live and would call
   platform_device_unregister() on the error pointer (oops). Clear
   cpufreq_dev to NULL after the failure warn so remove() skips it.

Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 drivers/clk/clk-scpi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-scpi.c b/drivers/clk/clk-scpi.c
index 4e22ba12b157..ce6baea8f82e 100644
--- a/drivers/clk/clk-scpi.c
+++ b/drivers/clk/clk-scpi.c
@@ -283,10 +283,14 @@ static int scpi_clocks_probe(struct platform_device *pdev)
 		if (match->data != &scpi_dvfs_ops)
 			continue;
 		/* Add the virtual cpufreq device if it's DVFS clock provider */
+		if (cpufreq_dev)
+			continue;
 		cpufreq_dev = platform_device_register_simple("scpi-cpufreq",
 							      -1, NULL, 0);
-		if (IS_ERR(cpufreq_dev))
+		if (IS_ERR(cpufreq_dev)) {
 			pr_warn("unable to register cpufreq device");
+			cpufreq_dev = NULL;
+		}
 	}
 	return 0;
 }
-- 
2.53.0

[PATCH v1 0/4] firmware/clk: arm_scpi hardening (leak, OPP bounds, cpufreq)

[Xixin Liu]

Hi,

This series hardens the OF SCPI firmware and clock paths against a few
real defects found while reviewing linux-next:

  1) device_node leak in scpi_dev_domain_id() after
     of_parse_phandle_with_args()
  2) DVFS OPP count from SCP trusted beyond MAX_DVFS_OPPS (OOB read /
     bad OPP table size)
  3) DVFS index used as clock rate without an upper bound check
  4) scpi-cpufreq registered once only, and cleared on register failure
     so remove() does not unregister an ERR_PTR

Please review.

Thanks,
Xixin Liu

---

Xixin Liu (4):
  firmware: arm_scpi: fix device_node leak in scpi_dev_domain_id
  firmware: arm_scpi: reject DVFS OPP count above MAX_DVFS_OPPS
  clk: scpi: bound-check DVFS index in scpi_dvfs_recalc_rate
  clk: scpi: register scpi-cpufreq once and clear on failure

 drivers/clk/clk-scpi.c      | 8 ++++++--
 drivers/firmware/arm_scpi.c | 9 ++++++---
 2 files changed, 12 insertions(+), 5 deletions(-)

-- 
2.53.0

Re: [PATCH v1 2/4] firmware: arm_scpi: reject DVFS OPP count above MAX_DVFS_OPPS

[Sudeep Holla]

On Mon, Jul 27, 2026 at 10:19:15AM +0800, Xixin Liu wrote:
> scpi_dvfs_get_info() already rejected a zero opp_count, but still trusted
> any larger value from the SCP firmware. The shared-memory reply only holds
> MAX_DVFS_OPPS entries in buf.opps[]; a bigger count over-reads that array
> and then sizes the allocated OPP table incorrectly (garbage OPPs / OOB).
> 
> Reject zero and out-of-range counts in one check and return -EINVAL.
> 

This fails to apply, you need to rebase to the latest.

-- 
Regards,
Sudeep

Re: [PATCH v1 2/4] firmware: arm_scpi: reject DVFS OPP count above MAX_DVFS_OPPS

[Sudeep Holla]

On Mon, Jul 27, 2026 at 10:19:15AM +0800, Xixin Liu wrote:
> scpi_dvfs_get_info() already rejected a zero opp_count, but still trusted
> any larger value from the SCP firmware. The shared-memory reply only holds
> MAX_DVFS_OPPS entries in buf.opps[]; a bigger count over-reads that array
> and then sizes the allocated OPP table incorrectly (garbage OPPs / OOB).
> 
> Reject zero and out-of-range counts in one check and return -EINVAL.
>

This issue of out-of-bounds read is present since the original SCPI DVFS
implementation. So please include the below fixes tag:

Fixes: 8cb7cf56c9fe ("firmware: add support for ARM System Control and Power Interface(SCPI) protocol")

-- 
Regards,
Sudeep

Re: [PATCH v1 3/4] clk: scpi: bound-check DVFS index in scpi_dvfs_recalc_rate

[Sudeep Holla]

On Mon, Jul 27, 2026 at 10:19:15AM +0800, Xixin Liu wrote:
> dvfs_get_idx() may return an out-of-range index if the SCP firmware is
> buggy or returns a stale value. Only negative indexes were rejected, so a
> large index walked past info->opps and could treat garbage as a clock rate
> (KASAN OOB / wrong frequency to consumers).
> 
> Treat indexes >= opp count as invalid and return 0, same as idx < 0.
> 

Again this issue of out-of-bounds read is in the original SCPI clock driver.
So include the below fixes tag:

Fixes: cd52c2a4b5c4 ("clk: add support for clocks provided by SCP(System Control Processor)")

-- 
Regards,
Sudeep

Re: [PATCH v1 4/4] clk: scpi: register scpi-cpufreq once and clear on failure

[Sudeep Holla]

On Mon, Jul 27, 2026 at 10:19:15AM +0800, Xixin Liu wrote:
> scpi_clk_probe() walks clock children and, for each DVFS provider, calls
> platform_device_register_simple("scpi-cpufreq"). Two related bugs:
> 
The function named above appears to be scpi_clocks_probe(), not
scpi_clk_probe(). Fix that.

> 1) Multiple DVFS children each spawned another virtual cpufreq device.
>    If cpufreq_dev is already set, continue and skip a second register.
> 

The registration uses id -1, so every device must be named "scpi-cpufreq".
A second device_add() cannot register that name and must return -EEXIST;
platform_device_register_full() then drops the second pdev.

So the above information could be misleading.

Can you state it as each additional DVFS child attempts a duplicate
registration and overwrites the first valid pointer with ERR_PTR(-EEXIST),
rather than saying that each child spawns another device?

> 2) On IS_ERR, the pointer still held ERR_PTR (e.g. -ENOMEM). remove()
>    treats any non-NULL cpufreq_dev as live and would call
>    platform_device_unregister() on the error pointer (oops). Clear
>    cpufreq_dev to NULL after the failure warn so remove() skips it.
>

This also fixes bugs introduced by the original virtual device support and
by moving registration inside the child loop. So

Fixes: 9490f01e2471 ("clk: scpi: add support for cpufreq virtual device")
Fixes: 67bcc2c5f1da ("clk: scpi: don't add cpufreq device if the scpi dvfs node is disabled")

-- 
Regards,
Sudeep

[PATCH v2 4/5] clk: scpi: register scpi-cpufreq once and clear on failure

[Xixin Liu]

scpi_clocks_probe() walks clock children and, for each DVFS provider, calls
platform_device_register_simple("scpi-cpufreq", -1, ...). Two related bugs:

1) Registration uses id = -1 and the fixed name "scpi-cpufreq". A second
   registration of the same name returns -EEXIST and overwrites the first
   good cpufreq_dev with ERR_PTR(-EEXIST). When cpufreq_dev is already
   set, continue so only the first successful registration remains.

2) On IS_ERR, cpufreq_dev still held the ERR_PTR; remove() would then
   call platform_device_unregister() on it (oops). Clear cpufreq_dev to
   NULL after the failure warn so remove() skips it.

Fixes: 9490f01e2471 ("clk: scpi: add support for cpufreq virtual device")
Fixes: 67bcc2c5f1da ("clk: scpi: don't add cpufreq device if the scpi dvfs node is disabled")
Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 drivers/clk/clk-scpi.c | 6 +++++-
 
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk-scpi.c b/drivers/clk/clk-scpi.c
index 3c33a9e3c690..e1891351c221 100644
--- a/drivers/clk/clk-scpi.c
+++ b/drivers/clk/clk-scpi.c
@@ -284,10 +284,14 @@ static int scpi_clocks_probe(struct platform_device *pdev)
 		if (match->data != &scpi_dvfs_ops)
 			continue;
 		/* Add the virtual cpufreq device if it's DVFS clock provider */
+		if (cpufreq_dev)
+			continue;
 		cpufreq_dev = platform_device_register_simple("scpi-cpufreq",
 							      -1, NULL, 0);
-		if (IS_ERR(cpufreq_dev))
+		if (IS_ERR(cpufreq_dev)) {
 			pr_warn("unable to register cpufreq device");
+			cpufreq_dev = NULL;
+		}
 	}
 	return 0;
 }
-- 
2.53.0

[PATCH v2 1/5] firmware: arm_scpi: fix device_node leak in scpi_dev_domain_id

[Xixin Liu]

of_parse_phandle_with_args() takes a reference on clkspec.np that must be
released with of_node_put(). scpi_dev_domain_id() returned clkspec.args[0]
without dropping that reference, so every domain lookup leaked a device
node. Paths such as scpi_dvfs_info() / cpufreq init call this per CPU, so
the leak accumulates over time.

Save the domain id, of_node_put(clkspec.np), then return the saved value.

Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 drivers/firmware/arm_scpi.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
index 2acad5fa5a28..43d6d9bbc7a9 100644
--- a/drivers/firmware/arm_scpi.c
+++ b/drivers/firmware/arm_scpi.c
@@ -661,12 +661,15 @@ static struct scpi_dvfs_info *scpi_dvfs_get_info(u8 domain)
 static int scpi_dev_domain_id(struct device *dev)
 {
 	struct of_phandle_args clkspec;
+	int domain;
 
 	if (of_parse_phandle_with_args(dev->of_node, "clocks", "#clock-cells",
 				       0, &clkspec))
 		return -EINVAL;
 
-	return clkspec.args[0];
+	domain = clkspec.args[0];
+	of_node_put(clkspec.np);
+	return domain;
 }
 
 static struct scpi_dvfs_info *scpi_dvfs_info(struct device *dev)
-- 
2.53.0

[PATCH v2 2/5] firmware: arm_scpi: reject DVFS OPP count above MAX_DVFS_OPPS

[Xixin Liu]

scpi_dvfs_get_info() already rejected a zero opp_count, but still trusted
any larger value from the SCP firmware. The shared-memory reply only holds
MAX_DVFS_OPPS entries in buf.opps[]; a bigger count over-reads that array
and then sizes the allocated OPP table incorrectly (garbage OPPs / OOB).
The missing upper bound dates back to the original SCPI DVFS support.

Reject zero and out-of-range counts in one check and return -EINVAL.

Fixes: 8cb7cf56c9fe ("firmware: add support for ARM System Control and Power Interface(SCPI) protocol")
Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 drivers/firmware/arm_scpi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/firmware/arm_scpi.c b/drivers/firmware/arm_scpi.c
index 43d6d9bbc7a9..68a730d22037 100644
--- a/drivers/firmware/arm_scpi.c
+++ b/drivers/firmware/arm_scpi.c
@@ -631,8 +631,8 @@ static struct scpi_dvfs_info *scpi_dvfs_get_info(u8 domain)
 	if (ret)
 		return ERR_PTR(ret);
 
-	if (!buf.opp_count)
-		return ERR_PTR(-ENOENT);
+	if (!buf.opp_count || buf.opp_count > MAX_DVFS_OPPS)
+		return ERR_PTR(-EINVAL);
 
 	info = kmalloc_obj(*info);
 	if (!info)
-- 
2.53.0

[PATCH v2 3/5] clk: scpi: bound-check DVFS index in scpi_dvfs_recalc_rate

[Xixin Liu]

dvfs_get_idx() may return an out-of-range index if the SCP firmware is
buggy or returns a stale value. Only negative indexes were rejected, so a
large index walked past info->opps and could treat garbage as a clock rate
(KASAN OOB / wrong frequency to consumers). The missing upper bound dates
back to the original SCPI clock driver.

Treat indexes >= opp count as invalid and return 0, same as idx < 0.

Fixes: cd52c2a4b5c4 ("clk: add support for clocks provided by SCP(System Control Processor)")
Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 drivers/clk/clk-scpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-scpi.c b/drivers/clk/clk-scpi.c
index 24cee7c9fda6..3c33a9e3c690 100644
--- a/drivers/clk/clk-scpi.c
+++ b/drivers/clk/clk-scpi.c
@@ -85,7 +85,7 @@ static unsigned long scpi_dvfs_recalc_rate(struct clk_hw *hw,
 	int idx = clk->scpi_ops->dvfs_get_idx(clk->id);
 	const struct scpi_opp *opp;
 
-	if (idx < 0)
+	if (idx < 0 || idx >= clk->info->count)
 		return 0;
 
 	opp = clk->info->opps + idx;
-- 
2.53.0

[PATCH v2 0/5] firmware/clk: arm_scpi hardening (leak, OPP bounds, cpufreq)

[Xixin Liu]

Hi Sudeep,

Thanks for reviewing v1.

This v2 hardens the OF SCPI firmware and clock paths against a few
real defects found while reviewing linux-next:

  1) device_node leak in scpi_dev_domain_id() after
     of_parse_phandle_with_args()
  2) DVFS OPP count from SCP trusted beyond MAX_DVFS_OPPS (OOB read /
     bad OPP table size)
  3) DVFS index used as clock rate without an upper bound check
  4) scpi-cpufreq registered once only, and cleared on register failure
     so remove() does not unregister an ERR_PTR
  5) use PLATFORM_DEVID_NONE instead of bare -1 for the scpi-cpufreq
     platform device id (readability; Fixes the original registration)

Changes since v1:
- 2/5: add Fixes: 8cb7cf56c9fe ("firmware: add support for ARM System
  Control and Power Interface(SCPI) protocol")
- 3/5: add Fixes: cd52c2a4b5c4 ("clk: add support for clocks provided by
  SCP(System Control Processor)")
- 4/5: correct commit message (scpi_clocks_probe; multi-DVFS child
  overwrites the first good cpufreq_dev with ERR_PTR(-EEXIST)) and
  add Fixes: 9490f01e2471 / 67bcc2c5f1da
  All per maintainer feedback; code unchanged.
- add 5/5: clk: scpi: use PLATFORM_DEVID_NONE for scpi-cpufreq
  (readability; Fixes: 9490f01e2471)
- Rebased onto linux-next next-20260727 (0d33d21e47d9); 2/5 context
  updated for kmalloc_obj() (logic unchanged).

Please review.

Thanks,
Xixin Liu

---

Xixin Liu (5):
  firmware: arm_scpi: fix device_node leak in scpi_dev_domain_id
  firmware: arm_scpi: reject DVFS OPP count above MAX_DVFS_OPPS
  clk: scpi: bound-check DVFS index in scpi_dvfs_recalc_rate
  clk: scpi: register scpi-cpufreq once and clear on failure
  clk: scpi: use PLATFORM_DEVID_NONE for scpi-cpufreq

 drivers/clk/clk-scpi.c      | 10 ++++++++--
 drivers/firmware/arm_scpi.c |  9 ++++++---
 2 files changed, 13 insertions(+), 6 deletions(-)

-- 
2.53.0

[PATCH v2 5/5] clk: scpi: use PLATFORM_DEVID_NONE for scpi-cpufreq

[Xixin Liu]

Replace the magic -1 passed to platform_device_register_simple() with
PLATFORM_DEVID_NONE. The constant is defined as (-1) in
platform_device.h; this is a readability cleanup only and does not
change behavior.

Fixes: 9490f01e2471 ("clk: scpi: add support for cpufreq virtual device")
Signed-off-by: Xixin Liu <liuxixin@kylinos.cn>
---
 drivers/clk/clk-scpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-scpi.c b/drivers/clk/clk-scpi.c
index e1891351c221..900488dedf95 100644
--- a/drivers/clk/clk-scpi.c
+++ b/drivers/clk/clk-scpi.c
@@ -287,7 +287,7 @@ static int scpi_clocks_probe(struct platform_device *pdev)
 		if (cpufreq_dev)
 			continue;
 		cpufreq_dev = platform_device_register_simple("scpi-cpufreq",
-							      -1, NULL, 0);
+							      PLATFORM_DEVID_NONE, NULL, 0);
 		if (IS_ERR(cpufreq_dev)) {
 			pr_warn("unable to register cpufreq device");
 			cpufreq_dev = NULL;
-- 
2.53.0