Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] drm/msm/dpu: use-after-free on hw_vbif during bind retry --
commit 010fe925950b01a30ca06eec05c2a4db435b69ed Author: Dmitry Baryshkov <[email protected]> drm/msm/dpu: unwind a failed dpu_kms_hw_init() step by step This commit replaces the blanket _dpu_kms_hw_destroy() call in the error path of dpu_kms_hw_init() with step-by-step unwinding. Only the completed steps are undone, in reverse order. > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > index 65ba8fa697e94..2de4d881d13d9 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c [ ... ] > @@ -1263,7 +1263,7 @@ static int dpu_kms_hw_init(struct dpu_kms *dpu_kms) > rc = _dpu_kms_drm_obj_init(dpu_kms); > if (rc) { > DPU_ERROR("modeset init failed: %d\n", rc); > - goto err_pm_put; > + goto err_mmu_destroy; > } > > dpu_vbif_init_memtypes(dpu_kms); > @@ -1272,10 +1272,12 @@ static int dpu_kms_hw_init(struct dpu_kms *dpu_kms) > > return 0; > > +err_mmu_destroy: > + _dpu_kms_mmu_destroy(dpu_kms); [Severity: High] If initialization fails after dpu_kms->hw_vbif is allocated, such as when _dpu_kms_drm_obj_init() fails, the error path jumps to err_mmu_destroy. Can this regression leave a dangling pointer in dpu_kms->hw_vbif? Since the memory for dpu_kms->hw_vbif is allocated via drmm, it will be freed when the component bind fails and the DRM device is torn down. However, dpu_kms itself persists because it is allocated via devm. Could a subsequent bind retry trigger a use-after-free? When a deferred probe or bind retry occurs, dpu_kms_hw_init() calls pm_runtime_resume_and_get(), which triggers the runtime PM resume callback and accesses the dangling pointer: dpu_runtime_resume() dpu_vbif_init_memtypes() vbif = dpu_kms->hw_vbif; if (vbif && vbif->cap && ...) This would result in a use-after-free on the previously freed vbif memory. Is it necessary to explicitly clear dpu_kms->hw_vbif in the unwind path to prevent it from being accessed during bind retries? > err_pm_put: > pm_runtime_put_sync(&dpu_kms->pdev->dev); > -error: > - _dpu_kms_hw_destroy(dpu_kms); > +err_global_obj_fini: > + dpu_kms_global_obj_fini(dpu_kms); > > return rc; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20261003-msm-kms-destroy-fixes-v1-0-e062b7dae...@oss.qualcomm.com?part=9
