[PATCH v2] arm/xen: Fix some refcount leaks
The of_find_compatible_node() function returns a node pointer with refcount incremented, We should use of_node_put() on it when done Add the missing of_node_put() to release the refcount. Fixes: 9b08aaa3199a ("ARM: XEN: Move xen_early_init() before efi_init()") Fixes: b2371587fe0c ("arm/xen: Read extended regions from DT and init Xen resource") Signed-off-by: Miaoqian Lin --- changes in v2: - call of_node_put in non-error path in xen_dt_guest_init - fix same refcount leak error in arch_xen_unpopulated_init --- arch/arm/xen/enlighten.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index ec5b082f3de6..07eb69f9e7df 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -337,12 +337,15 @@ int __init arch_xen_unpopulated_init(struct resource **res) if (!nr_reg) { pr_err("No extended regions are found\n"); + of_node_put(np); return -EINVAL; } regs = kcalloc(nr_reg, sizeof(*regs), GFP_KERNEL); - if (!regs) + if (!regs) { + of_node_put(np); return -ENOMEM; + } /* * Create resource from extended regions provided by the hypervisor to be @@ -403,8 +406,8 @@ int __init arch_xen_unpopulated_init(struct resource **res) *res = _resource; err: + of_node_put(np); kfree(regs); - return rc; } #endif @@ -424,8 +427,10 @@ static void __init xen_dt_guest_init(void) if (of_address_to_resource(xen_node, GRANT_TABLE_INDEX, )) { pr_err("Xen grant table region is not found\n"); + of_node_put(xen_node); return; } + of_node_put(xen_node); xen_grant_frames = res.start; } -- 2.17.1
Re: [PATCH] arm/xen: Fix refcount leak in xen_dt_guest_init
Hi, On Fri, Mar 11, 2022 at 06:01:11PM -0800, Stefano Stabellini wrote: > On Wed, 9 Mar 2022, Miaoqian Lin wrote: > > The of_find_compatible_node() function returns a node pointer with > > refcount incremented, We should use of_node_put() on it when done > > Add the missing of_node_put() to release the refcount. > > > > Fixes: 9b08aaa3199a ("ARM: XEN: Move xen_early_init() before efi_init()") > > Signed-off-by: Miaoqian Lin > > Thanks for the patch! > > > > --- > > arch/arm/xen/enlighten.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c > > index ec5b082f3de6..262f45f686b6 100644 > > --- a/arch/arm/xen/enlighten.c > > +++ b/arch/arm/xen/enlighten.c > > @@ -424,6 +424,7 @@ static void __init xen_dt_guest_init(void) > > > > if (of_address_to_resource(xen_node, GRANT_TABLE_INDEX, )) { > > pr_err("Xen grant table region is not found\n"); > > + of_node_put(xen_node); > > return; > > } > > This is adding a call to of_node_put on the error path. Shouldn't it > be called also in the non-error path? You're right. It should be called also in the non-error path. I made a mistake. > Also, there is another instance of of_address_to_resource being called > in this file (in arch_xen_unpopulated_init), does it make sense to call > of_node_put there too? I think so, becase device node pointer np is a local variable. So the reference it taken should be released in the scope. I look into the whole codebase for this kind of usage pattern ($ret=of_find_compatible_node();of_address_to_resource($ret,_,_), $ret is a local variable), Most of them call of_node_put() when done. And document of of_find_compatible_node() also mentions > Return: A node pointer with refcount incremented, use > of_node_put() on it when done. But I am not sure, Since I am unfamiliar with other code logic. It better if the developers could double check. I found some similar cases in arch/arm.
[PATCH] arm/xen: Fix refcount leak in xen_dt_guest_init
The of_find_compatible_node() function returns a node pointer with refcount incremented, We should use of_node_put() on it when done Add the missing of_node_put() to release the refcount. Fixes: 9b08aaa3199a ("ARM: XEN: Move xen_early_init() before efi_init()") Signed-off-by: Miaoqian Lin --- arch/arm/xen/enlighten.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c index ec5b082f3de6..262f45f686b6 100644 --- a/arch/arm/xen/enlighten.c +++ b/arch/arm/xen/enlighten.c @@ -424,6 +424,7 @@ static void __init xen_dt_guest_init(void) if (of_address_to_resource(xen_node, GRANT_TABLE_INDEX, )) { pr_err("Xen grant table region is not found\n"); + of_node_put(xen_node); return; } xen_grant_frames = res.start; -- 2.17.1