RE: [PATCH] drm/amdgpu: fix NULL pointer deref in amdgpu_vm_handle_moved for BO-less mappings

AMD General

> -----Original Message-----
> From: Koenig, Christian <[email protected]>
> Sent: Wednesday, August 5, 2026 5:31 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 8/5/26 11:01, Zhang, Jesse(Jie) wrote:
> > AMD General
> >
> >> -----Original Message-----
> >> From: Koenig, Christian <[email protected]>
> >> Sent: Wednesday, August 5, 2026 3:53 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 8/5/26 05:00, Zhang, Jesse(Jie) wrote:
> >>> 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.
> >>
> >> Something is seriously wrong here. I've fixed
> >> amdgpu_vm_bo_reset_state_machine() to not do that weeks ago.
> >>
> >> Where do you see that issue? On some release branch?
> > Hi Christian,
> >
> > Thanks — I found the fix you mean: 989bdec8ca04 ("drm/amdgpu: fix
> amdgpu_vm_bo_reset_state_machine", 2026-05-22)。 It's already in amd-staging-
> drm-next.
> >
> > But that commit addresses a different problem: it replaces the list_splice in
> reset_state_machine() so the root PD gets its special handling and the moved flag
> is set correctly.
> > It does not touch the NULL-bo case — after it, an entry on individual.idle is still
> list_move'd onto individual.needs_update.
> >
> > The crash I'm reporting has a different root cause: a PRT/sparse mapping's
> bo_va has base.bo == NULL (fpriv->prt_va, created via amdgpu_vm_bo_add(vm,
> NULL)).
> > The tail of amdgpu_vm_bo_update() unconditionally calls amdgpu_vm_bo_idle() in
> the !always_valid else branch, putting that NULL-bo bo_va onto individual.idle.
> > After a GPU reset reset_state_machine() promotes it to individual.needs_update,
> and amdgpu_vm_handle_moved() then dereferences bo_va->base.bo
> unconditionally.
> >
> > I can still reproduce this on the latest amd-staging-drm-next (HEAD includes
> 989bdec8ca04).
> > the log:
> > [ 3107.632098] RIP: 0010:amdgpu_vm_handle_moved+0x17a/0x200 [amdgpu] [
> > 3107.632469] Code: e8 5b c7 f9 ee 49 8b 84 24 60 02 00 00 48 3b 45 b8
> > 0f 84 45 ff ff ff 4d 8b ac 24 60 02 00 00 4c 89 ff 49 8b 55 f0 4d 8d
> > 75 e8 <48> 8b 82 58 01 00 00 48 89 55 c8 48 89 45 d0 e8 92 c0 f9 ee 0f
> > b6 [ 3107.632478] RSP: 0018:ffffcda785f23cb0 EFLAGS: 00010283 [
> > 3107.632484] RAX: ffff8ad9cc15fe98 RBX: ffff8ada86100000 RCX:
> > ffff8ad9cf4a8188 [ 3107.632488] RDX: 0000000000000000 RSI:
> > 000000000000000f RDI: ffff8adab01c0248 [ 3107.632493] RBP:
> > ffffcda785f23d00 R08: 0000000000000000 R09: 0000000000000004 [
> > 3107.632497] R10: 0000000000000000 R11: 0000000000000000 R12:
> > ffff8adab01c0000 [ 3107.632501] R13: ffff8ad9cc15fe98 R14:
> > ffff8ad9cc15fe80 R15: ffff8adab01c0248 [ 3107.632505] FS:
> > 0000000000000000(0000) GS:ffff8add3c8ff000(0000) knlGS:0000000000000000
> [ 3107.632510] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 3107.632515] CR2: 0000000000000158 CR3: 0000000117903000 CR4:
> 0000000000750ef0 [ 3107.632520] PKRU: 55555554 [ 3107.632523] Call Trace:
> > [ 3107.632527]  <TASK>
> > [ 3107.632533]  amdgpu_userq_vm_validate_and_restore_queue+0x2ce/0x920
> > [amdgpu] [ 3107.632846]  amdgpu_userq_restore_worker+0xa6/0x180
> > [amdgpu] [ 3107.633036]  process_scheduled_works+0xa6/0x460
> > [ 3107.633042]  worker_thread+0x13c/0x290 [ 3107.633045]
> > kthread+0xfb/0x140 [ 3107.633048]  ? __pfx_worker_thread+0x10/0x10 [
> > 3107.633050]  ? __pfx_kthread+0x10/0x10 [ 3107.633053]
> > ret_from_fork+0x1b6/0x2b0 [ 3107.633056]  ? __pfx_kthread+0x10/0x10 [
> > 3107.633058]  ret_from_fork_asm+0x1a/0x30
>
> There is still something going wrong here.
>
> The PRT should be updated in the PTEs separately, otherwise we use the wrong
> fence to sync it.
>
> There should be something like this in the userq validation code as well:
>
>         r = amdgpu_vm_bo_update(adev, fpriv->prt_va, false);
>         if (r)
>                 return r;
>
>         r = amdgpu_sync_fence(&p->sync, fpriv->prt_va->last_pt_update,
>                               GFP_KERNEL); ...
>
> Similar to how we have it in the CS code.
You're right — the PRT PTEs are updated separately, so their fence lands in prt_va->last_pt_update, not vm->last_update, and nothing waited on it.
v3 adds an explicit dma_fence_wait on fpriv->prt_va->last_pt_update in the userq restore path before restarting the queues, mirroring the CS path.

Thanks
Jesse
>
> Regards,
> Christian.
>
> >
> >
> > So this is a distinct issue from 989bdec8ca04.
> > My v2 keeps the PRT bo_va off the state lists at the producer side
> (amdgpu_vm_bo_update: else → else if (bo)), which is what your earlier feedback
> asked for.
> >
> > Thanks,
> > Jesse
> >>
> >> Regards,
> >> Christian.
> >>
> >>>
> >>> 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))
> >>>
> >