RE: [EXT] [PATCH] media: imx-jpeg: finish the job on device_run() error paths

[Ming Qian]

>-----Original Message-----
>From: Fan Wu <fanwu01@xxxxxxxxxx>
>Sent: Wednesday, June 24, 2026 8:32 AM
>To: Mirela Rabulea <mirela.rabulea@xxxxxxx>; mchehab@xxxxxxxxxx
>Cc: shawnguo@xxxxxxxxxx; s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx;
>festevam@xxxxxxxxx; imx@xxxxxxxxxxxxxxx; linux-media@xxxxxxxxxxxxxxx;
>linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
>stable@xxxxxxxxxxxxxxx; Fan Wu <fanwu01@xxxxxxxxxx>
>Subject: [EXT] [PATCH] media: imx-jpeg: finish the job on device_run() error
>paths
>
>[You don't often get email from fanwu01@xxxxxxxxxx. Learn why this is
>important at https://aka.ms/LearnAboutSenderIdentification ]
>
>Caution: This is an external email. Please take care when clicking links or
>opening attachments. When in doubt, report the message using the 'Report
>this email' button
>
>
>mxc_jpeg_device_run() returns early through a shared "end" label on several
>error paths (no free slot, mxc_jpeg_alloc_slot_data() failure, or missing buffers
>or queue data), and none of them calls v4l2_m2m_job_finish().
>Since the delayed work is queued only after the hardware is started, those
>paths neither finish the job directly nor queue timeout work that could finish it
>later. The job is left with TRANS_RUNNING set, so the
>wait_event() in v4l2_m2m_cancel_job() (reached from
>v4l2_m2m_ctx_release() at close) waits indefinitely and the close hangs.
>
>mxc_jpeg_alloc_slot_data() uses dma_alloc_coherent(), so the failure path is
>reachable under memory pressure.
>
>Return the src/dst buffers with VB2_BUF_STATE_ERROR and call
>v4l2_m2m_job_finish() on those paths: paths that have buffers use a
>"buf_finish" label; the no-buffer path uses "job_finish" directly. This mirrors
>the existing jpeg_parse_error path.
>
>This bug was found by static analysis.
>
>Fixes: 2db16c6ed72c ("media: imx-jpeg: Add V4L2 driver for i.MX8 JPEG
>Encoder/Decoder")
>Cc: stable@xxxxxxxxxxxxxxx
>Signed-off-by: Fan Wu <fanwu01@xxxxxxxxxx>

Reviewed-by: Ming Qian <ming.qian@xxxxxxxxxxx>

>---
> .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 21 +++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>index 9e4a813489c0..6b224a19f40e 100644
>--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>@@ -1525,15 +1525,15 @@ static void mxc_jpeg_device_run(void *priv)
>        dst_buf = v4l2_m2m_next_dst_buf(ctx->fh.m2m_ctx);
>        if (!src_buf || !dst_buf) {
>                dev_err(dev, "Null src or dst buf\n");
>-               goto end;
>+               goto job_finish;
>        }
>
>        q_data_cap = mxc_jpeg_get_q_data(ctx,
>V4L2_BUF_TYPE_VIDEO_CAPTURE);
>        if (!q_data_cap)
>-               goto end;
>+               goto buf_finish;
>        q_data_out = mxc_jpeg_get_q_data(ctx,
>V4L2_BUF_TYPE_VIDEO_OUTPUT);
>        if (!q_data_out)
>-               goto end;
>+               goto buf_finish;
>        src_buf->sequence = q_data_out->sequence++;
>        dst_buf->sequence = q_data_cap->sequence++;
>
>@@ -1571,11 +1571,11 @@ static void mxc_jpeg_device_run(void *priv)
>        ctx->slot = mxc_get_free_slot(&jpeg->slot_data);
>        if (ctx->slot < 0) {
>                dev_err(dev, "No more free slots\n");
>-               goto end;
>+               goto buf_finish;
>        }
>        if (!mxc_jpeg_alloc_slot_data(jpeg)) {
>                dev_err(dev, "Cannot allocate slot data\n");
>-               goto end;
>+               goto buf_finish;
>        }
>
>        mxc_jpeg_enable_slot(reg, ctx->slot); @@ -1597,8 +1597,17 @@ static
>void mxc_jpeg_device_run(void *priv)
>                mxc_jpeg_dec_mode_go(dev, reg);
>        }
>        schedule_delayed_work(&ctx->task_timer, msecs_to_jiffies(hw_timeout));
>-end:
>+
>        spin_unlock_irqrestore(&ctx->mxc_jpeg->hw_lock, flags);
>+       return;
>+buf_finish:
>+       v4l2_m2m_src_buf_remove(ctx->fh.m2m_ctx);
>+       v4l2_m2m_dst_buf_remove(ctx->fh.m2m_ctx);
>+       v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_ERROR);
>+       v4l2_m2m_buf_done(dst_buf, VB2_BUF_STATE_ERROR);
>+job_finish:
>+       spin_unlock_irqrestore(&ctx->mxc_jpeg->hw_lock, flags);
>+       v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> }
>
> static int mxc_jpeg_decoder_cmd(struct file *file, void *priv,
>--
>2.34.1
>