drm/sched: Fix drm_sched_fence_free() so it can be passed an uninitialized fence

drm_sched_job_cleanup() will pass an uninitialized fence to
drm_sched_fence_free(), which will cause to_drm_sched_fence() to return
a NULL fence object, causing a NULL pointer deref when this NULL object
is passed to kmem_cache_free().

Let's create a new drm_sched_fence_free() function that takes a
drm_sched_fence pointer and suffix the old function with _rcu. While at
it, complain if drm_sched_fence_free() is passed an initialized fence
or if drm_sched_fence_free_rcu() is passed an uninitialized fence.

Fixes: dbe48d030b28 ("drm/sched: Split drm_sched_job_init")
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Link: https://patchwork.freedesktop.org/patch/msgid/20210903120554.444101-1-boris.brezillon@collabora.com

diff --git a/drivers/gpu/drm/scheduler/sched_fence.c b/drivers/gpu/drm/scheduler/sched_fence.c
index db3fd1303fc485e5a0459f170d66b6335fe82bc5..7fd869520ef2cd7d56ad20b8fca5913da53a36c2 100644 (file)
--- a/drivers/gpu/drm/scheduler/sched_fence.c
+++ b/drivers/gpu/drm/scheduler/sched_fence.c
@@ -69,19 +69,28 @@ static const char *drm_sched_fence_get_timeline_name(struct dma_fence *f)
        return (const char *)fence->sched->name;
 }
 
+static void drm_sched_fence_free_rcu(struct rcu_head *rcu)
+{
+       struct dma_fence *f = container_of(rcu, struct dma_fence, rcu);
+       struct drm_sched_fence *fence = to_drm_sched_fence(f);
+
+       if (!WARN_ON_ONCE(!fence))
+               kmem_cache_free(sched_fence_slab, fence);
+}
+
 /**
- * drm_sched_fence_free - free up the fence memory
+ * drm_sched_fence_free - free up an uninitialized fence
  *
- * @rcu: RCU callback head
+ * @fence: fence to free
  *
- * Free up the fence memory after the RCU grace period.
+ * Free up the fence memory. Should only be used if drm_sched_fence_init()
+ * has not been called yet.
  */
-void drm_sched_fence_free(struct rcu_head *rcu)
+void drm_sched_fence_free(struct drm_sched_fence *fence)
 {
-       struct dma_fence *f = container_of(rcu, struct dma_fence, rcu);
-       struct drm_sched_fence *fence = to_drm_sched_fence(f);
-
-       kmem_cache_free(sched_fence_slab, fence);
+       /* This function should not be called if the fence has been initialized. */
+       if (!WARN_ON_ONCE(fence->sched))
+               kmem_cache_free(sched_fence_slab, fence);
 }
 
 /**
@@ -97,7 +106,7 @@ static void drm_sched_fence_release_scheduled(struct dma_fence *f)
        struct drm_sched_fence *fence = to_drm_sched_fence(f);
 
        dma_fence_put(fence->parent);
-       call_rcu(&fence->finished.rcu, drm_sched_fence_free);
+       call_rcu(&fence->finished.rcu, drm_sched_fence_free_rcu);
 }
 
 /**

diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
index fbbd3b03902fa0f427327ce25d1627aa4ca48a37..6987d412a946c031d80aa5af19fc0bcf5eba6177 100644 (file)
--- a/drivers/gpu/drm/scheduler/sched_main.c
+++ b/drivers/gpu/drm/scheduler/sched_main.c
@@ -750,7 +750,7 @@ void drm_sched_job_cleanup(struct drm_sched_job *job)
                dma_fence_put(&job->s_fence->finished);
        } else {
                /* aborted job before committing to run it */
-               drm_sched_fence_free(&job->s_fence->finished.rcu);
+               drm_sched_fence_free(job->s_fence);
        }
 
        job->s_fence = NULL;

diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index 7f77a455722cadd3841767db9965e835caf044e5..f011e4c407f2e453ec29f8a9a08278aeb00d1ad7 100644 (file)
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -509,7 +509,7 @@ struct drm_sched_fence *drm_sched_fence_alloc(
        struct drm_sched_entity *s_entity, void *owner);
 void drm_sched_fence_init(struct drm_sched_fence *fence,
                          struct drm_sched_entity *entity);
-void drm_sched_fence_free(struct rcu_head *rcu);
+void drm_sched_fence_free(struct drm_sched_fence *fence);
 
 void drm_sched_fence_scheduled(struct drm_sched_fence *fence);
 void drm_sched_fence_finished(struct drm_sched_fence *fence);