* FAILED: patch "[PATCH] media: tuner: xc5000: Fix use-after-free in xc5000_release
@ 2025-10-03 13:12 gregkh
2025-10-04 0:40 ` [PATCH 5.4.y 1/2] media: tunner: xc5000: Refactor firmware load Sasha Levin
0 siblings, 1 reply; 3+ messages in thread
From: gregkh @ 2025-10-03 13:12 UTC (permalink / raw)
To: duoming, hverkuil+cisco; +Cc: stable
The patch below does not apply to the 5.4-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.4.y
git checkout FETCH_HEAD
git cherry-pick -x 40b7a19f321e65789612ebaca966472055dab48c
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2025100341-dime-left-e15f@gregkh' --subject-prefix 'PATCH 5.4.y' HEAD^..
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 40b7a19f321e65789612ebaca966472055dab48c Mon Sep 17 00:00:00 2001
From: Duoming Zhou <duoming@zju.edu.cn>
Date: Wed, 17 Sep 2025 17:56:08 +0800
Subject: [PATCH] media: tuner: xc5000: Fix use-after-free in xc5000_release
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: stable@vger.kernel.org
Signed-off-by: Duoming Zhou <duoming@zju.edu.cn>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
[hverkuil: fix typo in Subject: tunner -> tuner]
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);
}
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH 5.4.y 1/2] media: tunner: xc5000: Refactor firmware load 2025-10-03 13:12 FAILED: patch "[PATCH] media: tuner: xc5000: Fix use-after-free in xc5000_release gregkh @ 2025-10-04 0:40 ` Sasha Levin 2025-10-04 0:40 ` [PATCH 5.4.y 2/2] media: tuner: xc5000: Fix use-after-free in xc5000_release Sasha Levin 0 siblings, 1 reply; 3+ messages in thread From: Sasha Levin @ 2025-10-04 0:40 UTC (permalink / raw) To: stable; +Cc: Ricardo Ribalda, Shuah Khan, Hans Verkuil, Sasha Levin From: Ricardo Ribalda <ribalda@chromium.org> [ Upstream commit 8e1f5da59dd4a1966f859639860b803a7e8b8bfb ] Make sure the firmware is released when we leave xc_load_fw_and_init_tuner() This change makes smatch happy: drivers/media/tuners/xc5000.c:1213 xc_load_fw_and_init_tuner() warn: 'fw' from request_firmware() not released on lines: 1213. Cc: Shuah Khan <shuah.kh@samsung.com> Signed-off-by: Ricardo Ribalda <ribalda@chromium.org> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Stable-dep-of: 40b7a19f321e ("media: tuner: xc5000: Fix use-after-free in xc5000_release") Signed-off-by: Sasha Levin <sashal@kernel.org> --- drivers/media/tuners/xc5000.c | 39 +++++++++++++++-------------------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/drivers/media/tuners/xc5000.c b/drivers/media/tuners/xc5000.c index 65b8863385575..8b939ab84ee77 100644 --- a/drivers/media/tuners/xc5000.c +++ b/drivers/media/tuners/xc5000.c @@ -58,7 +58,7 @@ struct xc5000_priv { struct dvb_frontend *fe; struct delayed_work timer_sleep; - const struct firmware *firmware; + bool inited; }; /* Misc Defines */ @@ -1110,23 +1110,19 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe, int force) if (!force && xc5000_is_firmware_loaded(fe) == 0) return 0; - if (!priv->firmware) { - ret = request_firmware(&fw, desired_fw->name, - priv->i2c_props.adap->dev.parent); - if (ret) { - pr_err("xc5000: Upload failed. rc %d\n", ret); - return ret; - } - dprintk(1, "firmware read %zu bytes.\n", fw->size); + ret = request_firmware(&fw, desired_fw->name, + priv->i2c_props.adap->dev.parent); + if (ret) { + pr_err("xc5000: Upload failed. rc %d\n", ret); + return ret; + } + dprintk(1, "firmware read %zu bytes.\n", fw->size); - if (fw->size != desired_fw->size) { - pr_err("xc5000: Firmware file with incorrect size\n"); - release_firmware(fw); - return -EINVAL; - } - priv->firmware = fw; - } else - fw = priv->firmware; + if (fw->size != desired_fw->size) { + pr_err("xc5000: Firmware file with incorrect size\n"); + release_firmware(fw); + return -EINVAL; + } /* Try up to 5 times to load firmware */ for (i = 0; i < 5; i++) { @@ -1204,6 +1200,7 @@ static int xc_load_fw_and_init_tuner(struct dvb_frontend *fe, int force) } err: + release_firmware(fw); if (!ret) printk(KERN_INFO "xc5000: Firmware %s loaded and running.\n", desired_fw->name); @@ -1274,7 +1271,7 @@ static int xc5000_resume(struct dvb_frontend *fe) /* suspended before firmware is loaded. Avoid firmware load in resume path. */ - if (!priv->firmware) + if (!priv->inited) return 0; return xc5000_set_params(fe); @@ -1293,6 +1290,8 @@ static int xc5000_init(struct dvb_frontend *fe) if (debug) xc_debug_dump(priv); + priv->inited = true; + return 0; } @@ -1306,10 +1305,6 @@ static void xc5000_release(struct dvb_frontend *fe) if (priv) { cancel_delayed_work(&priv->timer_sleep); - if (priv->firmware) { - release_firmware(priv->firmware); - priv->firmware = NULL; - } hybrid_tuner_release_state(priv); } -- 2.51.0 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 5.4.y 2/2] media: tuner: xc5000: Fix use-after-free in xc5000_release 2025-10-04 0:40 ` [PATCH 5.4.y 1/2] media: tunner: xc5000: Refactor firmware load Sasha Levin @ 2025-10-04 0:40 ` Sasha Levin 0 siblings, 0 replies; 3+ messages in thread From: Sasha Levin @ 2025-10-04 0:40 UTC (permalink / raw) To: stable; +Cc: Duoming Zhou, Hans Verkuil, Sasha Levin From: Duoming Zhou <duoming@zju.edu.cn> [ Upstream commit 40b7a19f321e65789612ebaca966472055dab48c ] 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: stable@vger.kernel.org Signed-off-by: Duoming Zhou <duoming@zju.edu.cn> Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org> [hverkuil: fix typo in Subject: tunner -> tuner] Signed-off-by: Sasha Levin <sashal@kernel.org> --- 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 8b939ab84ee77..ea4f1ae4d686e 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); } -- 2.51.0 ^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-10-04 0:40 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-03 13:12 FAILED: patch "[PATCH] media: tuner: xc5000: Fix use-after-free in xc5000_release gregkh 2025-10-04 0:40 ` [PATCH 5.4.y 1/2] media: tunner: xc5000: Refactor firmware load Sasha Levin 2025-10-04 0:40 ` [PATCH 5.4.y 2/2] media: tuner: xc5000: Fix use-after-free in xc5000_release Sasha Levin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).