Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:

Pre-existing issues:
- [High] Pre-existing NULL pointer dereference in `drm_sched_job_arm()` if all 
schedulers in the entity's list are not ready.
--

commit 7804887bbadf221d2ccaad830fc83a57f0e74d93
Author: Philipp Stanner <[email protected]>

drm/sched: Lock drm_sched_rq_pop_entity() externally

As a preparational step for protecting entity->last_scheduled with a
spinlock, this commit modifies drm_sched_entity_pop_job() to lock
drm_sched_rq_pop_entity() externally.

> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c 
> b/drivers/gpu/drm/scheduler/sched_entity.c
> index bf97508a45b9a..274d7a7022987 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -565,7 +565,9 @@ struct drm_sched_job *drm_sched_entity_pop_job(struct 
> drm_sched_entity *entity)
>  
>       spsc_queue_pop(&entity->job_queue);
>  
> +     spin_lock(&entity->lock);
>       drm_sched_rq_pop_entity(entity);
> +     spin_unlock(&entity->lock);

[Severity: High]
This isn't a bug introduced by this patch, but this series leaves a
pre-existing NULL pointer dereference unresolved in the entity runqueue
selection path.

If all schedulers in the entity's list have sched->ready == false (e.g.,
during a GPU hang or reset, as documented in the drm_sched_job_init
comments), drm_sched_pick_best() returns NULL. This causes
drm_sched_entity_select_rq() to assign NULL to entity->rq:

drivers/gpu/drm/scheduler/sched_entity.c:drm_sched_entity_select_rq() {
    ...
    sched = drm_sched_pick_best(entity->sched_list, entity->num_sched_list);
    rq = sched ? sched->sched_rq[entity->rq_priority] : NULL;
    if (rq != entity->rq) {
        drm_sched_rq_remove_entity(entity->rq, entity);
        entity->rq = rq;
    }
    ...
}

Upon returning to drm_sched_job_arm(), the code unconditionally dereferences
the NULL entity->rq:

drivers/gpu/drm/scheduler/sched_main.c:drm_sched_job_arm() {
    ...
    drm_sched_entity_select_rq(entity);
    sched = entity->rq->sched;
    ...
}

Can this lead to a kernel panic (NULL pointer dereference) if userspace
continues to submit jobs via ioctl after a hardware fault?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to