[PATCH] media: st-delta: fix decoder use-after-free on release

[Sicong Huang]

The st-delta driver processes decoding jobs asynchronously. Its V4L2
mem2mem .device_run callback, delta_device_run(), queues ctx->run_work,
which executes delta_run_work() to decode an access unit. For MJPEG,
the worker accesses the decoder-private context stored in ctx->priv
and the IPC buffer shared with the firmware.

During release, delta_release() closes the decoder before calling
v4l2_m2m_ctx_release() to cancel outstanding jobs. For MJPEG,
delta_mjpeg_close() frees the decoder-private context and closes the
IPC channel, releasing its shared buffer if the channel was opened.
However, the V4L2 core does not stop streaming before invoking
delta_release(), so ctx->run_work may still be queued or running.
The delta->lock held by delta_release() does not synchronize with the
worker, which uses ctx->lock. As a result, the worker may access
freed decoder state, causing a use-after-free.

One possible interleaving is:

  CPU0 (release)                    CPU1 (worker)
                                    delta_run_work()
                                      delta_mjpeg_decode()
                                        ctx = to_ctx(pctx)
  delta_release()
    delta_mjpeg_close()
      kfree(ctx)
                                        if (!ctx->header) /* UAF */
    v4l2_m2m_ctx_release()
      /* Waits for the job too late. */

Here, ctx in the MJPEG functions refers to struct delta_mjpeg_ctx.

Move v4l2_m2m_ctx_release() before closing the decoder. It prevents
further scheduling for the instance, removes jobs waiting in the mem2mem
queue, and waits for an active job to finish. Since delta_run_work()
completes its decoder accesses before signalling job completion, the
decoder can be safely closed afterwards.

Found by code inspection.
Fixes: f386509e4959 ("[media] st-delta: STiH4xx multi-format video decoder v4l2 driver")

Signed-off-by: Sicong Huang <congei42@xxxxxxx>
---
 drivers/media/platform/st/sti/delta/delta-v4l2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/st/sti/delta/delta-v4l2.c b/drivers/media/platform/st/sti/delta/delta-v4l2.c
index 7dd27de37b78..586dae169d35 100644
--- a/drivers/media/platform/st/sti/delta/delta-v4l2.c
+++ b/drivers/media/platform/st/sti/delta/delta-v4l2.c
@@ -1699,6 +1699,9 @@ static int delta_release(struct file *file)
 
 	mutex_lock(&delta->lock);
 
+	/* make sure no m2m job runs once the decoder is closed */
+	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
+
 	/* close decoder */
 	call_dec_op(dec, close, ctx);
 
@@ -1708,8 +1711,6 @@ static int delta_release(struct file *file)
 	 */
 	delta_trace_summary(ctx);
 
-	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
-
 	v4l2_fh_del(&ctx->fh, file);
 	v4l2_fh_exit(&ctx->fh);
 

base-commit: 27285b099fbc5cb2ae3aef20f3bbd7b3c88b3005
-- 
2.34.1