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

Ming Qian <[email protected]>
Newsgroups dev.linux.lists.imx,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-media,org.kernel.vger.stable
Message-ID <AM9PR04MB857016483639B476A396CE45E7CC2@AM9PR04MB8570.eurprd04.prod.outlook.com>

>-----Original Message-----
>From: Fan Wu <[email protected]>
>Sent: Wednesday, June 24, 2026 8:32 AM
>To: Mirela Rabulea <[email protected]>; [email protected]
>Cc: [email protected]; [email protected]; [email protected];
>[email protected]; [email protected]; [email protected];
>[email protected]; [email protected];
>[email protected]; Fan Wu <[email protected]>
>Subject: [EXT] [PATCH] media: imx-jpeg: finish the job on device_run() error
>paths
>
>[You don't often get email from [email protected]. 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: [email protected]
>Signed-off-by: Fan Wu <[email protected]>

Reviewed-by: Ming Qian <[email protected]>

>---
> .../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
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.