[PATCH v3] mailbox: mtk-cmdq: fix runtime PM usage counter leak in cmdq_mbox_flush

"Manush Prajwal" <[email protected]>
Newsgroups org.infradead.lists.linux-mediatek,org.infradead.lists.linux-arm-kernel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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 <[email protected]>
---
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.