linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] dm-raid: do not include dm-core.h
@ 2025-07-21  3:49 Pavel Tikhomirov
  2025-07-21  3:56 ` [PATCH v2] arch: fix resource leak in jbusmc_probe() jackysliu
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Pavel Tikhomirov @ 2025-07-21  3:49 UTC (permalink / raw)
  To: Alasdair Kergon, Mike Snitzer, Mikulas Patocka
  Cc: Xiao Ni, Yu Kuai, dm-devel, linux-kernel, Konstantin Khorenko,
	Denis Lunev

In commit 4cc96131afce ("dm: move request-based code out to dm-rq.[hc]")
we have a note: "DM targets should _never_ include dm-core.h!". And it
is not used in any DM targets except dm-raid now, so let's remove it
from dm-raid for consistency, also use special helpers instead of
accessing dm_table and mapper_device fields directly. This change is
merely a cleanup and should not affect functionality.

Fixes: 7168be3c8a6b ("md: record dm-raid gendisk in mddev")
Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
---
 drivers/md/dm-raid.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index e8c0a8c6fb51..4fb5ddf50560 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -14,7 +14,6 @@
 #include "raid5.h"
 #include "raid10.h"
 #include "md-bitmap.h"
-#include "dm-core.h"
 
 #include <linux/device-mapper.h>
 
@@ -3305,7 +3304,7 @@ static int raid_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 
 	/* Disable/enable discard support on raid set. */
 	configure_discard_support(rs);
-	rs->md.dm_gendisk = ti->table->md->disk;
+	rs->md.dm_gendisk = dm_disk(dm_table_get_md(ti->table));
 
 	mddev_unlock(&rs->md);
 	return 0;
-- 
2.50.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2] arch: fix resource leak in jbusmc_probe()
  2025-07-21  3:49 [PATCH] dm-raid: do not include dm-core.h Pavel Tikhomirov
@ 2025-07-21  3:56 ` jackysliu
  2025-07-21  6:05 ` [PATCH] dm-raid: do not include dm-core.h Yu Kuai
  2025-07-21  9:15 ` Mikulas Patocka
  2 siblings, 0 replies; 7+ messages in thread
From: jackysliu @ 2025-07-21  3:56 UTC (permalink / raw)
  To: ptikhomirov
  Cc: agk, den, dm-devel, khorenko, linux-kernel, mpatocka, snitzer,
	xni, yukuai3, Siyang 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 | 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2] arch: fix resource leak in jbusmc_probe()
  2025-07-18 13:43 [PATCH] arch: fix resource leak in chmc.c Markus Elfring
@ 2025-07-21  3:57 ` jackysliu
  2025-07-21  7:18   ` Markus Elfring
  2025-07-21  8:25   ` Krzysztof Kozlowski
  0 siblings, 2 replies; 7+ messages in thread
From: jackysliu @ 2025-07-21  3:57 UTC (permalink / raw)
  To: markus.elfring; +Cc: 1972843537, andreas, davem, linux-kernel, sparclinux

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] dm-raid: do not include dm-core.h
  2025-07-21  3:49 [PATCH] dm-raid: do not include dm-core.h Pavel Tikhomirov
  2025-07-21  3:56 ` [PATCH v2] arch: fix resource leak in jbusmc_probe() jackysliu
@ 2025-07-21  6:05 ` Yu Kuai
  2025-07-21  9:15 ` Mikulas Patocka
  2 siblings, 0 replies; 7+ messages in thread
From: Yu Kuai @ 2025-07-21  6:05 UTC (permalink / raw)
  To: Pavel Tikhomirov, Alasdair Kergon, Mike Snitzer, Mikulas Patocka
  Cc: Xiao Ni, dm-devel, linux-kernel, Konstantin Khorenko, Denis Lunev,
	yukuai (C)

在 2025/07/21 11:49, Pavel Tikhomirov 写道:
> In commit 4cc96131afce ("dm: move request-based code out to dm-rq.[hc]")
> we have a note: "DM targets should _never_ include dm-core.h!". And it
> is not used in any DM targets except dm-raid now, so let's remove it
> from dm-raid for consistency, also use special helpers instead of
> accessing dm_table and mapper_device fields directly. This change is
> merely a cleanup and should not affect functionality.
> 
> Fixes: 7168be3c8a6b ("md: record dm-raid gendisk in mddev")
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> ---
>   drivers/md/dm-raid.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
Reviewed-by: Yu Kuai <yukuai3@huawei.com>
Thanks

> diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
> index e8c0a8c6fb51..4fb5ddf50560 100644
> --- a/drivers/md/dm-raid.c
> +++ b/drivers/md/dm-raid.c
> @@ -14,7 +14,6 @@
>   #include "raid5.h"
>   #include "raid10.h"
>   #include "md-bitmap.h"
> -#include "dm-core.h"
>   
>   #include <linux/device-mapper.h>
>   
> @@ -3305,7 +3304,7 @@ static int raid_ctr(struct dm_target *ti, unsigned int argc, char **argv)
>   
>   	/* Disable/enable discard support on raid set. */
>   	configure_discard_support(rs);
> -	rs->md.dm_gendisk = ti->table->md->disk;
> +	rs->md.dm_gendisk = dm_disk(dm_table_get_md(ti->table));
>   
>   	mddev_unlock(&rs->md);
>   	return 0;
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] arch: fix resource leak in jbusmc_probe()
  2025-07-21  3:57 ` [PATCH v2] arch: fix resource leak in jbusmc_probe() jackysliu
@ 2025-07-21  7:18   ` Markus Elfring
  2025-07-21  8:25   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 7+ messages in thread
From: Markus Elfring @ 2025-07-21  7:18 UTC (permalink / raw)
  To: Siyang Liu, sparclinux; +Cc: LKML, Andreas Larsson, David S. Miller

…> 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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] arch: fix resource leak in jbusmc_probe()
  2025-07-21  3:57 ` [PATCH v2] arch: fix resource leak in jbusmc_probe() jackysliu
  2025-07-21  7:18   ` Markus Elfring
@ 2025-07-21  8:25   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 7+ messages in thread
From: Krzysztof Kozlowski @ 2025-07-21  8:25 UTC (permalink / raw)
  To: jackysliu, markus.elfring; +Cc: andreas, davem, linux-kernel, sparclinux

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] dm-raid: do not include dm-core.h
  2025-07-21  3:49 [PATCH] dm-raid: do not include dm-core.h Pavel Tikhomirov
  2025-07-21  3:56 ` [PATCH v2] arch: fix resource leak in jbusmc_probe() jackysliu
  2025-07-21  6:05 ` [PATCH] dm-raid: do not include dm-core.h Yu Kuai
@ 2025-07-21  9:15 ` Mikulas Patocka
  2 siblings, 0 replies; 7+ messages in thread
From: Mikulas Patocka @ 2025-07-21  9:15 UTC (permalink / raw)
  To: Pavel Tikhomirov
  Cc: Alasdair Kergon, Mike Snitzer, Xiao Ni, Yu Kuai, dm-devel,
	linux-kernel, Konstantin Khorenko, Denis Lunev



On Mon, 21 Jul 2025, Pavel Tikhomirov wrote:

> In commit 4cc96131afce ("dm: move request-based code out to dm-rq.[hc]")
> we have a note: "DM targets should _never_ include dm-core.h!". And it
> is not used in any DM targets except dm-raid now, so let's remove it
> from dm-raid for consistency, also use special helpers instead of
> accessing dm_table and mapper_device fields directly. This change is
> merely a cleanup and should not affect functionality.
> 
> Fixes: 7168be3c8a6b ("md: record dm-raid gendisk in mddev")
> Signed-off-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
> ---
>  drivers/md/dm-raid.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
> index e8c0a8c6fb51..4fb5ddf50560 100644
> --- a/drivers/md/dm-raid.c
> +++ b/drivers/md/dm-raid.c
> @@ -14,7 +14,6 @@
>  #include "raid5.h"
>  #include "raid10.h"
>  #include "md-bitmap.h"
> -#include "dm-core.h"
>  
>  #include <linux/device-mapper.h>
>  
> @@ -3305,7 +3304,7 @@ static int raid_ctr(struct dm_target *ti, unsigned int argc, char **argv)
>  
>  	/* Disable/enable discard support on raid set. */
>  	configure_discard_support(rs);
> -	rs->md.dm_gendisk = ti->table->md->disk;
> +	rs->md.dm_gendisk = dm_disk(dm_table_get_md(ti->table));
>  
>  	mddev_unlock(&rs->md);
>  	return 0;
> -- 
> 2.50.0
> 

Applied, thanks.

(I deleted the Fixes line, because it is not really a bugfix and it 
doesn't have to be backported to the stable kernel branches)

Mikulas


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-07-21  9:15 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-21  3:49 [PATCH] dm-raid: do not include dm-core.h Pavel Tikhomirov
2025-07-21  3:56 ` [PATCH v2] arch: fix resource leak in jbusmc_probe() jackysliu
2025-07-21  6:05 ` [PATCH] dm-raid: do not include dm-core.h Yu Kuai
2025-07-21  9:15 ` Mikulas Patocka
  -- strict thread matches above, loose matches on Subject: below --
2025-07-18 13:43 [PATCH] arch: fix resource leak in chmc.c Markus Elfring
2025-07-21  3:57 ` [PATCH v2] arch: fix resource leak in jbusmc_probe() jackysliu
2025-07-21  7:18   ` Markus Elfring
2025-07-21  8:25   ` Krzysztof Kozlowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).