[Crash-utility] [PATCH] Fix memory leak when accessing radix tree

Dave Anderson anderson at redhat.com
Mon Nov 29 14:34:53 UTC 2010


----- "Wang Chao" <wang.chao at cn.fujitsu.com> wrote:

> Hi Dave,
> 
> It seems that function do_radix_tree doesn't free dynamic
> allocated memory and in turn cause memory leak.
> Hope that the attached patch would help.
> 
> Thanks,
> Wang Chao

The do_radix_tree() function is not used by any other entity
in the crash utility, but could be used by an extension module.

It was added back in 2004 in crash version 3.8-1 -- before I kept the
changelog file -- and I'm afraid I don't even have a record of who
wrote the original version to give them credit.  It was updated slightly
by atyson at hp.com in 2008 (version 4.0-5.1), but as I recently discovered,
it doesn't work for any kernel after linux-2.6.19, because it was written
with a hardwired dependency on the radix_tree_node structure never changing,
and it had hardwired copies of the kernel's RADIX_TREE_MAP_SHIFT, 
RADIX_TREE_MAP_SIZE and RADIX_TREE_MAP_MASK #define's.

In any case, for the next crash release, I have resurrected its functionality
by re-writing it to not depend upon the radix_tree_node structure remaining
unchanged, and made the 3 #define's dynamically calculated values.  I did so
in order to use do_radix_tree() with an update the "irq" command, so that it
will work with 2.6.34 and later kernels that have replace the irq_desc[] array
with the irq_desc_tree radix tree.

Anyway, getting back to your patch, while it's certainly good practice to
call FREEBUF() for any prior GETBUF() call, in reality it's not a memory leak.
All buffers allocated with GETBUF() are *only* in effect for the lifetime of a 
particular crash command.  If any buffers allocated by GETBUF() are not explicitly
freed by FREEBUF(), they will be freed automatically by the free_all_bufs() function
before the next "crash> " prompt.  So it's impossible for them to cause a memory
leak.

But I'll certainly add the FREEBUF() call in your patch.

Thanks,
  Dave
 





More information about the Crash-utility mailing list