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

[Tudor Ambarus <tudor.ambarus@linaro.org>]

On 9/8/26 3:37 PM, Peter Griffin wrote:
> 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");
> -		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>

> +		dev_pm_opp_put_opp_table(opp_table);
> +		return ERR_PTR(ret);
>  	}
>  
>  	if (opp_table->clk)
> 
> ---
> base-commit: df2908090cda368b01ff43709f51890076c56157
> change-id: 20260908-opp-core-uaf-update-opp-table-0d9ea3741231
> 
> Best regards,