[PATCH] arch: fix resource leak in chmc.c

[PATCH] arch: fix resource leak in chmc.c

[Jacky Liu]

From: Siyang Liu <1972843537@qq.com>

In the jbusmc_probe function, the device node mem_node fetched 
via of_find_node_by_path("/memory") is not properly freed 
on all code paths. 
This can lead to leakage of device node reference counts, 
which may result in kernel resources not being released.

This issue was detected by rule based static tools 
developed by Tencent.

Signed-off-by: Siyang Liu <1972843537@qq.com>
---
 arch/sparc/kernel/chmc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/sparc/kernel/chmc.c b/arch/sparc/kernel/chmc.c
index d4c74d6b2e1b..a7040ee7e5bc 100644
--- a/arch/sparc/kernel/chmc.c
+++ b/arch/sparc/kernel/chmc.c
@@ -4,6 +4,7 @@
  * Copyright (C) 2001, 2007, 2008 David S. Miller (davem@davemloft.net)
  */
 
+#include <linux/cleanup.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
@@ -397,14 +398,14 @@ static void jbusmc_construct_dimm_groups(struct jbusmc *p,
 static int jbusmc_probe(struct platform_device *op)
 {
 	const struct linux_prom64_registers *mem_regs;
-	struct device_node *mem_node;
+	struct device_node *mem_node __free(device_node) =
+			of_find_node_by_path("/memory");
 	int err, len, num_mem_regs;
 	struct jbusmc *p;
 	const u32 *prop;
 	const void *ml;
 
 	err = -ENODEV;
-	mem_node = of_find_node_by_path("/memory");
 	if (!mem_node) {
 		printk(KERN_ERR PFX "Cannot find /memory node.\n");
 		goto out;
-- 
2.43.5

Re: [PATCH] arch: fix resource leak in chmc.c

[Krzysztof Kozlowski]

On 18/07/2025 11:22, Jacky Liu wrote:
> From: Siyang Liu <1972843537@qq.com>
> 
> In the jbusmc_probe function, the device node mem_node fetched 
> via of_find_node_by_path("/memory") is not properly freed 
> on all code paths. 
> This can lead to leakage of device node reference counts, 
> which may result in kernel resources not being released.
> 
> This issue was detected by rule based static tools 
> developed by Tencent.


Last time you were using AI, so I have doubts this is "static tools".
Please describe it properly, so we can make informed decision whether to
allocate time on this.

> 
> Signed-off-by: Siyang Liu <1972843537@qq.com>
> ---
>  arch/sparc/kernel/chmc.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/sparc/kernel/chmc.c b/arch/sparc/kernel/chmc.c
> index d4c74d6b2e1b..a7040ee7e5bc 100644
> --- a/arch/sparc/kernel/chmc.c
> +++ b/arch/sparc/kernel/chmc.c
> @@ -4,6 +4,7 @@
>   * Copyright (C) 2001, 2007, 2008 David S. Miller (davem@davemloft.net)
>   */
>  
> +#include <linux/cleanup.h>
>  #include <linux/module.h>
>  #include <linux/kernel.h>
>  #include <linux/types.h>
> @@ -397,14 +398,14 @@ static void jbusmc_construct_dimm_groups(struct jbusmc *p,
>  static int jbusmc_probe(struct platform_device *op)
>  {
>  	const struct linux_prom64_registers *mem_regs;
> -	struct device_node *mem_node;
> +	struct device_node *mem_node __free(device_node) =
> +			of_find_node_by_path("/memory");


Nah, just free it immediately after user. Don't over complicate this.


Best regards,
Krzysztof

Re: [PATCH] arch: fix resource leak in chmc.c

[Markus Elfring]

> In the jbusmc_probe function, the device node mem_node fetched 
> via of_find_node_by_path("/memory") is not properly freed 
> on all code paths. 
…

You may occasionally put more than 62 characters into text lines
of such a change description.

See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16-rc6#n94


* How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?

* Will an other subsystem specification be more appropriate?

* Would it be more helpful to refer to “jbusmc_probe()” instead of “chmc.c”
  in the summary phrase?


Regards,
Markus

Re:[PATCH] arch: fix resource leak in chmc.c

[jackysliu]

On Fri, Jul 18 2025 15:25:47 +0200  Krzysztof Kozlowski wrote:
>Last time you were using AI, so I have doubts this is "static tools".
>Please describe it properly, so we can make informed decision whether to
>allocate time on this.

The tool determines which api is responsible for resource request 
and release, and determines if there are resource leak or double free 
issues in the code by determining the use of release specifications.
So no AI involved, don't worry.

>Nah, just free it immediately after user. Don't over complicate this.
OK, I'll resubmit a new patch later


Siyang Liu

[PATCH v2] arch: fix resource leak in jbusmc_probe()

[jackysliu]

From: Siyang Liu <1972843537@qq.com>

In the jbusmc_probe function, the device node mem_node fetched
via of_find_node_by_path("/memory") is not properly freed
on all code paths.
This can lead to leakage of device node reference counts,
which may result in kernel resources not being released.

This issue was detected by rule based static tools
developed by Tencent.

Signed-off-by: Siyang Liu <1972843537@qq.com>
---
 arch/sparc/kernel/chmc.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/sparc/kernel/chmc.c b/arch/sparc/kernel/chmc.c
index d4c74d6b2e1b..fd20e4ee0971 100644
--- a/arch/sparc/kernel/chmc.c
+++ b/arch/sparc/kernel/chmc.c
@@ -412,7 +412,7 @@ static int jbusmc_probe(struct platform_device *op)
 	mem_regs = of_get_property(mem_node, "reg", &len);
 	if (!mem_regs) {
 		printk(KERN_ERR PFX "Cannot get reg property of /memory node.\n");
-		goto out;
+		goto out_put;
 	}
 	num_mem_regs = len / sizeof(*mem_regs);
 
@@ -420,7 +420,7 @@ static int jbusmc_probe(struct platform_device *op)
 	p = kzalloc(sizeof(*p), GFP_KERNEL);
 	if (!p) {
 		printk(KERN_ERR PFX "Cannot allocate struct jbusmc.\n");
-		goto out;
+		goto out_put;
 	}
 
 	INIT_LIST_HEAD(&p->list);
@@ -473,6 +473,10 @@ static int jbusmc_probe(struct platform_device *op)
 
 	err = 0;
 
+out_put:
+	of_node_put(mem_node);
+	goto out;
+
 out:
 	return err;
 
@@ -481,7 +485,7 @@ static int jbusmc_probe(struct platform_device *op)
 
 out_free:
 	kfree(p);
-	goto out;
+	goto out_put;
 }
 
 /* Does BANK decode PHYS_ADDR? */
-- 
2.43.5

Re: [PATCH v2] arch: fix resource leak in jbusmc_probe()

[Markus Elfring]

…> This can lead to leakage of device node reference counts,
> which may result in kernel resources not being released.
…

See also one more:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16-rc6#n94


* Will you become more interested in additional tags (like “Fixes” and “Cc”)?

* Would the subsystem specification “sparc” be more appropriate
  in the summary phrase?


…> ---
>  arch/sparc/kernel/chmc.c | 10 +++++++---
…

How do you think about to improve your version management?
https://lore.kernel.org/all/?q=%22This+looks+like+a+new+version+of+a+previously+submitted+patch%22
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16-rc6#n784

Regards,
Markus

[PATCH v3] sparc: fix resource leak in jbusmc_probe()

[jackysliu]

From: Siyang Liu <1972843537@qq.com>

In the jbusmc_probe function, the device node mem_node fetched
via of_find_node_by_path("/memory") is not properly freed
on all code paths.
This can lead to leakage of device node reference counts,
which may result in kernel resources not being released.

This issue was detected by rule based static tools
developed by Tencent.

Fixes: e70140ba0d2b ("Get rid of 'remove_new' relic from platform driver struct")

Signed-off-by: Siyang Liu <1972843537@qq.com>
---
 arch/sparc/kernel/chmc.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/sparc/kernel/chmc.c b/arch/sparc/kernel/chmc.c
index d4c74d6b2e1b..fd20e4ee0971 100644
--- a/arch/sparc/kernel/chmc.c
+++ b/arch/sparc/kernel/chmc.c
@@ -412,7 +412,7 @@ static int jbusmc_probe(struct platform_device *op)
 	mem_regs = of_get_property(mem_node, "reg", &len);
 	if (!mem_regs) {
 		printk(KERN_ERR PFX "Cannot get reg property of /memory node.\n");
-		goto out;
+		goto out_put;
 	}
 	num_mem_regs = len / sizeof(*mem_regs);
 
@@ -420,7 +420,7 @@ static int jbusmc_probe(struct platform_device *op)
 	p = kzalloc(sizeof(*p), GFP_KERNEL);
 	if (!p) {
 		printk(KERN_ERR PFX "Cannot allocate struct jbusmc.\n");
-		goto out;
+		goto out_put;
 	}
 
 	INIT_LIST_HEAD(&p->list);
@@ -473,6 +473,10 @@ static int jbusmc_probe(struct platform_device *op)
 
 	err = 0;
 
+out_put:
+	of_node_put(mem_node);
+	goto out;
+
 out:
 	return err;
 
@@ -481,7 +485,7 @@ static int jbusmc_probe(struct platform_device *op)
 
 out_free:
 	kfree(p);
-	goto out;
+	goto out_put;
 }
 
 /* Does BANK decode PHYS_ADDR? */
-- 
2.43.5

Re: [PATCH v3] sparc: fix resource leak in jbusmc_probe()

[Markus Elfring]

…> ---
>  arch/sparc/kernel/chmc.c | 10 +++++++---
…


Why did you overlook mentioned patch requirements once more?

https://lore.kernel.org/all/?q=%22This+looks+like+a+new+version+of+a+previously+submitted+patch%22
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16-rc6#n784

Regards,
Markus

Re: [PATCH v2] arch: fix resource leak in jbusmc_probe()

[Krzysztof Kozlowski]

On 21/07/2025 05:57, jackysliu wrote:
> From: Siyang Liu <1972843537@qq.com>
> 
> In the jbusmc_probe function, the device node mem_node fetched
> via of_find_node_by_path("/memory") is not properly freed
> on all code paths.
> This can lead to leakage of device node reference counts,
> which may result in kernel resources not being released.

Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597


Do not attach (thread) your patchsets to some other threads (unrelated
or older versions). This buries them deep in the mailbox and might
interfere with applying entire sets.

> 
> This issue was detected by rule based static tools
> developed by Tencent.

What tools? This must be specific, give their name.

Previously Tencent reports were often bogus or low quality.

> 
> Signed-off-by: Siyang Liu <1972843537@qq.com>
> ---
>  arch/sparc/kernel/chmc.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/sparc/kernel/chmc.c b/arch/sparc/kernel/chmc.c
> index d4c74d6b2e1b..fd20e4ee0971 100644
> --- a/arch/sparc/kernel/chmc.c
> +++ b/arch/sparc/kernel/chmc.c
> @@ -412,7 +412,7 @@ static int jbusmc_probe(struct platform_device *op)
>  	mem_regs = of_get_property(mem_node, "reg", &len);
>  	if (!mem_regs) {
>  		printk(KERN_ERR PFX "Cannot get reg property of /memory node.\n");
> -		goto out;
> +		goto out_put;

You did not implement at all what I asked. I wanted the code to be
simpler, you just made it more complicated. Read again previous feedback.

Best regards,
Krzysztof

Re: [PATCH v3] sparc: fix resource leak in jbusmc_probe()

[Krzysztof Kozlowski]

On 21/07/2025 09:55, jackysliu wrote:
> From: Siyang Liu <1972843537@qq.com>
> 
> In the jbusmc_probe function, the device node mem_node fetched
> via of_find_node_by_path("/memory") is not properly freed
> on all code paths.
> This can lead to leakage of device node reference counts,
> which may result in kernel resources not being released.
> 
> This issue was detected by rule based static tools
> developed by Tencent.
> 
> Fixes: e70140ba0d2b ("Get rid of 'remove_new' relic from platform driver struct")
> 
> Signed-off-by: Siyang Liu <1972843537@qq.com>
> ---
>  arch/sparc/kernel/chmc.c | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/sparc/kernel/chmc.c b/arch/sparc/kernel/chmc.c
> index d4c74d6b2e1b..fd20e4ee0971 100644
> --- a/arch/sparc/kernel/chmc.c
> +++ b/arch/sparc/kernel/chmc.c
> @@ -412,7 +412,7 @@ static int jbusmc_probe(struct platform_device *op)
>  	mem_regs = of_get_property(mem_node, "reg", &len);
>  	if (!mem_regs) {
>  		printk(KERN_ERR PFX "Cannot get reg property of /memory node.\n");
> -		goto out;
> +		goto out_put;

Nothing improved. You just ignored all the comments.

Best regards,
Krzysztof