* [PATCH 0/3] media: cancel timeout delayed work before freeing its owner
@ 2026-08-24 19:34 Shengzhuo Wei
2026-08-24 19:34 ` [PATCH 2/3] media: verisilicon: hantro: cancel watchdog work before m2m release Shengzhuo Wei
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Shengzhuo Wei @ 2026-08-24 19:34 UTC (permalink / raw)
To: Mirela Rabulea, Mauro Carvalho Chehab, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Hans Verkuil, Ming Qian,
Nicolas Dufresne, Benjamin Gaignard, Philipp Zabel,
Ezequiel Garcia, Bin Liu, Matthias Brugger,
AngeloGioacchino Del Regno, irui wang, kyrie wu
Cc: imx, linux-media, linux-arm-kernel, stable, Shengzhuo Wei
Three m2m codec drivers arm a per-job timeout delayed work on the
system workqueue and only cancel it on the job-completion path. If the
hardware never completes the job, the release/remove path frees the
object the timeout callback dereferences (via container_of or through
the m2m device) with the work still pending -- a use-after-free when
the timer expires.
All three are the same missed-twins class as the recent mtk-jpeg
jpeg_work release fix and the host1x timeout-worker fix. In each case
the fix is a single cancel_delayed_work_sync() placed before the object
is freed:
- mxc-jpeg: cancel ctx->task_timer in mxc_jpeg_release() before
kfree(ctx); the timer is otherwise only cancelled in the job IRQ.
- hantro: cancel vpu->watchdog_work in hantro_remove() before
v4l2_m2m_put() frees the m2m device the watchdog dereferences.
- mtk-jpeg: cancel jpeg->job_timeout_work in mtk_jpeg_remove()
before v4l2_m2m_release(); ctx->jpeg_work in the same driver got
the equivalent fix earlier, the device-level work was missed.
Patches are independent of each other.
---
Shengzhuo Wei (3):
media: nxp: imx-jpeg: cancel task_timer before freeing ctx
media: verisilicon: hantro: cancel watchdog work before m2m release
media: mediatek: jpeg: cancel job timeout work before m2m release
drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c | 1 +
drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 2 ++
drivers/media/platform/verisilicon/hantro_drv.c | 1 +
3 files changed, 4 insertions(+)
---
base-commit: 075b74841bd0065a3bda3440873c747938e69b68
change-id: 20260825-media-timeout-work-1cef7720c63b
Best regards,
--
Shengzhuo Wei <me@cherr.cc>
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 2/3] media: verisilicon: hantro: cancel watchdog work before m2m release 2026-08-24 19:34 [PATCH 0/3] media: cancel timeout delayed work before freeing its owner Shengzhuo Wei @ 2026-08-24 19:34 ` Shengzhuo Wei 2026-08-24 19:49 ` sashiko-bot 2026-08-24 19:34 ` [PATCH 3/3] media: mediatek: jpeg: cancel job timeout " Shengzhuo Wei [not found] ` <20260825-media-timeout-work-v1-1-ebfebbeb6c31@cherr.cc> 2 siblings, 1 reply; 8+ messages in thread From: Shengzhuo Wei @ 2026-08-24 19:34 UTC (permalink / raw) To: Mirela Rabulea, Mauro Carvalho Chehab, Frank Li, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Hans Verkuil, Ming Qian, Nicolas Dufresne, Benjamin Gaignard, Philipp Zabel, Ezequiel Garcia, Bin Liu, Matthias Brugger, AngeloGioacchino Del Regno, irui wang, kyrie wu Cc: imx, linux-media, linux-arm-kernel, stable, Shengzhuo Wei Each job arms vpu->watchdog_work, cancelled only by the job-completion path. If a job stalls, hantro_remove() drops the last reference to vpu->m2m_dev via v4l2_m2m_put() with the watchdog still pending, and hantro_watchdog() then dereferences the freed m2m_dev -- a use-after-free. Drain the watchdog before the m2m device is released. Fixes: 775fec69008d ("media: add Rockchip VPU JPEG encoder driver") Cc: stable@vger.kernel.org Signed-off-by: Shengzhuo Wei <me@cherr.cc> Assisted-by: GLM:5.3 --- drivers/media/platform/verisilicon/hantro_drv.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c index 2e81877f640fb429b18d2f4fd64bc0c392cf600e..1b04b98371a48184f390a13791243fd00d6b80ef 100644 --- a/drivers/media/platform/verisilicon/hantro_drv.c +++ b/drivers/media/platform/verisilicon/hantro_drv.c @@ -1284,6 +1284,7 @@ static void hantro_remove(struct platform_device *pdev) hantro_remove_dec_func(vpu); hantro_remove_enc_func(vpu); media_device_cleanup(&vpu->mdev); + cancel_delayed_work_sync(&vpu->watchdog_work); v4l2_m2m_put(vpu->m2m_dev); v4l2_device_unregister(&vpu->v4l2_dev); clk_bulk_unprepare(vpu->variant->num_clocks, vpu->clocks); -- 2.47.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] media: verisilicon: hantro: cancel watchdog work before m2m release 2026-08-24 19:34 ` [PATCH 2/3] media: verisilicon: hantro: cancel watchdog work before m2m release Shengzhuo Wei @ 2026-08-24 19:49 ` sashiko-bot 0 siblings, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-08-24 19:49 UTC (permalink / raw) To: Shengzhuo Wei; +Cc: imx, media-ci, Frank.Li Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: Pre-existing issues: - [High] Incomplete Fix: Pending V4L2 m2m jobs can re-arm the watchdog after cancellation, causing a Use-After-Free. - [High] Hardware interrupt is not masked or freed before freeing `m2m_dev`, leading to a Use-After-Free and Bus Error. - [High] Incorrect object lifetime management causes devres and `v4l2_m2m_put()` to unconditionally free `m2m_dev` and `vpu` while open file descriptors still exist. - [High] Missing cancellation of `watchdog_work` in the `hantro_probe()` error paths. -- --- Patch [2]: [PATCH 2/3] media: verisilicon: hantro: cancel watchdog work before m2m release --- commit 36d662041efb45f2c47f99503f908090f9994b94 Author: Shengzhuo Wei <me@cherr.cc> Subject: media: verisilicon: hantro: cancel watchdog work before m2m release This commit adds a cancel_delayed_work_sync() call to hantro_remove() to drain pending watchdog work before releasing the m2m device. This aims to prevent a use-after-free where the watchdog dereferences the m2m device after hantro_remove() drops the final reference. > diff --git a/drivers/media/platform/verisilicon/hantro_drv.c b/drivers/media/platform/verisilicon/hantro_drv.c > --- a/drivers/media/platform/verisilicon/hantro_drv.c > +++ b/drivers/media/platform/verisilicon/hantro_drv.c > @@ -1284,6 +1284,7 @@ static void hantro_remove(struct platform_device *pdev) > hantro_remove_dec_func(vpu); > hantro_remove_enc_func(vpu); > media_device_cleanup(&vpu->mdev); > + cancel_delayed_work_sync(&vpu->watchdog_work); [Severity: High] This isn't a bug introduced by this patch, but is this fix complete? Because the driver does not abort the m2m job queue or stop the hardware before cancelling the watchdog, an active or pending m2m job can run concurrently with driver removal. If a job is processing, the m2m framework executes it via m2m_dev->job_work, which eventually calls hantro_end_prepare_run(). That function explicitly calls schedule_delayed_work(&ctx->dev->watchdog_work, ...). Could this allow the watchdog to be re-armed immediately after the cancel_delayed_work_sync() call here, leading to the exact use-after-free this patch intends to resolve? > v4l2_m2m_put(vpu->m2m_dev); [Severity: High] This is a pre-existing issue, but does freeing the m2m_dev here leave us vulnerable to an interrupt handler use-after-free? The hardware interrupt is managed by devres and is not explicitly masked or freed before this point. If a hardware interrupt fires during this teardown window, hantro_irq_done() will execute and call v4l2_m2m_get_curr_priv(vpu->m2m_dev), dereferencing the freed m2m_dev. [Severity: High] This is also a pre-existing issue, but is it safe to unconditionally drop the m2m_dev reference here while userspace might still have open file descriptors? The vpu and func structures are allocated with devm_kzalloc(), and the V4L2 device release callback is stubbed out (video_device_release_empty). This means when hantro_remove() returns, devres will immediately free these structures. If userspace later closes an open file descriptor, hantro_release() will call v4l2_m2m_ctx_release(), which then calls v4l2_m2m_cancel_job(). This will access the already freed m2m_dev and vpu. Could this also cause the kernel workqueue subsystem to dereference a freed work_struct if the m2m_dev's embedded job_work is pending on the system workqueue when it is freed? > v4l2_device_unregister(&vpu->v4l2_dev); > clk_bulk_unprepare(vpu->variant->num_clocks, vpu->clocks); Since clk_bulk_unprepare() is called right after dropping the m2m_dev, could an active interrupt handler accessing registers with disabled clocks cause a synchronous bus error? There is another pre-existing issue in the hantro_probe() error path. In hantro_probe(), the video devices are registered and exposed to userspace before the rest of the initialization completes. If a later step in probe fails, the error path tears down the device and allows devres to free the vpu structure: hantro_probe() { ... err_rm_dec_func: hantro_remove_dec_func(vpu); err_rm_enc_func: hantro_remove_enc_func(vpu); err_m2m_rel: media_device_cleanup(&vpu->mdev); v4l2_m2m_put(vpu->m2m_dev); ... } [Severity: High] Since cancel_delayed_work_sync(&vpu->watchdog_work) is missing from this error path, could a job queued by userspace during the brief registration window arm the watchdog, leading to a use-after-free when the pending timer executes on the freed vpu memory? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260825-media-timeout-work-v1-0-ebfebbeb6c31@cherr.cc?part=2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] media: mediatek: jpeg: cancel job timeout work before m2m release 2026-08-24 19:34 [PATCH 0/3] media: cancel timeout delayed work before freeing its owner Shengzhuo Wei 2026-08-24 19:34 ` [PATCH 2/3] media: verisilicon: hantro: cancel watchdog work before m2m release Shengzhuo Wei @ 2026-08-24 19:34 ` Shengzhuo Wei 2026-08-24 19:49 ` sashiko-bot [not found] ` <20260825-media-timeout-work-v1-1-ebfebbeb6c31@cherr.cc> 2 siblings, 1 reply; 8+ messages in thread From: Shengzhuo Wei @ 2026-08-24 19:34 UTC (permalink / raw) To: Mirela Rabulea, Mauro Carvalho Chehab, Frank Li, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Hans Verkuil, Ming Qian, Nicolas Dufresne, Benjamin Gaignard, Philipp Zabel, Ezequiel Garcia, Bin Liu, Matthias Brugger, AngeloGioacchino Del Regno, irui wang, kyrie wu Cc: imx, linux-media, linux-arm-kernel, stable, Shengzhuo Wei Each job arms jpeg->job_timeout_work, cancelled only by the IRQ completion path. If a job stalls, mtk_jpeg_remove() frees jpeg->m2m_dev via v4l2_m2m_release() with the timeout still pending, and mtk_jpeg_job_timeout_work() then dereferences the freed m2m_dev and the stale ctx it returns -- a use-after-free. Drain the work before the m2m device is released. The device-level job_timeout_work was missed by the earlier fix for ctx->jpeg_work. Fixes: 5fb1c2361e56 ("mtk-jpegenc: add jpeg encode worker interface") Fixes: dedc21500334 ("media: mtk-jpegdec: add jpeg decode worker interface") Cc: stable@vger.kernel.org Signed-off-by: Shengzhuo Wei <me@cherr.cc> Assisted-by: GLM:5.3 --- drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c index d147ec48308110ae8520662e182dc0445447d8d0..f0fe14e83d515f5011cbfe051abbf565568013d0 100644 --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c @@ -1405,6 +1405,7 @@ static void mtk_jpeg_remove(struct platform_device *pdev) pm_runtime_disable(&pdev->dev); video_unregister_device(jpeg->vdev); + cancel_delayed_work_sync(&jpeg->job_timeout_work); v4l2_m2m_release(jpeg->m2m_dev); v4l2_device_unregister(&jpeg->v4l2_dev); } -- 2.47.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] media: mediatek: jpeg: cancel job timeout work before m2m release 2026-08-24 19:34 ` [PATCH 3/3] media: mediatek: jpeg: cancel job timeout " Shengzhuo Wei @ 2026-08-24 19:49 ` sashiko-bot 0 siblings, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-08-24 19:49 UTC (permalink / raw) To: Shengzhuo Wei; +Cc: imx, media-ci, Frank.Li Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider: New issues: - [High] Cancelling the job timeout work in remove leaves stalled jobs in TRANS_RUNNING state forever, causing a deadlock on file close. - [Medium] Calling cancel_delayed_work_sync() on an uninitialized delayed_work struct for multi-core variants causes a kernel WARNING. Pre-existing issues: - [High] Releasing m2m_dev in mtk_jpeg_remove() causes a Use-After-Free when open file descriptors are eventually closed. - [High] Missing cleanup of sub-device job_timeout_work in multi-core mode causes a use-after-free. - [High] Pending multi-core workers in jpeg->workqueue cause a use-after-free during devres cleanup. - [High] Missing hardware stop and IRQ disable before freeing m2m_dev causes UAF in IRQ handler. - [High] A race condition between the hardware IRQ handler and the timeout worker can result in a NULL pointer dereference. - [Medium] Missing PM usage counter balance on cancellation and error paths. -- --- Patch [3]: [PATCH 3/3] media: mediatek: jpeg: cancel job timeout work before m2m release --- commit 783b89e56f061c0e51e278c73a5f2399a27451fa Author: Shengzhuo Wei <me@cherr.cc> media: mediatek: jpeg: cancel job timeout work before m2m release This patch attempts to fix a use-after-free by cancelling the job timeout work before releasing the m2m device in mtk_jpeg_remove(). > diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c > index d147ec4830811..f0fe14e83d515 100644 > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c > @@ -1405,6 +1405,7 @@ static void mtk_jpeg_remove(struct platform_device *pdev) > > pm_runtime_disable(&pdev->dev); > video_unregister_device(jpeg->vdev); > + cancel_delayed_work_sync(&jpeg->job_timeout_work); [Severity: High] Does cancelling the job timeout work here leave stalled jobs in the TRANS_RUNNING state forever? If a job is currently hanging when the driver is unbound, the timeout work is cancelled and will never execute. Since the hardware is hung, no IRQ will arrive to complete the job, and the driver does not implement the job_abort m2m callback. When the user subsequently closes the file descriptor, v4l2_m2m_ctx_release() invokes v4l2_m2m_cancel_job(), which indefinitely blocks in wait_event() waiting for TRANS_RUNNING to be cleared. Could this create an unkillable process in the D state? [Severity: Medium] Will this unconditionally pass a zero-initialized work struct to the workqueue subsystem for multi-core variants? In mtk_jpeg_probe(), jpeg->job_timeout_work is only initialized via INIT_DELAYED_WORK if jpeg->variant->multi_core is false. For multi-core variants, it remains uninitialized. When cancel_delayed_work_sync() executes on multi-core variants during unbind, could it hit a WARN_ON(!work->func) inside flush_work() and trigger debug object warnings? [Severity: Medium] Does unconditionally cancelling the timeout worker here leak the PM usage counter? When a job starts, it takes a PM reference via pm_runtime_get_sync(). If the job stalls, the timeout work normally balances this reference by calling pm_runtime_put(). By cancelling the worker without issuing a compensating pm_runtime_put(), the usage counter may never be balanced. This also relates to a pre-existing issue in mtk_jpegenc_worker() and mtk_jpegdec_worker(). If pm_runtime_get_sync() fails, the code branches to the error path without calling pm_runtime_put_noidle(). Since pm_runtime_get_sync() increments the counter even on failure, does this also permanently leak the usage counter? > v4l2_m2m_release(jpeg->m2m_dev); [Severity: High] This is a pre-existing issue, but does releasing m2m_dev here cause a use-after-free when open file descriptors are eventually closed? video_unregister_device() does not forcefully close existing open file descriptors. If a user-space process holds an FD open during device unbind, jpeg->m2m_dev is freed while ctx->fh.m2m_ctx remains active. When the user eventually closes the FD, mtk_jpeg_release() accesses the freed jpeg struct and calls v4l2_m2m_ctx_release(). This internally accesses the freed m2m_dev (e.g., locking m2m_dev->job_spinlock in v4l2_m2m_cancel_job()), which seems reachable by unprivileged users simply keeping the FD open. [Severity: High] This is a pre-existing issue, but does freeing m2m_dev here race with pending multi-core workers in jpeg->workqueue? Active multi-core jobs are executed on a devm-managed workqueue (jpeg->workqueue). This workqueue is destroyed via devres, which flushes pending work after mtk_jpeg_remove() has completed and freed jpeg->m2m_dev. When the workqueue is flushed during devres destruction, delayed workers like mtk_jpegenc_worker() are forced to execute. Do they dereference the already freed jpeg->m2m_dev via v4l2_m2m_next_src_buf() and v4l2_m2m_job_finish(), resulting in a use-after-free? [Severity: High] This is a pre-existing issue, but is m2m_dev released here while the IRQ is still active and hardware might still be running? Because the interrupt was registered using devm_request_irq(), it remains fully active during and after mtk_jpeg_remove() executes. If a hardware job completes shortly after v4l2_m2m_release(), the IRQ handler mtk_jpeg_enc_irq() will execute and call mtk_jpeg_enc_done(). Does this unconditionally dereference the already freed jpeg->m2m_dev in the interrupt handler? > v4l2_device_unregister(&jpeg->v4l2_dev); > } [Severity: High] This is a pre-existing issue, but does the lack of a remove callback for sub-devices cause a use-after-free for their timeout works? Multi-core variants use timeout works embedded in component hardware devices, such as dev->job_timeout_work in mtk_jpeg_dec_hw.c. However, component drivers like mtk_jpegdec_hw_driver lack a remove callback. When the master device unbinds, devres frees the sub-devices' memory without cancelling their timeout works. If a multi-core job is stalled during removal, will the uncancelled component timeout work eventually execute on freed memory? [Severity: High] This is a pre-existing issue, but is there a race condition between the hardware IRQ handler and the timeout worker that can result in a NULL pointer dereference? In mtk_jpeg_job_timeout_work(), buffers are removed via v4l2_m2m_src_buf_remove() and passed directly to v4l2_m2m_buf_done(). The IRQ handlers, like mtk_jpeg_enc_irq(), call cancel_delayed_work(), which is asynchronous and returns immediately if the timeout worker is already running. The IRQ handler then proceeds to remove buffers from the same context. If the timeout worker and IRQ handler execute concurrently on different CPUs, they will both pop from the queue. One will receive a valid buffer and the other will receive NULL. Since the one receiving NULL passes it to v4l2_m2m_buf_done(), which directly dereferences buf->vb2_buf without a NULL check, could this cause a kernel panic? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260825-media-timeout-work-v1-0-ebfebbeb6c31@cherr.cc?part=3 ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <20260825-media-timeout-work-v1-1-ebfebbeb6c31@cherr.cc>]
* Re: [PATCH 1/3] media: nxp: imx-jpeg: cancel task_timer before freeing ctx [not found] ` <20260825-media-timeout-work-v1-1-ebfebbeb6c31@cherr.cc> @ 2026-08-24 19:48 ` sashiko-bot 2026-08-25 1:56 ` Ming Qian(OSS) 1 sibling, 0 replies; 8+ messages in thread From: sashiko-bot @ 2026-08-24 19:48 UTC (permalink / raw) To: Shengzhuo Wei; +Cc: Frank.Li, imx, media-ci Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: New issues: - [High] Canceling the timeout timer before releasing the V4L2 mem2mem context causes an uninterruptible kernel hang if the hardware is hung, and fails to fully prevent the original Use-After-Free. Pre-existing issues: - [High] The timeout handler checks the shared device-level slot state rather than context-specific state, leading to a race condition that causes a NULL pointer dereference. - [High] Error paths in mxc_jpeg_device_run() skip calling v4l2_m2m_job_finish(), stalling the M2M queue and causing a deadlock on release. -- --- Patch [1]: [PATCH 1/3] media: nxp: imx-jpeg: cancel task_timer before freeing ctx --- commit 80a788b1c14931407edfcc537272f62c4cc6d308 Author: Shengzhuo Wei <me@cherr.cc> media: nxp: imx-jpeg: cancel task_timer before freeing ctx This commit attempts to fix a use-after-free issue by canceling the ctx->task_timer before the ctx is torn down in mxc_jpeg_release(). The timer is canceled before acquiring the mxc_jpeg->lock to avoid deadlocks with workers. > diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c > index 725e941528848..fbb64a1ecb518 100644 > --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c > +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c > @@ -2796,6 +2796,8 @@ static int mxc_jpeg_release(struct file *file) > struct mxc_jpeg_ctx *ctx = mxc_jpeg_file_to_ctx(file); > struct device *dev = mxc_jpeg->dev; > > + cancel_delayed_work_sync(&ctx->task_timer); > + [Severity: High] Does canceling the timeout timer before releasing the mem2mem context create a potential deadlock and leave a race window? If the hardware has hung, the TRANS_RUNNING flag remains set. When v4l2_m2m_ctx_release() is called, v4l2_m2m_cancel_job() will wait indefinitely for the running flag to clear because this driver lacks a job_abort callback. Since the timeout timer was already canceled, it will never fire to clear the flag, causing the close() operation to hang in an uninterruptible state. Additionally, because the V4L2 M2M queues are not paused during the sync cancel, could a concurrent worker thread dequeue a pending job and call mxc_jpeg_device_run(), re-arming the timer after the cancel but before the context release? This would re-introduce the use-after-free this patch seeks to fix. > mutex_lock(&mxc_jpeg->lock); > if (mxc_jpeg->mode == MXC_JPEG_DECODE) > dev_dbg(dev, "Release JPEG decoder instance on slot %d.", [ ... ] > v4l2_ctrl_handler_free(&ctx->ctrl_handler); > v4l2_m2m_ctx_release(ctx->fh.m2m_ctx); > v4l2_fh_del(&ctx->fh, file); [Severity: High] This isn't a bug introduced by this patch, but does the timeout handler check the wrong state flag? In mxc_jpeg_device_run_timeout(): spin_lock_irqsave(&ctx->mxc_jpeg->hw_lock, flags); if (ctx->mxc_jpeg->slot_data.used) { ... mxc_jpeg_job_finish(ctx, VB2_BUF_STATE_ERROR, true); If mxc_jpeg_dec_irq() completes its job, sets jpeg->slot_data.used = false, unlocks hw_lock, and schedules a new job for a different context, jpeg->slot_data.used could be set back to true before the timeout handler acquires the lock. When the timeout handler acquires the lock, it checks this shared device-level flag instead of its own context state. Will it incorrectly assume its own job timed out and call mxc_jpeg_job_finish() with NULL buffers (since they were already removed by the IRQ handler), leading to a NULL pointer dereference in v4l2_m2m_buf_done()? [Severity: High] This is a pre-existing issue, but do the error paths in mxc_jpeg_device_run() leak the running state? In mxc_jpeg_device_run(): if (!mxc_jpeg_alloc_slot_data(jpeg)) { dev_err(dev, "Cannot allocate slot data\n"); goto end; } ... end: spin_unlock_irqrestore(&ctx->mxc_jpeg->hw_lock, flags); If mxc_jpeg_alloc_slot_data() fails, the function jumps to the end label and returns without calling v4l2_m2m_job_finish() and without arming the timeout timer. Does this leave the job in the TRANS_RUNNING state? If so, when mxc_jpeg_release() is later called, v4l2_m2m_cancel_job() will wait indefinitely for the running flag to clear. -- Sashiko AI review · https://sashiko.dev/#/patchset/20260825-media-timeout-work-v1-0-ebfebbeb6c31@cherr.cc?part=1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] media: nxp: imx-jpeg: cancel task_timer before freeing ctx [not found] ` <20260825-media-timeout-work-v1-1-ebfebbeb6c31@cherr.cc> 2026-08-24 19:48 ` [PATCH 1/3] media: nxp: imx-jpeg: cancel task_timer before freeing ctx sashiko-bot @ 2026-08-25 1:56 ` Ming Qian(OSS) 2026-08-25 3:39 ` Shengzhuo Wei 1 sibling, 1 reply; 8+ messages in thread From: Ming Qian(OSS) @ 2026-08-25 1:56 UTC (permalink / raw) To: Shengzhuo Wei Cc: Mirela Rabulea, Mauro Carvalho Chehab, Frank Li, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Hans Verkuil, Ming Qian, Nicolas Dufresne, Benjamin Gaignard, Philipp Zabel, Ezequiel Garcia, Bin Liu, Matthias Brugger, AngeloGioacchino Del Regno, irui wang, kyrie wu, imx, linux-media, linux-arm-kernel, stable On Tue, Aug 25, 2026 at 03:34:30AM +0800, Shengzhuo Wei wrote: > [You don't often get email from me@cherr.cc. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > Hi Shengzhuo, Thanks for the patch. This use-after-free has already been fixed by Fan Wu: https://lore.kernel.org/lkml/20260623103031.3051-1-fanwu01@zju.edu.cn/ [PATCH] media: imx-jpeg: cancel timeout worker when streaming stops Regards, Ming > mxc_jpeg_device_run() arms ctx->task_timer for each job; the only place > it is cancelled is the job-completion IRQ handler. If the hardware > never completes the job, mxc_jpeg_release() frees ctx with the timer > still pending, and mxc_jpeg_device_run_timeout() then dereferences the > freed ctx -- a use-after-free. > > Cancel the timer before the ctx is torn down, before taking > mxc_jpeg->lock so the cancel never waits on a worker that needs the > mutex. > > Fixes: cfed9632ca8e ("media: imx-jpeg: Add a timeout mechanism for each frame") > Cc: stable@vger.kernel.org > Signed-off-by: Shengzhuo Wei <me@cherr.cc> > Assisted-by: GLM:5.3 > --- > 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 725e941528848e8f224fe6a96ba7f746fc45ed63..fbb64a1ecb5189d2d7b953b99dcd7bcb54e6e20e 100644 > --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c > +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c > @@ -2796,6 +2796,8 @@ static int mxc_jpeg_release(struct file *file) > struct mxc_jpeg_ctx *ctx = mxc_jpeg_file_to_ctx(file); > struct device *dev = mxc_jpeg->dev; > > + cancel_delayed_work_sync(&ctx->task_timer); > + > mutex_lock(&mxc_jpeg->lock); > if (mxc_jpeg->mode == MXC_JPEG_DECODE) > dev_dbg(dev, "Release JPEG decoder instance on slot %d.", > > -- > 2.47.3 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] media: nxp: imx-jpeg: cancel task_timer before freeing ctx 2026-08-25 1:56 ` Ming Qian(OSS) @ 2026-08-25 3:39 ` Shengzhuo Wei 0 siblings, 0 replies; 8+ messages in thread From: Shengzhuo Wei @ 2026-08-25 3:39 UTC (permalink / raw) To: Ming Qian(OSS) Cc: Shengzhuo Wei, Mirela Rabulea, Mauro Carvalho Chehab, Frank Li, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam, Hans Verkuil, Ming Qian, Nicolas Dufresne, Benjamin Gaignard, Philipp Zabel, Ezequiel Garcia, Bin Liu, Matthias Brugger, AngeloGioacchino Del Regno, irui wang, kyrie wu, imx, linux-media, linux-arm-kernel, stable On 2026-08-25 10:56, Ming Qian(OSS) wrote: > Hi Shengzhuo, > > Thanks for the patch. > > This use-after-free has already been fixed by Fan Wu: > https://lore.kernel.org/lkml/20260623103031.3051-1-fanwu01@zju.edu.cn/ > [PATCH] media: imx-jpeg: cancel timeout worker when streaming stops > > Regards, > Ming > Hi Ming, Thanks for pointing me at Fan Wu's patch -- I had missed it (it is not in the 7.2-rc6 base I worked from, and I failed to check for in-flight patches on the list before sending). His version is also the better one: placing the cancel in mxc_jpeg_stop_streaming() closes the re-arm window that a cancel in mxc_jpeg_release() leaves open, which Sashiko also flagged on my 1/3. I'll drop that patch. The other two patches in the series (hantro watchdog, mtk-jpeg job_timeout_work) fix the same class of bug in different drivers and, as far as I can tell, have no equivalent fix in flight. Sashiko raised one real point on the mtk patch: jpeg->job_timeout_work is only initialized on the single-core path, so the unconditional cancel_delayed_work_sync() in mtk_jpeg_remove() would hit a zeroed work struct (WARN_ON(!work->func)) on multi-core variants. I plan to send a v2 that drops the imx-jpeg patch and guards the mtk cancel accordingly. Do you see anything else in the remaining two patches you'd like changed before I send the v2? Regards, Shengzhuo ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-08-25 3:39 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-24 19:34 [PATCH 0/3] media: cancel timeout delayed work before freeing its owner Shengzhuo Wei
2026-08-24 19:34 ` [PATCH 2/3] media: verisilicon: hantro: cancel watchdog work before m2m release Shengzhuo Wei
2026-08-24 19:49 ` sashiko-bot
2026-08-24 19:34 ` [PATCH 3/3] media: mediatek: jpeg: cancel job timeout " Shengzhuo Wei
2026-08-24 19:49 ` sashiko-bot
[not found] ` <20260825-media-timeout-work-v1-1-ebfebbeb6c31@cherr.cc>
2026-08-24 19:48 ` [PATCH 1/3] media: nxp: imx-jpeg: cancel task_timer before freeing ctx sashiko-bot
2026-08-25 1:56 ` Ming Qian(OSS)
2026-08-25 3:39 ` Shengzhuo Wei
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox