[PATCH] media: st-hva: fix encoder use-after-free on release

[Sicong Huang]

The st-hva driver encodes frames asynchronously. Its V4L2 mem2mem
.device_run callback, hva_device_run(), queues ctx->run_work to
execute hva_run_work(). For H.264, the worker calls encoder operations
that access the private encoder context in ctx->priv and its DMA
buffers.

During release, hva_release() closes the encoder and clears ctx->enc
before calling v4l2_m2m_ctx_release() to cancel outstanding jobs.
hva_h264_close() frees the private encoder context and its DMA buffers.
However, the V4L2 core does not stop streaming before invoking
hva_release(), so ctx->run_work may still be queued or running.
The release path does not synchronize with the worker: hva_release()
does not take ctx->lock, and hva_run_work() reads ctx->enc before
acquiring that lock. As a result, the worker may access freed encoder
state or dereference NULL through enc->encode() if ctx->enc has
already been cleared.

One possible interleaving is:

  CPU0 (release)                    CPU1 (worker)
                                    hva_run_work()
                                      enc = ctx->enc
                                      hva_h264_encode()
                                        ctx = pctx->priv
  hva_release()
    hva_h264_close()
      devm_kfree(dev, ctx)
    ctx->enc = NULL
                                        task = ctx->task->vaddr /* UAF */
    v4l2_m2m_ctx_release()
      /* Waits for the job too late. */

Here, ctx in the H.264 functions refers to struct hva_h264_ctx.

Move v4l2_m2m_ctx_release() before closing the encoder. It prevents
further scheduling for the instance, removes jobs waiting in the mem2mem
queue, and waits for an active job to finish.

Recheck ctx->enc after v4l2_m2m_ctx_release(), as releasing the vb2
queues may invoke hva_stop_streaming(), which closes the encoder and
clears ctx->enc. This avoids closing the encoder twice and accessing
already-freed resources.

Found by code inspection.
Fixes: 57b2c0628b60 ("[media] st-hva: multi-format video encoder V4L2 driver")
Signed-off-by: Sicong Huang <congei42@xxxxxxx>
---
 drivers/media/platform/st/sti/hva/hva-v4l2.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/st/sti/hva/hva-v4l2.c b/drivers/media/platform/st/sti/hva/hva-v4l2.c
index 33b768774e20..feca81eaeffa 100644
--- a/drivers/media/platform/st/sti/hva/hva-v4l2.c
+++ b/drivers/media/platform/st/sti/hva/hva-v4l2.c
@@ -1229,8 +1229,13 @@ static int hva_release(struct file *file)
 	struct hva_ctx *ctx = file_to_ctx(file);
 	struct hva_dev *hva = ctx_to_hdev(ctx);
 	struct device *dev = ctx_to_dev(ctx);
-	const struct hva_enc *enc = ctx->enc;
+	const struct hva_enc *enc;
+
+	/* Stop encoding before closing the encoder. */
+	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
 
+	/* Queue release may have already closed the encoder. */
+	enc = ctx->enc;
 	if (enc) {
 		dev_dbg(dev, "%s %s encoder closed\n", ctx->name, enc->name);
 		enc->close(ctx);
@@ -1244,8 +1249,6 @@ static int hva_release(struct file *file)
 	/* trace a summary of instance before closing (debug purpose) */
 	hva_dbg_summary(ctx);
 
-	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
-
 	v4l2_ctrl_handler_free(&ctx->ctrl_handler);
 
 	v4l2_fh_del(&ctx->fh, file);

base-commit: 27285b099fbc5cb2ae3aef20f3bbd7b3c88b3005
-- 
2.34.1