[xf86-video-intel] pI830->memory_manager is freed twice

Zhenyu Wang zhenyu.z.wang at intel.com
Mon Oct 8 21:16:58 PDT 2007


On 2007.10.08 18:23:31 +0000, Keith Packard wrote:
> Just want to get feedback on whether this will create some kind of nasty
> leak. I was trashing the libc memory allocator with a double free before
> I did this...
> 
> commit 751604506176b25d3262639d08680d927c566308
> Author: Keith Packard <keithp at koto.keithp.com>
> Date:   Mon Oct 8 18:22:10 2007 -0700
> 
>     Leave freeing pI830->memory_manager until we're done with it.
>     
>     pI830->memory_manager is the DRM memory management segment, and is used
>     while cleaning up the X server memory allocations. Freeing it before the
>     rest of the memory allocator is cleaned up turns out to be a bad idea.
> 
> diff --git a/src/i830_memory.c b/src/i830_memory.c
> index cdb7c47..5fc2043 100644
> --- a/src/i830_memory.c
> +++ b/src/i830_memory.c
> @@ -267,10 +267,20 @@ i830_reset_allocations(ScrnInfoPtr pScrn)
>  {
>      I830Ptr pI830 = I830PTR(pScrn);
>      int	    p;
> +    i830_memory	*mem, *this;
>  
>      /* While there is any memory between the start and end markers, free it. */
> -    while (pI830->memory_list->next->next != NULL)
> -	i830_free_memory(pScrn, pI830->memory_list->next);
> +    for (mem = pI830->memory_list->next; mem->next != NULL;)
> +    {
> +	this = mem;
> +	mem = mem->next;
> +#ifdef XF86DRI_MM
> +	/* Don't free the DRI memory manager segment; that is still needed */
> +	if (this == pI830->memory_manager)
> +	    continue;
> +#endif
> +	i830_free_memory(pScrn, this);
> +    }
>  
>      /* Free any allocations in buffer objects */
>  #ifdef XF86DRI_MM
> 
> -- 

Yeah, it also does the trick that if we failed in first try on tiled allocation,
we can't free memory_manager, as it's needed for second try. But with this we still 
might have leak here if both trys fail, we will leave memory_manager. Coz memory_manager 
is freed only in i830_allocator_fini() (called by pScreen->CloseScreen) after taking 
down ttm. As we failed in ScreenInit, we don't have chance to free it. I think this
is rare and shouldn't happen practically.



More information about the xorg mailing list