RE: [EXT] [PATCH] media: imx-jpeg: cancel timeout worker when streaming stops

>-----Original Message-----
>From: Fan Wu <[email protected]>
>Sent: Tuesday, June 23, 2026 6:31 PM
>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: cancel timeout worker when streaming
>stops
>
>[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
>
>
>Each per-fd context ctx owns a delayed_work (ctx->task_timer, callback
>mxc_jpeg_device_run_timeout) armed via schedule_delayed_work() at the
>end of mxc_jpeg_device_run() to recover a stalled encode/decode job. The
>only existing cancellation is cancel_delayed_work() in the frame-done IRQ
>handler, which de-queues a pending work item but does not wait for a callback
>that has already started, and it only runs when a frame completes.
>
>When the fd is closed while a job is in flight (the frame-done IRQ has not fired
>yet), nothing syncs the worker before mxc_jpeg_release() frees ctx with kfree()
>after v4l2_m2m_ctx_release(). A queued or executing
>mxc_jpeg_device_run_timeout() can then recover ctx through
>container_of(&ctx->task_timer) and dereference it (ctx->mxc_jpeg, slot_data,
>dev_warn) after ctx has been freed.
>
>Cancel the worker from mxc_jpeg_stop_streaming(). The cancel cannot live in
>mxc_jpeg_release(): mxc_jpeg_device_run() arms the timer while holding only
>hw_lock, not the mxc_jpeg->lock mutex that release holds, so a cancel in
>release could still race a concurrent mxc_jpeg_device_run() that re-arms the
>timer afterwards. mxc_jpeg_stop_streaming() instead runs inside
>v4l2_m2m_ctx_release() -> vb2_queue_release(), i.e. after
>v4l2_m2m_cancel_job() has set TRANS_ABORT and waited for any in-flight job
>to finish (so __v4l2_m2m_try_queue() will not queue and
>v4l2_m2m_try_run() will not run any further job for this context, which
>prevents
>mxc_jpeg_device_run() from re-arming the timer) and before the m2m context
>is freed. cancel_delayed_work_sync() removes a pending work item and waits
>for a running callback, so the worker can no longer race with the subsequent
>kfree(). The cancel is placed before the buffer-release loop so a concurrently
>running timeout callback cannot race with it over the same buffers. If the
>frame-done IRQ canceled a still-pending timer, this cancel is a no-op; if the
>timeout callback has already started, it waits for the callback to finish. The
>same mxc_jpeg_stop_streaming() call is also reached from
>VIDIOC_STREAMOFF, which drains the worker early, although STREAMOFF
>itself does not free ctx -- the use-after-free arises only when the fd is later
>closed.
>
>This bug was found by static analysis.
>
>Fixes: cfed9632ca8e ("media: imx-jpeg: Add a timeout mechanism for each
>frame")
>Cc: [email protected]
>Signed-off-by: Fan Wu <[email protected]>

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

>---
> drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>index 9e4a813489c0..d85a9d196269 100644
>--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>@@ -1735,6 +1735,8 @@ static void mxc_jpeg_stop_streaming(struct
>vb2_queue *q)
>
>        dev_dbg(ctx->mxc_jpeg->dev, "Stop streaming ctx=%p", ctx);
>
>+       cancel_delayed_work_sync(&ctx->task_timer);
>+
>        /* Release all active buffers */
>        for (;;) {
>                if (V4L2_TYPE_IS_OUTPUT(q->type))
>--
>2.34.1
>