Re: [PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
On Aug 8, 2011, at 6:18 AM, Julia Lawall wrote: > From: Julia Lawall > > 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/) > > // > @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 ...; > ) > } > ) > // > > Signed-off-by: Julia Lawall > > --- > arch/powerpc/sysdev/ehv_pic.c |1 + > 1 file changed, 1 insertion(+) applied to next - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
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 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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
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 FYI, Ashish is no longer with Freescale, so I've taken over maintainership of ehv_pic. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
On Mon, Aug 8, 2011 at 7:18 AM, Julia Lawall 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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 8/9] arch/powerpc/sysdev/ehv_pic.c: add missing kfree
From: Julia Lawall 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/) // @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 ...; ) } ) // Signed-off-by: Julia Lawall --- 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; } ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev