dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [bug report] drm/scheduler: rework job destruction
@ 2019-05-22 13:07 Dan Carpenter
  2019-05-22 13:53 ` Grodzovsky, Andrey
  0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2019-05-22 13:07 UTC (permalink / raw)
  To: christian.koenig, Andrey Grodzovsky; +Cc: dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [bug report] drm/scheduler: rework job destruction
  2019-05-22 13:07 [bug report] drm/scheduler: rework job destruction Dan Carpenter
@ 2019-05-22 13:53 ` Grodzovsky, Andrey
  0 siblings, 0 replies; 2+ messages in thread
From: Grodzovsky, Andrey @ 2019-05-22 13:53 UTC (permalink / raw)
  To: Dan Carpenter, Koenig, Christian; +Cc: dri-devel

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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-05-22 13:53 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-22 13:07 [bug report] drm/scheduler: rework job destruction Dan Carpenter
2019-05-22 13:53 ` Grodzovsky, Andrey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).