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))