Applied. Thanks! Alex
On Thu, Oct 15, 2026 at 3:41 PM Olivier Kaloudoff <[email protected]> wrote: > > Since commit 037b98a2312e ("drm/amdgpu: move UVD and VCE sched entity > init after sched init"), drm_sched_entity_init() for the VCE and UVD > kernel entities runs in the scheduler-init phase, well after the > IP-block sw_init phase has allocated the vcpu_bos. > > If probe fails between the two phases, amdgpu_device_fini_sw() still > runs the per-IP sw_fini callbacks for blocks that did complete > sw_init. Both amdgpu_vce_sw_fini() and amdgpu_uvd_sw_fini() then call > drm_sched_entity_destroy() on an entity that was never initialized: > entity->rq is NULL and drm_sched_entity_flush() dereferences it, > oopsing in whatever context drove the failed probe. > > The hole was latent-but-safe until commit 2df5efb45425 ("drm/sched: > Remove redundant entity->rq initialization and checks") removed the > if (!entity->rq) early return that commit 1decbf6bb0b4 ("drm/sched: > Fix entities with 0 rqs.") had put in drm_sched_entity_flush(), on the > grounds that entity->rq can never be NULL. That holds only for > entities that completed drm_sched_entity_init() — not for the > never-initialized entity this teardown path reaches. > > Observed on real hardware: MI50 (Vega20, VCE 4.0 / UVD 7.2) behind a > Thunderbolt link — a pciehp presence/link-change event re-probes the > device; when the re-probe fails (-EINVAL here), the cleanup path > NULL-derefs inside drm_sched_entity_flush() in the pciehp IRQ thread > and takes the whole machine down (panic_on_oops, EFI pstore capture). > Reproduced twice on 7.3-rc5, 2026-10-06 and 2026-10-09, identical > signature; the faulting instruction is `mov (%rax),%r15` on rax = > entity->rq == NULL, i.e. `sched = entity->rq->sched`: > > BUG: kernel NULL pointer dereference > RIP: drm_sched_entity_flush+0x34/0x2d0 [gpu_sched] > drm_sched_entity_destroy > amdgpu_vce_sw_fini > vce_v4_0_sw_fini > amdgpu_device_fini_sw > ... > pciehp_configure_device > pciehp_handle_presence_or_link_change > > Guard the entity teardown on entity->rq, which is set only by > drm_sched_entity_init(). amdgpu_ring_fini() already self-guards via > the rings[] registration, and the vcpu_bo/firmware cleanup still runs > as before. > > v2: retarget Fixes: at commit 2df5efb45425 ("drm/sched: Remove > redundant entity->rq initialization and checks") — the !entity->rq > check from commit 1decbf6bb0b4 ("drm/sched: Fix entities with 0 > rqs.") that Alex pointed to was removed by it in the 7.3 window, > so this is a regression there, not a pre-existing hole. > > Fixes: 2df5efb45425 ("drm/sched: Remove redundant entity->rq initialization > and checks") > Cc: Tvrtko Ursulin <[email protected]> > Cc: Philipp Stanner <[email protected]> > Cc: Alex Deucher <[email protected]> > Cc: Christian König <[email protected]> > Signed-off-by: Olivier Kaloudoff <[email protected]> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 3 ++- > drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c | 3 ++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c > index ecd7caa95..cd35ff7ca 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c > @@ -377,7 +377,8 @@ int amdgpu_uvd_sw_fini(struct amdgpu_device *adev) > void *addr = amdgpu_bo_kptr(adev->uvd.ib_bo); > int i, j; > > - drm_sched_entity_destroy(&adev->uvd.entity); > + if (adev->uvd.entity.rq) > + drm_sched_entity_destroy(&adev->uvd.entity); > > for (j = 0; j < adev->uvd.num_uvd_inst; ++j) { > if (adev->uvd.harvest_config & (1 << j)) > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c > index 1f7a98cdc..3952af9ae 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c > @@ -253,7 +253,8 @@ int amdgpu_vce_sw_fini(struct amdgpu_device *adev) > if (adev->vce.vcpu_bo == NULL) > return 0; > > - drm_sched_entity_destroy(&adev->vce.entity); > + if (adev->vce.entity.rq) > + drm_sched_entity_destroy(&adev->vce.entity); > > for (i = 0; i < adev->vce.num_rings; i++) > amdgpu_ring_fini(&adev->vce.ring[i]); > -- > 2.39.5
