From mboxrd@z Thu Jan  1 00:00:00 1970
From: Dave Littell <littelld@verizon.net>
Date: Mon, 17 Mar 2008 18:10:08 -0500
Subject: [U-Boot-Users] Possible memory leak in cpu/ppc4xx/4xx_enet.c
In-Reply-To: <200803170956.01571.sr@denx.de>
References: <47DB37FD.9000705@verizon.net> <200803150707.44465.sr@denx.de>
	<47DBE76D.9040706@verizon.net> <200803170956.01571.sr@denx.de>
Message-ID: <47DEFA50.4080601@verizon.net>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Stefan Roese wrote:
> On Saturday 15 March 2008, Dave Littell wrote:
>>>> I looked around this message and it seems that ppc_4xx_eth_initialize()
>>>> malloc()?s a EMAC_4XX_HW_PST structure and assigns it to dev->priv.
>>>> However, nothing ever frees this buffer so standing on a ping will
>>>> ultimately pull down all the available memory.
>>> Hmmm. IIRC ppc_4xx_eth_initialize() is only called once upon bootup for
>>> ethernet driver initialization. The function called each time before
>>> network action is ppc_4xx_eth_init().
>> It looks like ppc_4xx_eth_initialize() is called from eth_init() and
>> ppc_4xx_eth_init() is only called if there's an emac0_dev.
>>
>> Hm?
> 
> I see. You don't have CONFIG_NET_MULTI defined. Which 4xx platform are we 
> talking about btw?
> 

This is a platform similar to the AMCC sequoia, but CONFIG_NET_MULTI is
not defined.

>>>> One fix might be to free( dev->priv ) at the end of ppc_4xx_eth_halt().
>>>>  I tried this today and noted no ill effects.
>>> So did it work for you?
>> It seems to be working well.
> 
> OK. But your patch will most likely break all platforms with CONFIG_NET_MULTI 
> defined. Please give the attached patch a try and let me know if it works 
> too. I'll push it online if it fixes the problem.
> 

It does indeed seem to work - thanks!

> diff --git a/cpu/ppc4xx/4xx_enet.c b/cpu/ppc4xx/4xx_enet.c
> index d990250..479cb40 100644
> --- a/cpu/ppc4xx/4xx_enet.c
> +++ b/cpu/ppc4xx/4xx_enet.c
> @@ -1998,9 +1998,12 @@ int ppc_4xx_eth_initialize (bd_t * bis)
>  }
> 
>  #if !defined(CONFIG_NET_MULTI)
> -void eth_halt (void) {
> +void eth_halt (void)
> +{
>         if (emac0_dev) {
>                 ppc_4xx_eth_halt(emac0_dev);
> +               if (emac0_dev->priv)
> +                       free(emac0_dev->priv);
>                 free(emac0_dev);
>                 emac0_dev = NULL;
>         }
> 
> 

Thanks,
Dave