The run_job() callbacks for BIN, RENDER, TFU and CSD assign the incoming
job to queue->active_job before calling v3d_fence_create(). If
v3d_fence_create() fails, the callback returns NULL without clearing
active_job, leaving a dangling pointer.

Create a failure path in all run_job() callbacks that clears the active
job before returning NULL. The BIN path takes queue->queue_lock around the
clear as it races against v3d_overflow_mem_work(); RENDER, TFU and CSD
paths have no concurrent reader, so the clear is lock-free.

Fixes: a783a09ee76d ("drm/v3d: Refactor job management.")
Reviewed-by: Tvrtko Ursulin <[email protected]>
Signed-off-by: Maíra Canal <[email protected]>
---
 drivers/gpu/drm/v3d/v3d_sched.c | 52 ++++++++++++++++++++++++-----------------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c
index c01fa90def4c..66569b538e4e 100644
--- a/drivers/gpu/drm/v3d/v3d_sched.c
+++ b/drivers/gpu/drm/v3d/v3d_sched.c
@@ -206,12 +206,8 @@ static struct dma_fence *v3d_bin_job_run(struct 
drm_sched_job *sched_job)
        struct dma_fence *fence;
        unsigned long irqflags;
 
-       if (unlikely(job->base.base.s_fence->finished.error)) {
-               spin_lock_irqsave(&queue->queue_lock, irqflags);
-               queue->active_job = NULL;
-               spin_unlock_irqrestore(&queue->queue_lock, irqflags);
-               return NULL;
-       }
+       if (unlikely(job->base.base.s_fence->finished.error))
+               goto out_clean_job;
 
        /* Lock required around bin_job update vs
         * v3d_overflow_mem_work().
@@ -228,7 +224,7 @@ static struct dma_fence *v3d_bin_job_run(struct 
drm_sched_job *sched_job)
 
        fence = v3d_fence_create(v3d, V3D_BIN);
        if (IS_ERR(fence))
-               return NULL;
+               goto out_clean_job;
 
        if (job->base.irq_fence)
                dma_fence_put(job->base.irq_fence);
@@ -256,6 +252,12 @@ static struct dma_fence *v3d_bin_job_run(struct 
drm_sched_job *sched_job)
        V3D_CORE_WRITE(0, V3D_CLE_CT0QEA, job->end);
 
        return fence;
+
+out_clean_job:
+       spin_lock_irqsave(&queue->queue_lock, irqflags);
+       queue->active_job = NULL;
+       spin_unlock_irqrestore(&queue->queue_lock, irqflags);
+       return NULL;
 }
 
 static struct dma_fence *v3d_render_job_run(struct drm_sched_job *sched_job)
@@ -265,10 +267,8 @@ static struct dma_fence *v3d_render_job_run(struct 
drm_sched_job *sched_job)
        struct drm_device *dev = &v3d->drm;
        struct dma_fence *fence;
 
-       if (unlikely(job->base.base.s_fence->finished.error)) {
-               v3d->queue[V3D_RENDER].active_job = NULL;
-               return NULL;
-       }
+       if (unlikely(job->base.base.s_fence->finished.error))
+               goto out_clean_job;
 
        v3d->queue[V3D_RENDER].active_job = &job->base;
 
@@ -282,7 +282,7 @@ static struct dma_fence *v3d_render_job_run(struct 
drm_sched_job *sched_job)
 
        fence = v3d_fence_create(v3d, V3D_RENDER);
        if (IS_ERR(fence))
-               return NULL;
+               goto out_clean_job;
 
        if (job->base.irq_fence)
                dma_fence_put(job->base.irq_fence);
@@ -303,6 +303,10 @@ static struct dma_fence *v3d_render_job_run(struct 
drm_sched_job *sched_job)
        V3D_CORE_WRITE(0, V3D_CLE_CT1QEA, job->end);
 
        return fence;
+
+out_clean_job:
+       v3d->queue[V3D_RENDER].active_job = NULL;
+       return NULL;
 }
 
 static struct dma_fence *
@@ -313,16 +317,14 @@ v3d_tfu_job_run(struct drm_sched_job *sched_job)
        struct drm_device *dev = &v3d->drm;
        struct dma_fence *fence;
 
-       if (unlikely(job->base.base.s_fence->finished.error)) {
-               v3d->queue[V3D_TFU].active_job = NULL;
-               return NULL;
-       }
+       if (unlikely(job->base.base.s_fence->finished.error))
+               goto out_clean_job;
 
        v3d->queue[V3D_TFU].active_job = &job->base;
 
        fence = v3d_fence_create(v3d, V3D_TFU);
        if (IS_ERR(fence))
-               return NULL;
+               goto out_clean_job;
 
        if (job->base.irq_fence)
                dma_fence_put(job->base.irq_fence);
@@ -350,6 +352,10 @@ v3d_tfu_job_run(struct drm_sched_job *sched_job)
        V3D_WRITE(V3D_TFU_ICFG(v3d->ver), job->args.icfg | V3D_TFU_ICFG_IOC);
 
        return fence;
+
+out_clean_job:
+       v3d->queue[V3D_TFU].active_job = NULL;
+       return NULL;
 }
 
 static struct dma_fence *
@@ -361,10 +367,8 @@ v3d_csd_job_run(struct drm_sched_job *sched_job)
        struct dma_fence *fence;
        int i, csd_cfg0_reg;
 
-       if (unlikely(job->base.base.s_fence->finished.error)) {
-               v3d->queue[V3D_CSD].active_job = NULL;
-               return NULL;
-       }
+       if (unlikely(job->base.base.s_fence->finished.error))
+               goto out_clean_job;
 
        v3d->queue[V3D_CSD].active_job = &job->base;
 
@@ -372,7 +376,7 @@ v3d_csd_job_run(struct drm_sched_job *sched_job)
 
        fence = v3d_fence_create(v3d, V3D_CSD);
        if (IS_ERR(fence))
-               return NULL;
+               goto out_clean_job;
 
        if (job->base.irq_fence)
                dma_fence_put(job->base.irq_fence);
@@ -399,6 +403,10 @@ v3d_csd_job_run(struct drm_sched_job *sched_job)
        V3D_CORE_WRITE(0, csd_cfg0_reg, job->args.cfg[0]);
 
        return fence;
+
+out_clean_job:
+       v3d->queue[V3D_CSD].active_job = NULL;
+       return NULL;
 }
 
 static void

-- 
2.54.0

Reply via email to