* [PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
@ 2011-08-08 11:18 ` Julia Lawall
0 siblings, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2011-08-08 11:18 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: kernel-janitors, Paul Mackerras, Timur Tabi, Ashish Kalra,
Kumar Gala, linuxppc-dev, linux-kernel
From: Julia Lawall <julia@diku.dk>
At this point, ehv_pic has been allocated but not stored anywhere, so it
should be freed before leaving the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@exists@
local idexpression x;
statement S,S1;
expression E;
identifier fl;
expression *ptr != NULL;
@@
x = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x == NULL) S
<... when != x
when != if (...) { <+...kfree(x)...+> }
when any
when != true x == NULL
x->fl
...>
(
if (x == NULL) S1
|
if (...) { ... when != x
when forall
(
return \(0\|<+...x...+>\|ptr\);
|
* return ...;
)
}
)
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
arch/powerpc/sysdev/ehv_pic.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/sysdev/ehv_pic.c b/arch/powerpc/sysdev/ehv_pic.c
index af1a5df..b6731e4 100644
--- a/arch/powerpc/sysdev/ehv_pic.c
+++ b/arch/powerpc/sysdev/ehv_pic.c
@@ -280,6 +280,7 @@ void __init ehv_pic_init(void)
if (!ehv_pic->irqhost) {
of_node_put(np);
+ kfree(ehv_pic);
return;
}
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
@ 2011-08-08 11:18 ` Julia Lawall
0 siblings, 0 replies; 15+ messages in thread
From: Julia Lawall @ 2011-08-08 11:18 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: kernel-janitors, Ashish Kalra, linux-kernel, Paul Mackerras,
linuxppc-dev, Timur Tabi
From: Julia Lawall <julia@diku.dk>
At this point, ehv_pic has been allocated but not stored anywhere, so it
should be freed before leaving the function.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@exists@
local idexpression x;
statement S,S1;
expression E;
identifier fl;
expression *ptr != NULL;
@@
x = \(kmalloc\|kzalloc\|kcalloc\)(...);
...
if (x == NULL) S
<... when != x
when != if (...) { <+...kfree(x)...+> }
when any
when != true x == NULL
x->fl
...>
(
if (x == NULL) S1
|
if (...) { ... when != x
when forall
(
return \(0\|<+...x...+>\|ptr\);
|
* return ...;
)
}
)
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
arch/powerpc/sysdev/ehv_pic.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/powerpc/sysdev/ehv_pic.c b/arch/powerpc/sysdev/ehv_pic.c
index af1a5df..b6731e4 100644
--- a/arch/powerpc/sysdev/ehv_pic.c
+++ b/arch/powerpc/sysdev/ehv_pic.c
@@ -280,6 +280,7 @@ void __init ehv_pic_init(void)
if (!ehv_pic->irqhost) {
of_node_put(np);
+ kfree(ehv_pic);
return;
}
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
2011-08-08 11:18 ` Julia Lawall
(?)
@ 2011-08-15 22:55 ` Tabi Timur-B04825
-1 siblings, 0 replies; 15+ messages in thread
From: Tabi Timur-B04825 @ 2011-08-15 22:55 UTC (permalink / raw)
To: Julia Lawall, Benjamin Herrenschmidt, Kumar Gala
Cc: Paul Mackerras, kernel-janitors@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
On Mon, Aug 8, 2011 at 7:18 AM, Julia Lawall <julia@diku.dk> wrote:
> diff --git a/arch/powerpc/sysdev/ehv_pic.c b/arch/powerpc/sysdev/ehv_pic.c
> index af1a5df..b6731e4 100644
> --- a/arch/powerpc/sysdev/ehv_pic.c
> +++ b/arch/powerpc/sysdev/ehv_pic.c
> @@ -280,6 +280,7 @@ void __init ehv_pic_init(void)
>
> if (!ehv_pic->irqhost) {
> of_node_put(np);
> + kfree(ehv_pic);
> return;
> }
Although the fix is correct, I think there is another bug in this
function. 'np' is not released when the function finishes
successfully. I've looked at other functions that use
irq_alloc_host(), and most of them do the same thing: they don't call
of_node_put() on the device node pointer. The only exception I've
found is mpc5121_ads_cpld_pic_init().
Ben, Kumar: am I missing something? irq_alloc_host() calls of_node_get():
host->of_node = of_node_get(of_node);
so doesn't that mean that the caller of irq_alloc_host() should
release the device node pointer?
--
Timur Tabi
Linux kernel developer at Freescale
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
@ 2011-08-15 22:55 ` Tabi Timur-B04825
0 siblings, 0 replies; 15+ messages in thread
From: Tabi Timur-B04825 @ 2011-08-15 22:55 UTC (permalink / raw)
To: Julia Lawall, Benjamin Herrenschmidt, Kumar Gala
Cc: kernel-janitors@vger.kernel.org, Paul Mackerras,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
On Mon, Aug 8, 2011 at 7:18 AM, Julia Lawall <julia@diku.dk> wrote:
> diff --git a/arch/powerpc/sysdev/ehv_pic.c b/arch/powerpc/sysdev/ehv_pic.c
> index af1a5df..b6731e4 100644
> --- a/arch/powerpc/sysdev/ehv_pic.c
> +++ b/arch/powerpc/sysdev/ehv_pic.c
> @@ -280,6 +280,7 @@ void __init ehv_pic_init(void)
>
> if (!ehv_pic->irqhost) {
> of_node_put(np);
> + kfree(ehv_pic);
> return;
> }
Although the fix is correct, I think there is another bug in this
function. 'np' is not released when the function finishes
successfully. I've looked at other functions that use
irq_alloc_host(), and most of them do the same thing: they don't call
of_node_put() on the device node pointer. The only exception I've
found is mpc5121_ads_cpld_pic_init().
Ben, Kumar: am I missing something? irq_alloc_host() calls of_node_get():
host->of_node = of_node_get(of_node);
so doesn't that mean that the caller of irq_alloc_host() should
release the device node pointer?
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
@ 2011-08-15 22:55 ` Tabi Timur-B04825
0 siblings, 0 replies; 15+ messages in thread
From: Tabi Timur-B04825 @ 2011-08-15 22:55 UTC (permalink / raw)
To: Julia Lawall, Benjamin Herrenschmidt, Kumar Gala
Cc: Paul Mackerras, kernel-janitors@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
On Mon, Aug 8, 2011 at 7:18 AM, Julia Lawall <julia@diku.dk> wrote:
> diff --git a/arch/powerpc/sysdev/ehv_pic.c b/arch/powerpc/sysdev/ehv_pic.=
c
> index af1a5df..b6731e4 100644
> --- a/arch/powerpc/sysdev/ehv_pic.c
> +++ b/arch/powerpc/sysdev/ehv_pic.c
> @@ -280,6 +280,7 @@ void __init ehv_pic_init(void)
>
> =A0 =A0 =A0 =A0if (!ehv_pic->irqhost) {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0of_node_put(np);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 kfree(ehv_pic);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return;
> =A0 =A0 =A0 =A0}
Although the fix is correct, I think there is another bug in this
function. 'np' is not released when the function finishes
successfully. I've looked at other functions that use
irq_alloc_host(), and most of them do the same thing: they don't call
of_node_put() on the device node pointer. The only exception I've
found is mpc5121_ads_cpld_pic_init().
Ben, Kumar: am I missing something? irq_alloc_host() calls of_node_get():
host->of_node =3D of_node_get(of_node);
so doesn't that mean that the caller of irq_alloc_host() should
release the device node pointer?
--=20
Timur Tabi
Linux kernel developer at Freescale=
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
2011-08-15 22:55 ` Tabi Timur-B04825
(?)
@ 2011-08-23 19:04 ` Timur Tabi
-1 siblings, 0 replies; 15+ messages in thread
From: Timur Tabi @ 2011-08-23 19:04 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Kumar Gala
Cc: kernel-janitors, Julia Lawall, linuxppc-dev, Paul Mackerras,
linux-kernel
Ben, Kumar, can one of you take a look at my question and help me out?
wrote:
> On Mon, Aug 8, 2011 at 7:18 AM, Julia Lawall <julia@diku.dk> wrote:
>
>> diff --git a/arch/powerpc/sysdev/ehv_pic.c b/arch/powerpc/sysdev/ehv_pic.c
>> index af1a5df..b6731e4 100644
>> --- a/arch/powerpc/sysdev/ehv_pic.c
>> +++ b/arch/powerpc/sysdev/ehv_pic.c
>> @@ -280,6 +280,7 @@ void __init ehv_pic_init(void)
>>
>> if (!ehv_pic->irqhost) {
>> of_node_put(np);
>> + kfree(ehv_pic);
>> return;
>> }
>
> Although the fix is correct, I think there is another bug in this
> function. 'np' is not released when the function finishes
> successfully. I've looked at other functions that use
> irq_alloc_host(), and most of them do the same thing: they don't call
> of_node_put() on the device node pointer. The only exception I've
> found is mpc5121_ads_cpld_pic_init().
>
> Ben, Kumar: am I missing something? irq_alloc_host() calls of_node_get():
>
> host->of_node = of_node_get(of_node);
>
> so doesn't that mean that the caller of irq_alloc_host() should
> release the device node pointer?
>
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
@ 2011-08-23 19:04 ` Timur Tabi
0 siblings, 0 replies; 15+ messages in thread
From: Timur Tabi @ 2011-08-23 19:04 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Kumar Gala
Cc: Julia Lawall, kernel-janitors, Paul Mackerras, linuxppc-dev,
linux-kernel
Ben, Kumar, can one of you take a look at my question and help me out?
wrote:
> On Mon, Aug 8, 2011 at 7:18 AM, Julia Lawall <julia@diku.dk> wrote:
>
>> diff --git a/arch/powerpc/sysdev/ehv_pic.c b/arch/powerpc/sysdev/ehv_pic.c
>> index af1a5df..b6731e4 100644
>> --- a/arch/powerpc/sysdev/ehv_pic.c
>> +++ b/arch/powerpc/sysdev/ehv_pic.c
>> @@ -280,6 +280,7 @@ void __init ehv_pic_init(void)
>>
>> if (!ehv_pic->irqhost) {
>> of_node_put(np);
>> + kfree(ehv_pic);
>> return;
>> }
>
> Although the fix is correct, I think there is another bug in this
> function. 'np' is not released when the function finishes
> successfully. I've looked at other functions that use
> irq_alloc_host(), and most of them do the same thing: they don't call
> of_node_put() on the device node pointer. The only exception I've
> found is mpc5121_ads_cpld_pic_init().
>
> Ben, Kumar: am I missing something? irq_alloc_host() calls of_node_get():
>
> host->of_node = of_node_get(of_node);
>
> so doesn't that mean that the caller of irq_alloc_host() should
> release the device node pointer?
>
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
@ 2011-08-23 19:04 ` Timur Tabi
0 siblings, 0 replies; 15+ messages in thread
From: Timur Tabi @ 2011-08-23 19:04 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Kumar Gala
Cc: kernel-janitors, Julia Lawall, linuxppc-dev, Paul Mackerras,
linux-kernel
Ben, Kumar, can one of you take a look at my question and help me out?
wrote:
> On Mon, Aug 8, 2011 at 7:18 AM, Julia Lawall <julia@diku.dk> wrote:
>
>> diff --git a/arch/powerpc/sysdev/ehv_pic.c b/arch/powerpc/sysdev/ehv_pic.c
>> index af1a5df..b6731e4 100644
>> --- a/arch/powerpc/sysdev/ehv_pic.c
>> +++ b/arch/powerpc/sysdev/ehv_pic.c
>> @@ -280,6 +280,7 @@ void __init ehv_pic_init(void)
>>
>> if (!ehv_pic->irqhost) {
>> of_node_put(np);
>> + kfree(ehv_pic);
>> return;
>> }
>
> Although the fix is correct, I think there is another bug in this
> function. 'np' is not released when the function finishes
> successfully. I've looked at other functions that use
> irq_alloc_host(), and most of them do the same thing: they don't call
> of_node_put() on the device node pointer. The only exception I've
> found is mpc5121_ads_cpld_pic_init().
>
> Ben, Kumar: am I missing something? irq_alloc_host() calls of_node_get():
>
> host->of_node = of_node_get(of_node);
>
> so doesn't that mean that the caller of irq_alloc_host() should
> release the device node pointer?
>
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
2011-08-08 11:18 ` Julia Lawall
(?)
@ 2011-08-23 18:38 ` Timur Tabi
-1 siblings, 0 replies; 15+ messages in thread
From: Timur Tabi @ 2011-08-23 18:38 UTC (permalink / raw)
To: Julia Lawall; +Cc: kernel-janitors, linux-kernel, Paul Mackerras, linuxppc-dev
Julia Lawall wrote:
> At this point, ehv_pic has been allocated but not stored anywhere, so it
> should be freed before leaving the function.
Acked-by: Timur Tabi <timur@freescale.com>
FYI, Ashish is no longer with Freescale, so I've taken over maintainership of
ehv_pic.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
@ 2011-08-23 18:38 ` Timur Tabi
0 siblings, 0 replies; 15+ messages in thread
From: Timur Tabi @ 2011-08-23 18:38 UTC (permalink / raw)
To: Julia Lawall
Cc: Benjamin Herrenschmidt, kernel-janitors, Paul Mackerras,
Kumar Gala, linuxppc-dev, linux-kernel
Julia Lawall wrote:
> At this point, ehv_pic has been allocated but not stored anywhere, so it
> should be freed before leaving the function.
Acked-by: Timur Tabi <timur@freescale.com>
FYI, Ashish is no longer with Freescale, so I've taken over maintainership of
ehv_pic.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
@ 2011-08-23 18:38 ` Timur Tabi
0 siblings, 0 replies; 15+ messages in thread
From: Timur Tabi @ 2011-08-23 18:38 UTC (permalink / raw)
To: Julia Lawall; +Cc: kernel-janitors, linux-kernel, Paul Mackerras, linuxppc-dev
Julia Lawall wrote:
> At this point, ehv_pic has been allocated but not stored anywhere, so it
> should be freed before leaving the function.
Acked-by: Timur Tabi <timur@freescale.com>
FYI, Ashish is no longer with Freescale, so I've taken over maintainership of
ehv_pic.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
2011-08-08 11:18 ` Julia Lawall
(?)
@ 2011-11-24 7:16 ` Kumar Gala
-1 siblings, 0 replies; 15+ messages in thread
From: Kumar Gala @ 2011-11-24 7:16 UTC (permalink / raw)
To: Julia Lawall
Cc: kernel-janitors, linux-kernel, Ashish Kalra, Paul Mackerras,
linuxppc-dev, Timur Tabi
On Aug 8, 2011, at 6:18 AM, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
>
> At this point, ehv_pic has been allocated but not stored anywhere, so it
> should be freed before leaving the function.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @exists@
> local idexpression x;
> statement S,S1;
> expression E;
> identifier fl;
> expression *ptr != NULL;
> @@
>
> x = \(kmalloc\|kzalloc\|kcalloc\)(...);
> ...
> if (x = NULL) S
> <... when != x
> when != if (...) { <+...kfree(x)...+> }
> when any
> when != true x = NULL
> x->fl
> ...>
> (
> if (x = NULL) S1
> |
> if (...) { ... when != x
> when forall
> (
> return \(0\|<+...x...+>\|ptr\);
> |
> * return ...;
> )
> }
> )
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
>
> ---
> arch/powerpc/sysdev/ehv_pic.c | 1 +
> 1 file changed, 1 insertion(+)
applied to next
- k
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
@ 2011-11-24 7:16 ` Kumar Gala
0 siblings, 0 replies; 15+ messages in thread
From: Kumar Gala @ 2011-11-24 7:16 UTC (permalink / raw)
To: Julia Lawall
Cc: Benjamin Herrenschmidt, kernel-janitors, Paul Mackerras,
Timur Tabi, Ashish Kalra, linuxppc-dev, linux-kernel
On Aug 8, 2011, at 6:18 AM, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
>
> At this point, ehv_pic has been allocated but not stored anywhere, so it
> should be freed before leaving the function.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @exists@
> local idexpression x;
> statement S,S1;
> expression E;
> identifier fl;
> expression *ptr != NULL;
> @@
>
> x = \(kmalloc\|kzalloc\|kcalloc\)(...);
> ...
> if (x == NULL) S
> <... when != x
> when != if (...) { <+...kfree(x)...+> }
> when any
> when != true x == NULL
> x->fl
> ...>
> (
> if (x == NULL) S1
> |
> if (...) { ... when != x
> when forall
> (
> return \(0\|<+...x...+>\|ptr\);
> |
> * return ...;
> )
> }
> )
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
>
> ---
> arch/powerpc/sysdev/ehv_pic.c | 1 +
> 1 file changed, 1 insertion(+)
applied to next
- k
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
@ 2011-11-24 7:16 ` Kumar Gala
0 siblings, 0 replies; 15+ messages in thread
From: Kumar Gala @ 2011-11-24 7:16 UTC (permalink / raw)
To: Julia Lawall
Cc: kernel-janitors, linux-kernel, Ashish Kalra, Paul Mackerras,
linuxppc-dev, Timur Tabi
On Aug 8, 2011, at 6:18 AM, Julia Lawall wrote:
> From: Julia Lawall <julia@diku.dk>
>
> At this point, ehv_pic has been allocated but not stored anywhere, so it
> should be freed before leaving the function.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @exists@
> local idexpression x;
> statement S,S1;
> expression E;
> identifier fl;
> expression *ptr != NULL;
> @@
>
> x = \(kmalloc\|kzalloc\|kcalloc\)(...);
> ...
> if (x == NULL) S
> <... when != x
> when != if (...) { <+...kfree(x)...+> }
> when any
> when != true x == NULL
> x->fl
> ...>
> (
> if (x == NULL) S1
> |
> if (...) { ... when != x
> when forall
> (
> return \(0\|<+...x...+>\|ptr\);
> |
> * return ...;
> )
> }
> )
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
>
> ---
> arch/powerpc/sysdev/ehv_pic.c | 1 +
> 1 file changed, 1 insertion(+)
applied to next
- k
^ permalink raw reply [flat|nested] 15+ messages in thread