linux-kernel - Re: [PATCH] tty: serial: add missing clk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 18 Jul 2023 11:40:01 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     sunran001@...suo.com, gregkh@...uxfoundation.org
Cc:     linux-serial@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] tty: serial: add missing clk_put()

On 18/07/2023 10:55, sunran001@...suo.com wrote:
> This patch fixes the following Coccinelle error:
> 
> ./drivers/tty/serial/bcm63xx_uart.c:854:2-8: ERROR: missing clk_put;
> clk_get on line 849 and execution via conditional on line 853
> 
> Signed-off-by: Ran Sun <sunran001@...suo.com>
> ---
>   drivers/tty/serial/bcm63xx_uart.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/tty/serial/bcm63xx_uart.c 
> b/drivers/tty/serial/bcm63xx_uart.c
> index 55e82d0bf92d..7353b683952d 100644
> --- a/drivers/tty/serial/bcm63xx_uart.c
> +++ b/drivers/tty/serial/bcm63xx_uart.c
> @@ -851,6 +851,7 @@ static int bcm_uart_probe(struct platform_device *pdev)
>           clk = of_clk_get(pdev->dev.of_node, 0);
> 
>       if (IS_ERR(clk))
> +        clk_put(clk);

No!

First, calling clk_put() on the error pointer is incorrect. This will 
throw a warning in __clk_put(), but generally passing an error pointer 
to another function can cause different kinds of breakage.

Second, you have added the line, but this also moved the return 
statement out of the if condition. This way all bcm_uart_probe() calls 
will end up with -ENODEV return value.

Please stop blindly fixing the coccinelle warnings. Take care to read 
and understand the code.

>           return -ENODEV;
> 
>       port->iotype = UPIO_MEM;

-- 
With best wishes
Dmitry