Re: [PATCH] opp: fix use after free in _update_opp_table_clk()

On 08-09-26, 16:41, Tudor Ambarus wrote:

On 9/8/26 3:37 PM, Peter Griffin wrote:

quoted

dev_pm_opp_put_opp_table() frees the opp_table which is subsquently used
by dev_err_probe(). This causes an Oops during boot on gs101-oriole.

cpu cpu0: error 000000006b6b6b6b: Couldn't find clock
Unable to handle kernel paging request at virtual address 006b6b6b6b6b6cd3
...
Hardware name: Oriole (DT)
pstate: 00400005 (nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : _of_add_table_indexed+0x80/0xbb0
lr : _of_add_table_indexed+0x6c/0xbb0
...
Call trace:
 _of_add_table_indexed+0x80/0xbb0 (P)
 dev_pm_opp_of_cpumask_add_table+0x70/0x120
 dt_cpufreq_probe+0x23c/0x480
 platform_probe+0x64/0xb8

Fixes: 84f05af0975c9 ("opp: Use clk_get_optional() to avoid leaving opp_table->clk as an error pointer")
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
This UAF regression was introduced in v7.3-rc1, and causes an Oops on
boot for gs101-oriole (in part due to our default upstream kernel dev
config having memory poisoning enabled, hence the 0x6b6b6b6b
pattern). It is intended that this patch should get picked for the
next -rc.

Peter
---
---
 drivers/opp/core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 2fafd983de8f5..c8b941cb81b44 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c

@@ -1581,6 +1581,7 @@ static struct opp_table *_update_opp_table_clk(struct device *dev,
 					       struct opp_table *opp_table,
 					       bool getclk)
 {
+	int ret;
 	/*
 	 * Return early if we don't need to get clk or we have already done it
 	 * earlier.

@@ -1607,9 +1608,10 @@ static struct opp_table *_update_opp_table_clk(struct device *dev,
 	opp_table->clk = clk_get_optional(dev, NULL);
 
 	if (IS_ERR(opp_table->clk)) {
-		dev_pm_opp_put_opp_table(opp_table);
 		dev_err_probe(dev, PTR_ERR(opp_table->clk), "Couldn't find clock\n");

you can do ret = dev_err_probe(dev, PTR_ERR(opp_table->clk), "Couldn't find clock\n");

quoted

-		return ERR_CAST(opp_table->clk);
+		ret = PTR_ERR(opp_table->clk);

and you won't need this duplicated line.

with this addressed:
Reviewed-by: Tudor Ambarus <tudor.ambarus@linaro.org>

Applied with the fix. Thanks.

-- 
viresh