Messages in this thread | | | Date | Fri, 17 Jun 2022 22:29:01 +0800 (CST) | From | "Liang He" <> | Subject | Re:Re: [PATCH] powerpc: powernv: Fix refcount leak bug in opal-powercap |
| |
At 2022-06-17 13:01:27, "Christophe JAILLET" <christophe.jaillet@wanadoo.fr> wrote: >Le 17/06/2022 à 06:20, Liang He a écrit : >> In opal_powercap_init(), of_find_compatible_node() will return >> a node pointer with refcount incremented. We should use of_node_put() >> in fail path or when it is not used anymore. >> >> Besides, for_each_child_of_node() will automatically *inc* and *dec* >> refcount during iteration. However, we should add the of_node_put() >> if there is a break. > >Hi, > >I'm not sure that your patch is right here. Because of this *inc* and >*dec* things, do we still need to of_node_put(powercap) once we have >entered for_each_child_of_node? > >I think that this reference will be released on the first iteration of >the loop. > > >Maybe of_node_put(powercap) should be duplicated everywhere it is >relevant and removed from the error handling path? >Or an additional reference should be taken before the loop? >Or adding a new label with "powercap = NULL" and branching there when >needed? > >CJ > >> >> Signed-off-by: Liang He <windhl@126.com> >> --- >> arch/powerpc/platforms/powernv/opal-powercap.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/arch/powerpc/platforms/powernv/opal-powercap.c b/arch/powerpc/platforms/powernv/opal-powercap.c >> index 64506b46e77b..b102477d3f95 100644 >> --- a/arch/powerpc/platforms/powernv/opal-powercap.c >> +++ b/arch/powerpc/platforms/powernv/opal-powercap.c >> @@ -153,7 +153,7 @@ void __init opal_powercap_init(void) >> pcaps = kcalloc(of_get_child_count(powercap), sizeof(*pcaps), >> GFP_KERNEL); >> if (!pcaps) >> - return; >> + goto out_powercap; >> >> powercap_kobj = kobject_create_and_add("powercap", opal_kobj); >> if (!powercap_kobj) { >> @@ -236,6 +236,9 @@ void __init opal_powercap_init(void) >> kfree(pcaps[i].pg.name); >> } >> kobject_put(powercap_kobj); >> + of_node_put(node); >> out_pcaps: >> kfree(pcaps); >> +out_powercap: >> + of_node_put(powercap); >> }
Hi, CJ.
I think my patch is correct based on the old commit: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.19-rc2&id=09700c504d8e63faffd2a2235074e8c5d130cb8f
Bugs and fix solutions in this 09700c504d8e63-commit are very similar with mine.
Besides, I also find similar new bugs in other two files in the same directory 'powernv', so I have merged all three files' patches into one commit. '[PATCH v2] powerpc: powernv: Fix refcount leak bug'.
Thanks.
Liang
| |