[bug report] drm/scheduler: rework job destruction

[Grodzovsky, Andrey]

Thanks for letting know, I will send a fix soon.

Andrey

On 5/22/19 9:07 AM, Dan Carpenter wrote:
> [CAUTION: External Email]
>
> Hello Christian König,
>
> The patch 5918045c4ed4: "drm/scheduler: rework job destruction" from
> Apr 18, 2019, leads to the following static checker warning:
>
>          drivers/gpu/drm/scheduler/sched_main.c:297 drm_sched_job_timedout()
>          error: potential NULL dereference 'job'.
>
> drivers/gpu/drm/scheduler/sched_main.c
>     279  static void drm_sched_job_timedout(struct work_struct *work)
>     280  {
>     281          struct drm_gpu_scheduler *sched;
>     282          struct drm_sched_job *job;
>     283          unsigned long flags;
>     284
>     285          sched = container_of(work, struct drm_gpu_scheduler, work_tdr.work);
>     286          job = list_first_entry_or_null(&sched->ring_mirror_list,
>     287                                         struct drm_sched_job, node);
>     288
>     289          if (job)
>                      ^^^
> We assume that job can be NULL.
>
>     290                  job->sched->ops->timedout_job(job);
>     291
>     292          /*
>     293           * Guilty job did complete and hence needs to be manually removed
>     294           * See drm_sched_stop doc.
>     295           */
>     296          if (sched->free_guilty) {
>
> Originally (last week) this check was "if (list_empty(&job->node))"
> which is obviously problematic if job is NULL.  It's not clear to me
> that this new check ensures that job is non-NULL either.
>
>     297                  job->sched->ops->free_job(job);
>                          ^^^^^
> Dereference.
>
>     298                  sched->free_guilty = false;
>     299          }
>     300
>     301          spin_lock_irqsave(&sched->job_list_lock, flags);
>     302          drm_sched_start_timeout(sched);
>     303          spin_unlock_irqrestore(&sched->job_list_lock, flags);
>     304  }
>
> regards,
> dan carpenter