[PATCH] serial: vt8500: Fix clock reference leak in vt8500_serial_probe()

[PATCH] serial: vt8500: Fix clock reference leak in vt8500_serial_probe()

[Wentao Liang]

of_clk_get() returns a clock with a reference that has to be released
with clk_put(). If clk_prepare_enable() fails the function returns
without doing so, leaking the reference.

Add the missing clk_put() on that error path.

Fixes: 12faa35ae5cb ("serial: vt8500: UART uses gated clock rather than 24Mhz reference")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/tty/serial/vt8500_serial.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c
index 78a1c1eea11b..fc6fdc751b16 100644
--- a/drivers/tty/serial/vt8500_serial.c
+++ b/drivers/tty/serial/vt8500_serial.c
@@ -656,6 +656,7 @@ static int vt8500_serial_probe(struct platform_device *pdev)
 	ret = clk_prepare_enable(vt8500_port->clk);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to enable clock\n");
+		clk_put(vt8500_port->clk);
 		return ret;
 	}
 
-- 
2.34.1

Re: [PATCH] serial: vt8500: Fix clock reference leak in vt8500_serial_probe()

[Alexey Charkov]

On Thu, Sep 17, 2026 at 7:58 PM Wentao Liang <vulab@iscas.ac.cn> wrote:
>
> of_clk_get() returns a clock with a reference that has to be released
> with clk_put(). If clk_prepare_enable() fails the function returns
> without doing so, leaking the reference.
>
> Add the missing clk_put() on that error path.
>
> Fixes: 12faa35ae5cb ("serial: vt8500: UART uses gated clock rather than 24Mhz reference")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>  drivers/tty/serial/vt8500_serial.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c
> index 78a1c1eea11b..fc6fdc751b16 100644
> --- a/drivers/tty/serial/vt8500_serial.c
> +++ b/drivers/tty/serial/vt8500_serial.c
> @@ -656,6 +656,7 @@ static int vt8500_serial_probe(struct platform_device *pdev)
>         ret = clk_prepare_enable(vt8500_port->clk);
>         if (ret) {
>                 dev_err(&pdev->dev, "failed to enable clock\n");
> +               clk_put(vt8500_port->clk);

The probe function currently mixes devm-managed and manual resource
allocations, with the clk standing out in particular. Instead of
adding a manual clk_put, could you please switch the clk calls to
devm_clk_get_enabled and drop redundant boilerplate?

Best regards,
Alexey

[PATCH] serial: vt8500: Use devm_clk_get_enabled()

[Wentao Liang]

Hi Alexey,

Sorry for the very late reply. Working through the review comments I got
in September has taken me far longer than it should have.

Your suggestion was right, but the leak fix went in before I could act on
it: it is commit 9d8955281d92 ("serial: vt8500: Fix clock reference leak
in vt8500_serial_probe()") in mainline, and it kept the clk_put() you
asked me to replace. It has been backported to the stable trees by now as
well. So the patch below is not a v2 of that one, which would only resend
something that is already applied, but a follow-up that does the
conversion on top of it.

devm_clk_get_enabled() gets the clock and enables it in one call and
registers the disable and put with devm, so both the manual error handling
and the clk_put() go away. Probe now returns the error code from the clock
lookup instead of a hard-coded -EINVAL, and the clock is released
automatically through devres. The success path is unchanged and nothing
else in the driver is touched.

For the record on where this came from: the candidate came from our static
analysis tooling, and DeepSeek and GPT-6.1 Sol were used to audit it and to
help write the change. I reviewed the result and I am responsible for it.
The patch is build-tested only: it compiles cleanly with W=1 for arm
(vt8500_v6_v7_defconfig, CONFIG_SERIAL_VT8500=y) and passes
checkpatch.pl --strict; I have no VT8500 hardware to test it on.

Thanks for the review,
Wentao

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Wentao Liang <vulab@iscas.ac.cn>
Date: Sun, 8 Nov 2026 11:41:30 +0000
Subject: [PATCH] serial: vt8500: Use devm_clk_get_enabled()

vt8500_serial_probe() gets the clock with of_clk_get() and enables it
with clk_prepare_enable(), so the reference has to be dropped by hand on
the error path, and the clock is never disabled or released afterwards,
even though the rest of the probe is devm-managed.

Use devm_clk_get_enabled() instead, which gets and enables the clock in
one call and registers the teardown with devm. The manual error handling
and the clk_put() can then go, and probe returns the actual error code
from the clock lookup rather than a hard-coded -EINVAL.

The clock still stays enabled as long as the port is in use, so the
success path is unchanged.
---
 drivers/tty/serial/vt8500_serial.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c
index fc6fdc751b16..73922523dff6 100644
--- a/drivers/tty/serial/vt8500_serial.c
+++ b/drivers/tty/serial/vt8500_serial.c
@@ -603,7 +603,6 @@ static int vt8500_serial_probe(struct platform_device *pdev)
 	struct resource *mmres;
 	struct device_node *np = pdev->dev.of_node;
 	const unsigned int *flags;
-	int ret;
 	int port;
 	int irq;
 
@@ -647,17 +646,10 @@ static int vt8500_serial_probe(struct platform_device *pdev)
 	if (IS_ERR(vt8500_port->uart.membase))
 		return PTR_ERR(vt8500_port->uart.membase);
 
-	vt8500_port->clk = of_clk_get(pdev->dev.of_node, 0);
+	vt8500_port->clk = devm_clk_get_enabled(&pdev->dev, NULL);
 	if (IS_ERR(vt8500_port->clk)) {
 		dev_err(&pdev->dev, "failed to get clock\n");
-		return  -EINVAL;
-	}
-
-	ret = clk_prepare_enable(vt8500_port->clk);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to enable clock\n");
-		clk_put(vt8500_port->clk);
-		return ret;
+		return PTR_ERR(vt8500_port->clk);
 	}
 
 	vt8500_port->vt8500_uart_flags = *flags;
-- 
2.34.1

Re: [PATCH] serial: vt8500: Use devm_clk_get_enabled()

[Alexey Charkov]

Hi Wentao,

On Sun, Nov 8, 2026 at 4:15 PM Wentao Liang <vulab@iscas.ac.cn> wrote:
>
> Hi Alexey,
>
> Sorry for the very late reply. Working through the review comments I got
> in September has taken me far longer than it should have.
>
> Your suggestion was right, but the leak fix went in before I could act on
> it: it is commit 9d8955281d92 ("serial: vt8500: Fix clock reference leak
> in vt8500_serial_probe()") in mainline, and it kept the clk_put() you
> asked me to replace. It has been backported to the stable trees by now as
> well. So the patch below is not a v2 of that one, which would only resend
> something that is already applied, but a follow-up that does the
> conversion on top of it.
>
> devm_clk_get_enabled() gets the clock and enables it in one call and
> registers the disable and put with devm, so both the manual error handling
> and the clk_put() go away. Probe now returns the error code from the clock
> lookup instead of a hard-coded -EINVAL, and the clock is released
> automatically through devres. The success path is unchanged and nothing
> else in the driver is touched.
>
> For the record on where this came from: the candidate came from our static
> analysis tooling, and DeepSeek and GPT-6.1 Sol were used to audit it and to
> help write the change. I reviewed the result and I am responsible for it.
> The patch is build-tested only: it compiles cleanly with W=1 for arm
> (vt8500_v6_v7_defconfig, CONFIG_SERIAL_VT8500=y) and passes
> checkpatch.pl --strict; I have no VT8500 hardware to test it on.
>
> Thanks for the review,
> Wentao
>
> From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
> From: Wentao Liang <vulab@iscas.ac.cn>
> Date: Sun, 8 Nov 2026 11:41:30 +0000
> Subject: [PATCH] serial: vt8500: Use devm_clk_get_enabled()
>
> vt8500_serial_probe() gets the clock with of_clk_get() and enables it
> with clk_prepare_enable(), so the reference has to be dropped by hand on
> the error path, and the clock is never disabled or released afterwards,
> even though the rest of the probe is devm-managed.
>
> Use devm_clk_get_enabled() instead, which gets and enables the clock in
> one call and registers the teardown with devm. The manual error handling
> and the clk_put() can then go, and probe returns the actual error code
> from the clock lookup rather than a hard-coded -EINVAL.
>
> The clock still stays enabled as long as the port is in use, so the
> success path is unchanged.

Thanks for the patch, but you missed the sign-off line, without which
it cannot be taken any further. The concern raised by Sashiko in its
reply is also valid. The whole bitmap is quite an eye sore frankly, so
it should probably just go anyway.

Let me take a stab at these - I'll put you in Cc.

Best regards,
Alexey