Re: Re: [PATCH 5/5] powerpc/8xx: fix possible object reference leak
Hi, Christophe,
>> The call to of_find_compatible_node returns a node pointer with refcount
>> incremented thus it must be explicitly decremented after the last
>> usage.
>> irq_domain_add_linear also calls of_node_get to increase refcount,
>> so irq_domain will not be affected when it is released.
>
>
> Should you have a:
>
> Fixes: a8db8cf0d894 ("irq_domain: Replace irq_alloc_host() with
> revmap-specific initializers")
>
> If not, it means your change is in contradiction with commit
> b1725c9319aa ("[POWERPC] arch/powerpc/sysdev: Add missing of_node_put")
Thank you very much.
This problem existed before this commit (a8db8cf0d894).
and the SmPL we used is:
https://lkml.org/lkml/2019/3/14/880
We are still improving this SmPL, and it is somewhat different from the script
used by b1725c9319aa ("[POWERPC] arch/powerpc/sysdev: Add missing of_node_put").
>> Detected by coccinelle with the following warnings:
>> ./arch/powerpc/platforms/8xx/pic.c:158:1-7: ERROR: missing of_node_put;
>> acquired a node pointer with refcount incremented on line 136, but without a
>> corresponding object release within this function.
>>
>> arch/powerpc/platforms/8xx/pic.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/powerpc/platforms/8xx/pic.c
>> b/arch/powerpc/platforms/8xx/pic.c
>> index 8d5a25d..13d880b 100644
>> --- a/arch/powerpc/platforms/8xx/pic.c
>> +++ b/arch/powerpc/platforms/8xx/pic.c
>> @@ -155,6 +155,7 @@ int mpc8xx_pic_init(void)
>> ret = -ENOMEM;
>> goto out;
>> }
>> +of_node_put(np);
>> return 0;
>>
>> out:
>>
>
> I guess it would be better as follows:
>
> --- a/arch/powerpc/platforms/8xx/pic.c
> +++ b/arch/powerpc/platforms/8xx/pic.c
> @@ -153,9 +153,7 @@ int mpc8xx_pic_init(void)
> if (mpc8xx_pic_host == NULL) {
> printk(KERN_ERR "MPC8xx PIC: failed to allocate irq
> host!\n");
> ret = -ENOMEM;
> - goto out;
> }
> - return 0;
>
> out:
> of_node_put(np);
OK.
Thank you.
We will fix it soon.
Thanks and regards,
Wen
Re: [PATCH 5/5] powerpc/8xx: fix possible object reference leak
On 03/22/2019 03:05 AM, Wen Yang wrote:
The call to of_find_compatible_node returns a node pointer with refcount
incremented thus it must be explicitly decremented after the last
usage.
irq_domain_add_linear also calls of_node_get to increase refcount,
so irq_domain will not be affected when it is released.
Should you have a:
Fixes: a8db8cf0d894 ("irq_domain: Replace irq_alloc_host() with
revmap-specific initializers")
If not, it means your change is in contradiction with commit
b1725c9319aa ("[POWERPC] arch/powerpc/sysdev: Add missing of_node_put")
Detected by coccinelle with the following warnings:
./arch/powerpc/platforms/8xx/pic.c:158:1-7: ERROR: missing of_node_put;
acquired a node pointer with refcount incremented on line 136, but without a
corresponding object release within this function.
Signed-off-by: Wen Yang
Cc: Vitaly Bordug
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: Michael Ellerman
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-ker...@vger.kernel.org
---
arch/powerpc/platforms/8xx/pic.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/platforms/8xx/pic.c b/arch/powerpc/platforms/8xx/pic.c
index 8d5a25d..13d880b 100644
--- a/arch/powerpc/platforms/8xx/pic.c
+++ b/arch/powerpc/platforms/8xx/pic.c
@@ -155,6 +155,7 @@ int mpc8xx_pic_init(void)
ret = -ENOMEM;
goto out;
}
+ of_node_put(np);
return 0;
out:
I guess it would be better as follows:
--- a/arch/powerpc/platforms/8xx/pic.c
+++ b/arch/powerpc/platforms/8xx/pic.c
@@ -153,9 +153,7 @@ int mpc8xx_pic_init(void)
if (mpc8xx_pic_host == NULL) {
printk(KERN_ERR "MPC8xx PIC: failed to allocate irq
host!\n");
ret = -ENOMEM;
- goto out;
}
- return 0;
out:
of_node_put(np);
Christophe
[PATCH 5/5] powerpc/8xx: fix possible object reference leak
The call to of_find_compatible_node returns a node pointer with refcount incremented thus it must be explicitly decremented after the last usage. irq_domain_add_linear also calls of_node_get to increase refcount, so irq_domain will not be affected when it is released. Detected by coccinelle with the following warnings: ./arch/powerpc/platforms/8xx/pic.c:158:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 136, but without a corresponding object release within this function. Signed-off-by: Wen Yang Cc: Vitaly Bordug Cc: Benjamin Herrenschmidt Cc: Paul Mackerras Cc: Michael Ellerman Cc: linuxppc-dev@lists.ozlabs.org Cc: linux-ker...@vger.kernel.org --- arch/powerpc/platforms/8xx/pic.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/platforms/8xx/pic.c b/arch/powerpc/platforms/8xx/pic.c index 8d5a25d..13d880b 100644 --- a/arch/powerpc/platforms/8xx/pic.c +++ b/arch/powerpc/platforms/8xx/pic.c @@ -155,6 +155,7 @@ int mpc8xx_pic_init(void) ret = -ENOMEM; goto out; } + of_node_put(np); return 0; out: -- 2.9.5
