On Wed Sep 17 17:56:08 2025 +0800, Duoming Zhou wrote:
> The original code uses cancel_delayed_work() in xc5000_release(), which
> does not guarantee that the delayed work item timer_sleep has fully
> completed if it was already running. This leads to use-after-free scenarios
> where xc5000_release() may free the xc5000_priv while timer_sleep is still
> active and attempts to dereference the xc5000_priv.
>
> A typical race condition is illustrated below:
>
> CPU 0 (release thread) | CPU 1 (delayed work callback)
> xc5000_release() | xc5000_do_timer_sleep()
> cancel_delayed_work() |
> hybrid_tuner_release_state(priv) |
> kfree(priv) |
> | priv = container_of() // UAF
>
> Replace cancel_delayed_work() with cancel_delayed_work_sync() to ensure
> that the timer_sleep is properly canceled before the xc5000_priv memory
> is deallocated.
>
> A deadlock concern was considered: xc5000_release() is called in a process
> context and is not holding any locks that the timer_sleep work item might
> also need. Therefore, the use of the _sync() variant is safe here.
>
> This bug was initially identified through static analysis.
>
> Fixes: f7a27ff1fb77 ("[media] xc5000: delay tuner sleep to 5 seconds")
> Cc: [email protected]
> Signed-off-by: Duoming Zhou <[email protected]>
> Signed-off-by: Hans Verkuil <[email protected]>
> [hverkuil: fix typo in Subject: tunner -> tuner]
Patch committed.
Thanks,
Hans Verkuil
drivers/media/tuners/xc5000.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
---
diff --git a/drivers/media/tuners/xc5000.c b/drivers/media/tuners/xc5000.c
index bf4ff461e082..a28481edd22e 100644
--- a/drivers/media/tuners/xc5000.c
+++ b/drivers/media/tuners/xc5000.c
@@ -1304,7 +1304,7 @@ static void xc5000_release(struct dvb_frontend *fe)
mutex_lock(&xc5000_list_mutex);
if (priv) {
- cancel_delayed_work(&priv->timer_sleep);
+ cancel_delayed_work_sync(&priv->timer_sleep);
hybrid_tuner_release_state(priv);
}