media: mtk-jpeg: fix use-after-free in release path due to uncancelled work

[PATCH] media: mtk-jpeg: fix use-after-free in release path due to uncancelled work

Posted by Fan Wu 1 month, 1 week ago

The mtk_jpeg_release() function frees the context structure (ctx) without
first cancelling any pending or running work in ctx->jpeg_work. This
creates a race window where the workqueue callback may still be accessing
the context memory after it has been freed.

Race condition:

    CPU 0 (release)                    CPU 1 (workqueue)
    ----------------                   ------------------
    close()
      mtk_jpeg_release()
                                       mtk_jpegenc_worker()
                                         ctx = work->data
                                         // accessing ctx

        kfree(ctx)  // freed!
                                         access ctx  // UAF!

The work is queued via queue_work() during JPEG encode/decode operations
(via mtk_jpeg_device_run). If the device is closed while work is pending
or running, the work handler will access freed memory.

Fix this by calling cancel_work_sync() BEFORE acquiring the mutex. This
ordering is critical: if cancel_work_sync() is called after mutex_lock(),
and the work handler also tries to acquire the same mutex, it would cause
a deadlock.

Note: The open error path does NOT need cancel_work_sync() because
INIT_WORK() only initializes the work structure - it does not schedule
it. Work is only scheduled later during ioctl operations.

Fixes: 5fb1c2361e56 ("mtk-jpegenc: add jpeg encode worker interface")
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 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 c01124a349f6..8c684756d5fc 100644
--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
@@ -1202,6 +1202,7 @@ static int mtk_jpeg_release(struct file *file)
 	struct mtk_jpeg_dev *jpeg = video_drvdata(file);
 	struct mtk_jpeg_ctx *ctx = mtk_jpeg_file_to_ctx(file);
 
+	cancel_work_sync(&ctx->jpeg_work);
 	mutex_lock(&jpeg->lock);
 	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
 	v4l2_ctrl_handler_free(&ctx->ctrl_hdl);
-- 
2.34.1

Re: [PATCH] media: mtk-jpeg: fix use-after-free in release path due to uncancelled work

Posted by Nicolas Dufresne 3 weeks, 4 days ago

Le mercredi 04 mars 2026 à 03:19 +0000, Fan Wu a écrit :
> The mtk_jpeg_release() function frees the context structure (ctx) without
> first cancelling any pending or running work in ctx->jpeg_work. This
> creates a race window where the workqueue callback may still be accessing
> the context memory after it has been freed.
> 
> Race condition:
> 
>     CPU 0 (release)                    CPU 1 (workqueue)
>     ----------------                   ------------------
>     close()
>       mtk_jpeg_release()
>                                        mtk_jpegenc_worker()
>                                          ctx = work->data
>                                          // accessing ctx
> 
>         kfree(ctx)  // freed!
>                                          access ctx  // UAF!
> 
> The work is queued via queue_work() during JPEG encode/decode operations
> (via mtk_jpeg_device_run). If the device is closed while work is pending
> or running, the work handler will access freed memory.
> 
> Fix this by calling cancel_work_sync() BEFORE acquiring the mutex. This
> ordering is critical: if cancel_work_sync() is called after mutex_lock(),
> and the work handler also tries to acquire the same mutex, it would cause
> a deadlock.
> 
> Note: The open error path does NOT need cancel_work_sync() because
> INIT_WORK() only initializes the work structure - it does not schedule
> it. Work is only scheduled later during ioctl operations.
> 
> Fixes: 5fb1c2361e56 ("mtk-jpegenc: add jpeg encode worker interface")
> Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  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 c01124a349f6..8c684756d5fc 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1202,6 +1202,7 @@ static int mtk_jpeg_release(struct file *file)
>  	struct mtk_jpeg_dev *jpeg = video_drvdata(file);
>  	struct mtk_jpeg_ctx *ctx = mtk_jpeg_file_to_ctx(file);
>  
> +	cancel_work_sync(&ctx->jpeg_work);
>  	mutex_lock(&jpeg->lock);
>  	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
>  	v4l2_ctrl_handler_free(&ctx->ctrl_hdl);

Re: [PATCH] media: mtk-jpeg: fix use-after-free in release path due to uncancelled work

Posted by Nicolas Dufresne 3 weeks, 4 days ago

Le mercredi 04 mars 2026 à 03:19 +0000, Fan Wu a écrit :
> The mtk_jpeg_release() function frees the context structure (ctx) without
> first cancelling any pending or running work in ctx->jpeg_work. This
> creates a race window where the workqueue callback may still be accessing
> the context memory after it has been freed.
> 
> Race condition:
> 
>     CPU 0 (release)                    CPU 1 (workqueue)
>     ----------------                   ------------------
>     close()
>       mtk_jpeg_release()
>                                        mtk_jpegenc_worker()
>                                          ctx = work->data
>                                          // accessing ctx
> 
>         kfree(ctx)  // freed!
>                                          access ctx  // UAF!
> 
> The work is queued via queue_work() during JPEG encode/decode operations
> (via mtk_jpeg_device_run). If the device is closed while work is pending
> or running, the work handler will access freed memory.
> 
> Fix this by calling cancel_work_sync() BEFORE acquiring the mutex. This
> ordering is critical: if cancel_work_sync() is called after mutex_lock(),
> and the work handler also tries to acquire the same mutex, it would cause
> a deadlock.
> 
> Note: The open error path does NOT need cancel_work_sync() because
> INIT_WORK() only initializes the work structure - it does not schedule
> it. Work is only scheduled later during ioctl operations.
> 
> Fixes: 5fb1c2361e56 ("mtk-jpegenc: add jpeg encode worker interface")
> Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  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 c01124a349f6..8c684756d5fc 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -1202,6 +1202,7 @@ static int mtk_jpeg_release(struct file *file)
>  	struct mtk_jpeg_dev *jpeg = video_drvdata(file);
>  	struct mtk_jpeg_ctx *ctx = mtk_jpeg_file_to_ctx(file);
>  
> +	cancel_work_sync(&ctx->jpeg_work);
>  	mutex_lock(&jpeg->lock);
>  	v4l2_m2m_ctx_release(ctx->fh.m2m_ctx);
>  	v4l2_ctrl_handler_free(&ctx->ctrl_hdl);