[PATCH v3] mailbox: mtk-cmdq: fix runtime PM usage counter leak in cmdq_mbox_flush
From: Manush Prajwal <hidden>
Date: 2026-08-24 18:15:37
Also in:
linux-arm-kernel, lkml
Subsystem:
mailbox api, the rest · Maintainers:
Jassi Brar, Linus Torvalds
cmdq_mbox_flush() calls pm_runtime_get_sync() and returns its error
code directly on failure. Per Documentation/power/runtime_pm.rst,
pm_runtime_get_sync() does not drop the usage counter on error, so the
caller is expected to release the reference itself. The early return
here skipped that, leaking a runtime PM usage count on cmdq->mbox.dev
on every failed resume. v2 fixed this by switching to
pm_runtime_resume_and_get(), which performs the get-and-put
internally on failure.
Markus Elfring additionally pointed out that the "out:" and "wait:"
labels duplicate the same pm_runtime_mark_last_busy() +
pm_runtime_put_autosuspend() + return sequence. That duplication
hides a second, separate leak: the "wait:" path's timeout branch
(readl_poll_timeout_atomic() failing) returns -EFAULT directly,
without ever calling pm_runtime_mark_last_busy() /
pm_runtime_put_autosuspend(), so the pm_runtime reference taken at
function entry is leaked on every polling timeout too.
Fix both by merging the two exit sequences into one common "out_pm:"
label reached by both "out:" and "wait:", using "ret" to carry the
return value. "ret" is explicitly reset to 0 on both success paths
before reaching "out_pm:", since pm_runtime_resume_and_get() can
return a positive value (e.g. 1) on success, not just 0, and that
stale value must not leak into the function's return value.
Signed-off-by: Manush Prajwal <redacted>
---
v3: Fix the pm_runtime leak on the "wait:" timeout path found while
addressing Markus Elfring's duplicate-code comment, by merging
the "out:" and "wait:" exit sequences into a single "out_pm:"
label. Add Matthias Brugger and AngeloGioacchino Del Regno plus
the linux-mediatek and linux-arm-kernel lists to Cc, per
MAINTAINERS' "ARM/Mediatek SoC support" entry (matches any
drivers/mailbox/mtk-*.c filename), per Markus Elfring's review.
v2: Use pm_runtime_resume_and_get() instead of pm_runtime_get_sync()
plus a manual pm_runtime_put_noidle() on the error path, per
Markus Elfring's review.
drivers/mailbox/mtk-cmdq-mailbox.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c
index e523c84b4..d4e5f6a7b 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c@@ -565,7 +565,7 @@ static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout) int ret; - ret = pm_runtime_get_sync(cmdq->mbox.dev); + ret = pm_runtime_resume_and_get(cmdq->mbox.dev); if (ret < 0) return ret;
@@ -590,22 +590,21 @@ static int cmdq_mbox_flush(struct mbox_chan *chan, unsigned long timeout) cmdq_thread_disable(cmdq, thread); out: spin_unlock_irqrestore(&thread->chan->lock, flags); - pm_runtime_mark_last_busy(cmdq->mbox.dev); - pm_runtime_put_autosuspend(cmdq->mbox.dev); - - return 0; + ret = 0; + goto out_pm; wait: cmdq_thread_resume(thread); spin_unlock_irqrestore(&thread->chan->lock, flags); + ret = 0; if (readl_poll_timeout_atomic(thread->base + CMDQ_THR_ENABLE_TASK, enable, enable == 0, 1, timeout)) { dev_err(cmdq->mbox.dev, "Fail to wait GCE thread 0x%x done\n", (u32)(thread->base - cmdq->base)); - - return -EFAULT; + ret = -EFAULT; } + +out_pm: pm_runtime_mark_last_busy(cmdq->mbox.dev); pm_runtime_put_autosuspend(cmdq->mbox.dev); - return 0; + return ret; } --
2.46.2.windows.1