On Fri, Sep 4, 2026 at 5:58 AM Roman Demidov <[email protected]> wrote: > > The return values of pm_runtime_resume_and_get() calls in a6xx_pm_resume() > are not checked, which can lead to hardware access on suspended devices > and PM reference underflows. > > Fix this by checking the return value of each pm_runtime_resume_and_get() > call and properly unwinding the previously acquired resources on failure. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Fixes: 5a903a44a984 ("drm/msm/a6xx: Introduce GMU wrapper support") > Signed-off-by: Roman Demidov <[email protected]> > --- > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 38 +++++++++++++++------------ > 1 file changed, 21 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > index f9de9329dee3..0cf205ea744b 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > @@ -2175,44 +2175,48 @@ static int a6xx_pm_resume(struct msm_gpu *gpu) > opp = dev_pm_opp_find_freq_ceil(&gpu->pdev->dev, &freq); > if (IS_ERR(opp)) { > ret = PTR_ERR(opp); > - goto err_set_opp; > + goto err_unlock; > } > dev_pm_opp_put(opp); > > /* Set the core clock and bus bw, having VDD scaling in mind */ > dev_pm_opp_set_opp(&gpu->pdev->dev, opp); > > - pm_runtime_resume_and_get(gmu->dev); > - pm_runtime_resume_and_get(gmu->gxpd); > + ret = pm_runtime_resume_and_get(gmu->dev); > + if (ret < 0) > + goto err_opp_clear; > + ret = pm_runtime_resume_and_get(gmu->gxpd); > + if (ret < 0) > + goto err_put_dev; > > ret = clk_bulk_prepare_enable(gpu->nr_clocks, gpu->grp_clks); > if (ret) > - goto err_bulk_clk; > + goto err_put_gxpd; > > ret = clk_bulk_prepare_enable(gmu->nr_clocks, gmu->clocks); > if (ret) { > clk_bulk_disable_unprepare(gpu->nr_clocks, gpu->grp_clks); > - goto err_bulk_clk; > + goto err_put_gxpd; > } > > if (adreno_is_a619_holi(adreno_gpu)) > a6xx_sptprac_enable(gmu); > > - /* If anything goes south, tear the GPU down piece by piece.. */ > - if (ret) { > -err_bulk_clk: > - pm_runtime_put(gmu->gxpd); > - pm_runtime_put(gmu->dev); > - dev_pm_opp_set_opp(&gpu->pdev->dev, NULL); > - } > -err_set_opp: > mutex_unlock(&a6xx_gpu->gmu.lock); > + msm_devfreq_resume(gpu); > + a6xx_llc_activate(a6xx_gpu); > > - if (!ret) { > - msm_devfreq_resume(gpu); > - a6xx_llc_activate(a6xx_gpu); > - } > + return 0; > > + /* If anything goes south, tear the GPU down piece by piece.. */ > +err_put_gxpd: > + pm_runtime_put(gmu->gxpd); > +err_put_dev: > + pm_runtime_put(gmu->dev); > +err_opp_clear: > + dev_pm_opp_set_opp(&gpu->pdev->dev, NULL); > +err_unlock: > + mutex_unlock(&a6xx_gpu->gmu.lock);
this does at least look a bit less terrifying than what came before... but maybe guard(mutex)(&a6xx_gpu->gmu.lock); to simplify the locking part of this. And I think at least some of the runpm stuff could also be handled w/ guard/cleanup stuff? BR, -R > return ret; > } > > -- > 2.53.0 >
