2022-06-16 14:56:03

by Liang He

[permalink] [raw]
Subject: [PATCH] soc: amlogic: Fix refcount leak in meson-secure-pwrc.c

In meson_secure_pwrc_probe(), there is a refcount leak in one fail
path.

Signed-off-by: Liang He <[email protected]>
---
drivers/soc/amlogic/meson-secure-pwrc.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/amlogic/meson-secure-pwrc.c b/drivers/soc/amlogic/meson-secure-pwrc.c
index a10a417a87db..e93518763526 100644
--- a/drivers/soc/amlogic/meson-secure-pwrc.c
+++ b/drivers/soc/amlogic/meson-secure-pwrc.c
@@ -152,8 +152,10 @@ static int meson_secure_pwrc_probe(struct platform_device *pdev)
}

pwrc = devm_kzalloc(&pdev->dev, sizeof(*pwrc), GFP_KERNEL);
- if (!pwrc)
+ if (!pwrc) {
+ of_node_put(sm_np);
return -ENOMEM;
+ }

pwrc->fw = meson_sm_get(sm_np);
of_node_put(sm_np);
--
2.25.1


2022-06-16 20:19:10

by Martin Blumenstingl

[permalink] [raw]
Subject: Re: [PATCH] soc: amlogic: Fix refcount leak in meson-secure-pwrc.c

Hello,

On Thu, Jun 16, 2022 at 4:50 PM Liang He <[email protected]> wrote:
>
> In meson_secure_pwrc_probe(), there is a refcount leak in one fail
> path.
>
> Signed-off-by: Liang He <[email protected]>
Acked-by: Martin Blumenstingl <[email protected]>

[...]
> pwrc = devm_kzalloc(&pdev->dev, sizeof(*pwrc), GFP_KERNEL);
> - if (!pwrc)
> + if (!pwrc) {
> + of_node_put(sm_np);
> return -ENOMEM;
> + }
Another approach would be to just move devm_kzalloc() a few lines
further up (above of_find_compatible_node()).
That would catch similar issues in future when someone wants to add
more code right after devm_kzalloc(). That said, I don't think that
this is a likely scenario so the patch is fine for me as-is.

Thanks a lot for submitting this!


Best regards,
Martin

2022-06-17 01:42:39

by Liang He

[permalink] [raw]
Subject: Re:Re: [PATCH] soc: amlogic: Fix refcount leak in meson-secure-pwrc.c




At 2022-06-17 04:17:19, "Martin Blumenstingl" <[email protected]> wrote:
>Hello,
>
>On Thu, Jun 16, 2022 at 4:50 PM Liang He <[email protected]> wrote:
>>
>> In meson_secure_pwrc_probe(), there is a refcount leak in one fail
>> path.
>>
>> Signed-off-by: Liang He <[email protected]>
>Acked-by: Martin Blumenstingl <[email protected]>
>
>[...]
>> pwrc = devm_kzalloc(&pdev->dev, sizeof(*pwrc), GFP_KERNEL);
>> - if (!pwrc)
>> + if (!pwrc) {
>> + of_node_put(sm_np);
>> return -ENOMEM;
>> + }
>Another approach would be to just move devm_kzalloc() a few lines
>further up (above of_find_compatible_node()).
>That would catch similar issues in future when someone wants to add
>more code right after devm_kzalloc(). That said, I don't think that
>this is a likely scenario so the patch is fine for me as-is.
>
>Thanks a lot for submitting this!
>
>
>Best regards,
>Martin

Thanks for your confirm.

Liang

2022-06-17 08:03:42

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH] soc: amlogic: Fix refcount leak in meson-secure-pwrc.c

Hi,

On Thu, 16 Jun 2022 22:49:15 +0800, Liang He wrote:
> In meson_secure_pwrc_probe(), there is a refcount leak in one fail
> path.
>
>

Thanks, Applied to https://git.kernel.org/pub/scm/linux/kernel/git/amlogic/linux.git (v5.20/drivers)

[1/1] soc: amlogic: Fix refcount leak in meson-secure-pwrc.c
https://git.kernel.org/amlogic/c/d18529a4c12f66d83daac78045ea54063bd43257

These changes has been applied on the intermediate git tree [1].

The v5.20/drivers branch will then be sent via a formal Pull Request to the Linux SoC maintainers
for inclusion in their intermediate git branches in order to be sent to Linus during
the next merge window, or sooner if it's a set of fixes.

In the cases of fixes, those will be merged in the current release candidate
kernel and as soon they appear on the Linux master branch they will be
backported to the previous Stable and Long-Stable kernels [2].

The intermediate git branches are merged daily in the linux-next tree [3],
people are encouraged testing these pre-release kernels and report issues on the
relevant mailing-lists.

If problems are discovered on those changes, please submit a signed-off-by revert
patch followed by a corrective changeset.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/amlogic/linux.git
[2] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
[3] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git

--
Neil

2022-06-17 08:11:44

by Neil Armstrong

[permalink] [raw]
Subject: Re: [PATCH] soc: amlogic: Fix refcount leak in meson-secure-pwrc.c

On 16/06/2022 16:49, Liang He wrote:
> In meson_secure_pwrc_probe(), there is a refcount leak in one fail
> path.
>
> Signed-off-by: Liang He <[email protected]>
Fixes: b3dde5013e13 ("soc: amlogic: Add support for Secure power domains controller")
> ---
> drivers/soc/amlogic/meson-secure-pwrc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/amlogic/meson-secure-pwrc.c b/drivers/soc/amlogic/meson-secure-pwrc.c
> index a10a417a87db..e93518763526 100644
> --- a/drivers/soc/amlogic/meson-secure-pwrc.c
> +++ b/drivers/soc/amlogic/meson-secure-pwrc.c
> @@ -152,8 +152,10 @@ static int meson_secure_pwrc_probe(struct platform_device *pdev)
> }
>
> pwrc = devm_kzalloc(&pdev->dev, sizeof(*pwrc), GFP_KERNEL);
> - if (!pwrc)
> + if (!pwrc) {
> + of_node_put(sm_np);
> return -ENOMEM;
> + }
>
> pwrc->fw = meson_sm_get(sm_np);
> of_node_put(sm_np);


Reviewed-by: Neil Armstrong <[email protected]>