Re: [PATCH] drm/amdkfd: fix NULL pointer dereference in GFX12 CRIU queue restore

On Tue, Aug 4, 2026 at 3:33 PM Marioukhine, Vladimir
<[email protected]> wrote:
>
> AMD General
>
>
> kfd_mqd_manager_v12.c (GFX 12.0) and kfd_mqd_manager_v12_1.c (GFX 12.1)
>
> initialize their MQD managers without assigning restore_mqd or checkpoint_mqd
>
> callbacks, leaving both function pointers NULL.
>
>
>
> Both create_queue_cpsch() and create_queue_nocpsch() in
>
> kfd_device_queue_manager.c unconditionally call mqd_mgr->restore_mqd() when
>
> a CRIU restore is in progress (qd != NULL), with no NULL guard. On any system
>
> with a GFX 12.x GPU, a user holding CAP_CHECKPOINT_RESTORE can trigger a
>
> kernel NULL pointer dereference and panic the machine by issuing
>
> KFD_IOC_CRIU_OP_RESTORE with a crafted queue restore object.
>
>
>
> Fix this by:
>
> 1. Implementing restore_mqd and checkpoint_mqd callbacks for both compute
>
>    and SDMA queues in kfd_mqd_manager_v12.c and kfd_mqd_manager_v12_1.c,
>
>    modeled after the GFX 11 implementation.
>
> 2. Adding a NULL guard at both call sites in kfd_device_queue_manager.c
>
>    as a defensive measure against future regressions.
>
>
>
> Fixes: 48f0bdf4e38e ("drm/amdkfd: Added MQD manager files for GFX12.")
>
> Fixes: 01bbc4a4b947 ("drm/amdkfd: Add MQD manager for GFX 12.1.0")
>
> Signed-off-by: Vladimir Marioukhine <[email protected]>
>
> ---
>
> drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 10 ++++--
>
> drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12.c      | 69 +++++++++++++++++++++++++++++++++++++++
>
> drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12_1.c    | 69 ++++++++++++++++++++++++++++++++++++++
>
> 3 files changed, 146 insertions(+), 2 deletions(-)
>
>
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>
> index 51ee9c39104b..31bf479212e5 100644
>
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
>
> @@ -794,11 +794,16 @@ static int create_queue_nocpsch(struct device_queue_manager *dqm,
>
>                 goto out_deallocate_doorbell;
>
>         }
>
>
>
> -       if (qd)
>
> +       if (qd) {
>
> +               if (!mqd_mgr->restore_mqd) {
>
> +                       pr_err("amdkfd: restore_mqd not implemented for this GPU\n");
>
> +                       retval = -EINVAL;
>
> +                       goto out_deallocate_doorbell;
>
> +                }
>
>                 mqd_mgr->restore_mqd(mqd_mgr, &q->mqd, q->mqd_mem_obj, &q->gart_mqd_addr,
>
>                                      &q->properties, restore_mqd, restore_ctl_stack,
>
>                                      qd->ctl_stack_size);
>
> -       else
>
> +       } else
>

Kernel coding style requires converting all clauses to using parens.


>                 mqd_mgr->init_mqd(mqd_mgr, &q->mqd, q->mqd_mem_obj,
>
>                                         &q->gart_mqd_addr, &q->properties);
>
>
>
> @@ -2153,11 +2158,16 @@ static int create_queue_cpsch(struct device_queue_manager *dqm, struct queue *q,
>
>         q->properties.is_dbg_wa = qpd->pqm->process->debug_trap_enabled &&
>
>                                   kfd_dbg_has_cwsr_workaround(q->device);
>
>
>
> -       if (qd)
>
> +       if (qd) {
>
> +               if (!mqd_mgr->restore_mqd) {
>
> +                       pr_err("amdkfd: restore_mqd not implemented for this GPU\n");
>
> +                       retval = -EINVAL;
>
> +                       goto out;
>
> +                }
>
>                 mqd_mgr->restore_mqd(mqd_mgr, &q->mqd, q->mqd_mem_obj, &q->gart_mqd_addr,
>
>                                      &q->properties, restore_mqd, restore_ctl_stack,
>
>                                      qd->ctl_stack_size);
>
> -       else
>
> +       } else

Same here.  With those fixed:
Reviewed-by: Alex Deucher <[email protected]>

>
>                 mqd_mgr->init_mqd(mqd_mgr, &q->mqd, q->mqd_mem_obj,
>
>                                         &q->gart_mqd_addr, &q->properties);
>
>
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12.c
>
> index 7c387fa90076..c6a8e99ad069 100644
>
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12.c
>
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12.c
>
> @@ -380,6 +380,75 @@ static int debugfs_show_mqd_sdma(struct seq_file *m, void *data)
>
>
>
> #endif
>
>
>
> +static void checkpoint_mqd(struct mqd_manager *mm, void *mqd,
>
> +                           void *mqd_dst, void *ctl_stack_dst)
>
> +{
>
> +       struct v12_compute_mqd *m;
>
> +
>
> +       m = get_mqd(mqd);
>
> +
>
> +       memcpy(mqd_dst, m, sizeof(struct v12_compute_mqd));
>
> +}
>
> +
>
> +static void restore_mqd(struct mqd_manager *mm, void **mqd,
>
> +                        struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
>
> +                        struct queue_properties *qp, const void *mqd_src,
>
> +                        const void *ctl_stack_src, const u32 ctl_stack_size)
>
> +{
>
> +       uint64_t addr;
>
> +       struct v12_compute_mqd *m;
>
> +
>
> +       m = (struct v12_compute_mqd *) mqd_mem_obj->cpu_ptr;
>
> +       addr = mqd_mem_obj->gpu_addr;
>
> +
>
> +       memcpy(m, mqd_src, sizeof(*m));
>
> +
>
> +       *mqd = m;
>
> +       if (gart_addr)
>
> +               *gart_addr = addr;
>
> +
>
> +       m->cp_hqd_pq_doorbell_control =
>
> +               qp->doorbell_off << CP_HQD_PQ_DOORBELL_CONTROL__DOORBELL_OFFSET__SHIFT;
>
> +       pr_debug("cp_hqd_pq_doorbell_control 0x%x\n", m->cp_hqd_pq_doorbell_control);
>
> +
>
> +       qp->is_active = 0;
>
> +}
>
> +
>
> +static void checkpoint_mqd_sdma(struct mqd_manager *mm, void *mqd,
>
> +                                void *mqd_dst, void *ctl_stack_dst)
>
> +{
>
> +       struct v12_sdma_mqd *m;
>
> +
>
> +       m = get_sdma_mqd(mqd);
>
> +
>
> +       memcpy(mqd_dst, m, sizeof(struct v12_sdma_mqd));
>
> +}
>
> +
>
> +static void restore_mqd_sdma(struct mqd_manager *mm, void **mqd,
>
> +                             struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
>
> +                             struct queue_properties *qp,
>
> +                             const void *mqd_src,
>
> +                             const void *ctl_stack_src,
>
> +                             const u32 ctl_stack_size)
>
> +{
>
> +       uint64_t addr;
>
> +       struct v12_sdma_mqd *m;
>
> +
>
> +       m = (struct v12_sdma_mqd *) mqd_mem_obj->cpu_ptr;
>
> +       addr = mqd_mem_obj->gpu_addr;
>
> +
>
> +       memcpy(m, mqd_src, sizeof(*m));
>
> +
>
> +       m->sdmax_rlcx_doorbell_offset =
>
> +               qp->doorbell_off << SDMA0_QUEUE0_DOORBELL_OFFSET__OFFSET__SHIFT;
>
> +
>
> +       *mqd = m;
>
> +       if (gart_addr)
>
> +               *gart_addr = addr;
>
> +
>
> +       qp->is_active = 0;
>
> +}
>
> +
>
> struct mqd_manager *mqd_manager_init_v12(enum KFD_MQD_TYPE type,
>
>                 struct kfd_node *dev)
>
> {
>
> @@ -407,6 +476,8 @@ struct mqd_manager *mqd_manager_init_v12(enum KFD_MQD_TYPE type,
>
>                 mqd->mqd_size = sizeof(struct v12_compute_mqd);
>
>                 mqd->get_wave_state = get_wave_state;
>
>                 mqd->mqd_stride = kfd_mqd_stride;
>
> +               mqd->checkpoint_mqd = checkpoint_mqd;
>
> +               mqd->restore_mqd = restore_mqd;
>
> #if defined(CONFIG_DEBUG_FS)
>
>                 mqd->debugfs_show_mqd = debugfs_show_mqd;
>
> #endif
>
> @@ -453,6 +524,8 @@ struct mqd_manager *mqd_manager_init_v12(enum KFD_MQD_TYPE type,
>
>                 mqd->is_occupied = kfd_is_occupied_sdma;
>
>                 mqd->mqd_size = sizeof(struct v12_sdma_mqd);
>
>                 mqd->mqd_stride = kfd_mqd_stride;
>
> +               mqd->checkpoint_mqd = checkpoint_mqd_sdma;
>
> +               mqd->restore_mqd = restore_mqd_sdma;
>
> #if defined(CONFIG_DEBUG_FS)
>
>                 mqd->debugfs_show_mqd = debugfs_show_mqd_sdma;
>
> #endif
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12_1.c b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12_1.c
>
> index 431a940f91f3..7ee98ef084ea 100644
>
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12_1.c
>
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_mqd_manager_v12_1.c
>
> @@ -641,6 +641,75 @@ static int debugfs_show_mqd_sdma(struct seq_file *m, void *data)
>
>
>
> #endif
>
>
>
> +static void checkpoint_mqd_v12_1(struct mqd_manager *mm, void *mqd,
>
> +                                 void *mqd_dst, void *ctl_stack_dst)
>
> +{
>
> +       struct v12_1_compute_mqd *m;
>
> +
>
> +       m = get_mqd(mqd);
>
> +
>
> +       memcpy(mqd_dst, m, sizeof(struct v12_1_compute_mqd));
>
> +}
>
> +
>
> +static void restore_mqd_v12_1(struct mqd_manager *mm, void **mqd,
>
> +                              struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
>
> +                              struct queue_properties *qp, const void *mqd_src,
>
> +                              const void *ctl_stack_src, const u32 ctl_stack_size)
>
> +{
>
> +       uint64_t addr;
>
> +       struct v12_1_compute_mqd *m;
>
> +
>
> +       m = (struct v12_1_compute_mqd *) mqd_mem_obj->cpu_ptr;
>
> +       addr = mqd_mem_obj->gpu_addr;
>
> +
>
> +       memcpy(m, mqd_src, sizeof(*m));
>
> +
>
> +       *mqd = m;
>
> +       if (gart_addr)
>
> +               *gart_addr = addr;
>
> +
>
> +       m->cp_hqd_pq_doorbell_control =
>
> +               qp->doorbell_off << CP_HQD_PQ_DOORBELL_CONTROL__DOORBELL_OFFSET__SHIFT;
>
> +       pr_debug("cp_hqd_pq_doorbell_control 0x%x\n", m->cp_hqd_pq_doorbell_control);
>
> +
>
> +       qp->is_active = 0;
>
> +}
>
> +
>
> +static void checkpoint_mqd_sdma_v12_1(struct mqd_manager *mm, void *mqd,
>
> +                                      void *mqd_dst, void *ctl_stack_dst)
>
> +{
>
> +       struct v12_sdma_mqd *m;
>
> +
>
> +       m = get_sdma_mqd(mqd);
>
> +
>
> +       memcpy(mqd_dst, m, sizeof(struct v12_sdma_mqd));
>
> +}
>
> +
>
> +static void restore_mqd_sdma_v12_1(struct mqd_manager *mm, void **mqd,
>
> +                                   struct kfd_mem_obj *mqd_mem_obj, uint64_t *gart_addr,
>
> +                                   struct queue_properties *qp,
>
> +                                   const void *mqd_src,
>
> +                                   const void *ctl_stack_src,
>
> +                                   const u32 ctl_stack_size)
>
> +{
>
> +       uint64_t addr;
>
> +       struct v12_sdma_mqd *m;
>
> +
>
> +       m = (struct v12_sdma_mqd *) mqd_mem_obj->cpu_ptr;
>
> +       addr = mqd_mem_obj->gpu_addr;
>
> +
>
> +       memcpy(m, mqd_src, sizeof(*m));
>
> +
>
> +       m->sdmax_rlcx_doorbell_offset =
>
> +               qp->doorbell_off << SDMA0_QUEUE0_DOORBELL_OFFSET__OFFSET__SHIFT;
>
> +
>
> +       *mqd = m;
>
> +       if (gart_addr)
>
> +               *gart_addr = addr;
>
> +
>
> +       qp->is_active = 0;
>
> +}
>
> +
>
> struct mqd_manager *mqd_manager_init_v12_1(enum KFD_MQD_TYPE type,
>
>                 struct kfd_node *dev)
>
> {
>
> @@ -668,6 +737,8 @@ struct mqd_manager *mqd_manager_init_v12_1(enum KFD_MQD_TYPE type,
>
>                 mqd->mqd_size = sizeof(struct v12_1_compute_mqd);
>
>                 mqd->get_wave_state = get_wave_state_v12_1;
>
>                 mqd->mqd_stride = kfd_mqd_stride;
>
> +               mqd->checkpoint_mqd = checkpoint_mqd_v12_1;
>
> +               mqd->restore_mqd = restore_mqd_v12_1;
>
> #if defined(CONFIG_DEBUG_FS)
>
>                 mqd->debugfs_show_mqd = debugfs_show_mqd;
>
> #endif
>
> @@ -714,6 +785,8 @@ struct mqd_manager *mqd_manager_init_v12_1(enum KFD_MQD_TYPE type,
>
>                 mqd->is_occupied = kfd_is_occupied_sdma;
>
>                 mqd->mqd_size = sizeof(struct v12_sdma_mqd);
>
>                 mqd->mqd_stride = kfd_mqd_stride;
>
> +               mqd->checkpoint_mqd = checkpoint_mqd_sdma_v12_1;
>
> +               mqd->restore_mqd = restore_mqd_sdma_v12_1;
>
> #if defined(CONFIG_DEBUG_FS)
>
>                 mqd->debugfs_show_mqd = debugfs_show_mqd_sdma;
>
> #endif