gpio: rockchip: fix OF node leak in probe()

[PATCH] gpio: rockchip: fix OF node leak in probe()

Posted by Krzysztof Kozlowski 1 year, 3 months ago

Driver code is leaking OF node reference from of_get_parent() in
probe().

Fixes: 936ee2675eee ("gpio/rockchip: add driver for rockchip gpio")
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 drivers/gpio/gpio-rockchip.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
index 0bd339813110..365ab947983c 100644
--- a/drivers/gpio/gpio-rockchip.c
+++ b/drivers/gpio/gpio-rockchip.c
@@ -713,6 +713,7 @@ static int rockchip_gpio_probe(struct platform_device *pdev)
 		return -ENODEV;
 
 	pctldev = of_pinctrl_get(pctlnp);
+	of_node_put(pctlnp);
 	if (!pctldev)
 		return -EPROBE_DEFER;
 
-- 
2.43.0

Re: [PATCH] gpio: rockchip: fix OF node leak in probe()

Posted by Bartosz Golaszewski 1 year, 3 months ago

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>


On Mon, 26 Aug 2024 17:08:32 +0200, Krzysztof Kozlowski wrote:
> Driver code is leaking OF node reference from of_get_parent() in
> probe().
> 
> 

Applied, thanks!

[1/1] gpio: rockchip: fix OF node leak in probe()
      commit: adad2e460e505a556f5ea6f0dc16fe95e62d5d76

Best regards,
-- 
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Re: [PATCH] gpio: rockchip: fix OF node leak in probe()

Posted by Heiko Stübner 1 year, 3 months ago

Am Montag, 26. August 2024, 17:08:32 CEST schrieb Krzysztof Kozlowski:
> Driver code is leaking OF node reference from of_get_parent() in
> probe().
> 
> Fixes: 936ee2675eee ("gpio/rockchip: add driver for rockchip gpio")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/gpio/gpio-rockchip.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
> index 0bd339813110..365ab947983c 100644
> --- a/drivers/gpio/gpio-rockchip.c
> +++ b/drivers/gpio/gpio-rockchip.c
> @@ -713,6 +713,7 @@ static int rockchip_gpio_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  
>  	pctldev = of_pinctrl_get(pctlnp);
> +	of_node_put(pctlnp);
>  	if (!pctldev)
>  		return -EPROBE_DEFER;
>  
> 

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

Re: [PATCH] gpio: rockchip: fix OF node leak in probe()

Posted by Bartosz Golaszewski 1 year, 3 months ago

On Mon, Aug 26, 2024 at 5:08 PM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> Driver code is leaking OF node reference from of_get_parent() in
> probe().
>
> Fixes: 936ee2675eee ("gpio/rockchip: add driver for rockchip gpio")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  drivers/gpio/gpio-rockchip.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
> index 0bd339813110..365ab947983c 100644
> --- a/drivers/gpio/gpio-rockchip.c
> +++ b/drivers/gpio/gpio-rockchip.c
> @@ -713,6 +713,7 @@ static int rockchip_gpio_probe(struct platform_device *pdev)
>                 return -ENODEV;
>
>         pctldev = of_pinctrl_get(pctlnp);
> +       of_node_put(pctlnp);
>         if (!pctldev)
>                 return -EPROBE_DEFER;
>
> --
> 2.43.0
>

How about using __free(device_node)?

Bart

Re: [PATCH] gpio: rockchip: fix OF node leak in probe()

Posted by Krzysztof Kozlowski 1 year, 3 months ago

On 02/09/2024 12:23, Bartosz Golaszewski wrote:
> On Mon, Aug 26, 2024 at 5:08 PM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> Driver code is leaking OF node reference from of_get_parent() in
>> probe().
>>
>> Fixes: 936ee2675eee ("gpio/rockchip: add driver for rockchip gpio")
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> ---
>>  drivers/gpio/gpio-rockchip.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
>> index 0bd339813110..365ab947983c 100644
>> --- a/drivers/gpio/gpio-rockchip.c
>> +++ b/drivers/gpio/gpio-rockchip.c
>> @@ -713,6 +713,7 @@ static int rockchip_gpio_probe(struct platform_device *pdev)
>>                 return -ENODEV;
>>
>>         pctldev = of_pinctrl_get(pctlnp);
>> +       of_node_put(pctlnp);
>>         if (!pctldev)
>>                 return -EPROBE_DEFER;
>>
>> --
>> 2.43.0
>>
> 
> How about using __free(device_node)?

I can but I find it less readable. Existing code, after my patch, is
pretty straightforward:

        struct device_node *np = dev->of_node;
        struct device_node *pctlnp = of_get_parent(np);
        struct pinctrl_dev *pctldev = NULL;
        ...

        if (!np || !pctlnp)
                return -ENODEV;

        pctldev = of_pinctrl_get(pctlnp);
        of_node_put(pctlnp);
        if (!pctldev)
                return -EPROBE_DEFER;


So __free() would safe only one of_node_put() and grow its scope
significantly. Above pattern - of_get + use + of_node_put - is pretty
self-contained and readable.

Best regards,
Krzysztof