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

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

[Manush Prajwal]

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 <manushprajwal555@gmail.com>
---
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