Thread (2 messages) 2 messages, 2 authors, 2022-07-29

Re: [PATCH v2] powerpc/85xx: Fix reference leak in xes_mpc85xx_setup_arch

From: Michael Ellerman <mpe@ellerman.id.au>
Date: 2022-07-29 06:53:51
Also in: lkml

Miaoqian Lin [off-list ref] writes:
quoted hunk ↗ jump to hunk
of_find_node_by_path() returns remote device nodepointer with
refcount incremented, we should use of_node_put() on it when done.
Add missing of_node_put() to avoid refcount leak.

Fixes: 3038acf9091f ("powerpc/85xx: Add platform support for X-ES MPC85xx boards")
Signed-off-by: Miaoqian Lin <redacted>
---
changes in v2:
- update Fixes tag.
v1 Link: https://lore.kernel.org/all/20220603120907.19999-1-linmq006@gmail.com (local)
---
 arch/powerpc/platforms/85xx/xes_mpc85xx.c | 2 ++
 1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/platforms/85xx/xes_mpc85xx.c b/arch/powerpc/platforms/85xx/xes_mpc85xx.c
index 5836e4ecb7a0..93f67b430714 100644
--- a/arch/powerpc/platforms/85xx/xes_mpc85xx.c
+++ b/arch/powerpc/platforms/85xx/xes_mpc85xx.c
@@ -130,6 +130,8 @@ static void __init xes_mpc85xx_setup_arch(void)
 	mpc85xx_smp_init();
 
 	fsl_pci_assign_primary();
+
+	of_node_put(root);
 }
For context, here is the full function:

static void __init xes_mpc85xx_setup_arch(void)
{
	struct device_node *root;
	const char *model = "Unknown";

	root = of_find_node_by_path("/");
	if (root == NULL)
		return;

	model = of_get_property(root, "model", NULL);

	printk(KERN_INFO "X-ES MPC85xx-based single-board computer: %s\n",
	       model + strlen("xes,"));

	xes_mpc85xx_fixups();

	mpc85xx_smp_init();

	fsl_pci_assign_primary();
}


So yes it's missing an of_node_put(root).

But rather than add the of_node_put(), it would be simpler to just use
of_root directly, then it doesn't need an of_node_put() at all.

But then look closer. To begin with model is assigned "Unknown", but
then unconditionally overwritten by the of_get_property() call. So
that's a waste of space.

And then if there's no model property of_get_property() will return
NULL, and then the model + strlen("xes,") would oops.

And all of that just to print the model at boot, which is not really
necessary, anyone who cares can look it up in /proc/device-tree
after the system has booted.

So please just remove the lookup and printing of model entirely.

cheers
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help