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

[Viresh Kumar]

On 08-09-26, 16:41, Tudor Ambarus wrote:
> 
> 
> 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@xxxxxxxxxx>
> > ---
> > 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@xxxxxxxxxx>

Applied with the fix. Thanks.

-- 
viresh