RE: [PATCH] drm/amdgpu: fix NULL pointer deref in amdgpu_vm_handle_moved for BO-less mappings
"Zhang, Jesse(Jie)" <[email protected]> Wed, 5 Aug 2026 03:00:11 +0000
| Newsgroups | org.freedesktop.lists.amd-gfx |
|---|---|
| Message-ID | <DM4PR12MB5152FFAFB3DCD06996A45988E3D32@DM4PR12MB5152.namprd12.prod.outlook.com> |
AMD General > -----Original Message----- > From: Koenig, Christian <[email protected]> > Sent: Tuesday, August 4, 2026 9:58 PM > To: Zhang, Jesse(Jie) <[email protected]>; [email protected] > Cc: Deucher, Alexander <[email protected]> > Subject: Re: [PATCH] drm/amdgpu: fix NULL pointer deref in > amdgpu_vm_handle_moved for BO-less mappings > > On 7/27/26 10:40, Jesse Zhang wrote: > > The individual.needs_update loop in amdgpu_vm_handle_moved() > > unconditionally dereferenced bo_va->base.bo (to read its reservation > > object, ttm_tt and to test for a DMABuf import). However > > bo_va->base.bo can legitimately be NULL for PRT/sparse mappings - > > amdgpu_vm_bo_update() already handles a NULL bo via its "else if > > (!bo)" PRT path. When such a bo_va reaches the moved list (e.g. the > > userq eviction restore worker running amdgpu_vm_handle_moved() while a user > queue is being torn down during a GPU reset), the NULL deref crashes the kernel: > > > > BUG: kernel NULL pointer dereference, address: 0000000000000158 > > #PF: supervisor read access in kernel mode > > Oops: 0000 [#1] SMP NOPTI > > Workqueue: events amdgpu_userq_restore_worker [amdgpu] > > RIP: 0010:amdgpu_vm_handle_moved+0x17a/0x200 [amdgpu] > > Call Trace: > > <TASK> > > amdgpu_userq_vm_validate_and_restore_queue+0x2ce/0x920 [amdgpu] > > amdgpu_userq_restore_worker+0xce/0x210 [amdgpu] > > process_scheduled_works+0xa6/0x460 > > worker_thread+0x13c/0x290 > > kthread+0xfb/0x140 > > ret_from_fork+0x1b6/0x2b0 > > ret_from_fork_asm+0x1a/0x30 > > </TASK> > > > > The faulting instruction is "mov rax,[rdx+0x158]" with rdx (bo) == 0 > > and > > CR2 == 0x158, i.e. reading bo->tbo.base.resv off a NULL bo. > > > > Guard the BO-less case: skip the reservation dance and the > > DMABuf-import check when bo is NULL, and let amdgpu_vm_bo_update() take > its existing PRT path. > > Clear NAK. PRT mappings should *never* be on the moved list in the first place. Thanks for the review. I tracked the issue to the state-machine path: amdgpu_vm_bo_update() can put a NULL-bo bo_va onto individual.idle via amdgpu_vm_bo_idle(), and then amdgpu_vm_bo_reset_state_machine() promotes it to individual.needs_update after a VM generation reset. I’ll send v2 that fixes this at the producer side by keeping NULL-bo (PRT) mappings off the vm_bo state lists, and I’ll drop the consumer-side NULL guard from v1. Thanks, Jesse > > Regards, > Christian. > > > > > Signed-off-by: Jesse Zhang <[email protected]> > > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > index aac8ace9d7a6..fbb76c32bff9 100644 > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > > @@ -1631,11 +1631,17 @@ int amdgpu_vm_handle_moved(struct > amdgpu_device *adev, > > bo_va = list_first_entry(&vm->individual.needs_update, > > typeof(*bo_va), base.vm_status); > > bo = bo_va->base.bo; > > - resv = bo->tbo.base.resv; > > + resv = bo ? bo->tbo.base.resv : NULL; > > spin_unlock(&vm->individual_lock); > > > > + /* PRT/sparse mappings have no BO to reserve; just update the > > + * page tables (amdgpu_vm_bo_update() handles a NULL bo). > > + */ > > + if (!bo) { > > + clear = false; > > + unlock = false; > > /* Try to reserve the BO to avoid clearing its ptes */ > > - if (!adev->debug_vm && !amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) > && > > + } else if (!adev->debug_vm && > > +!amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) && > > dma_resv_trylock(resv)) { > > clear = false; > > unlock = true; > > @@ -1659,7 +1665,7 @@ int amdgpu_vm_handle_moved(struct amdgpu_device > *adev, > > /* Remember evicted DMABuf imports in compute VMs for later > > * validation > > */ > > - if (vm->is_compute_context && > > + if (bo && vm->is_compute_context && > > drm_gem_is_imported(&bo_va->base.bo->tbo.base) && > > (!bo_va->base.bo->tbo.resource || > > bo_va->base.bo->tbo.resource->mem_type == > TTM_PL_SYSTEM))