* [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
* [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 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 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
* 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
* 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