Bug 233294 - crash utility: a set of memory leak fixes
crash utility: a set of memory leak fixes
Status: CLOSED NOTABUG
Product: Red Hat Enterprise Linux 5
Classification: Red Hat
Component: crash (Show other bugs)
5.0
All Linux
medium Severity medium
: ---
: ---
Assigned To: Dave Anderson
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-03-21 10:40 EDT by Konstantin Khorenko
Modified: 2007-11-30 17:07 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-03-21 11:38:10 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
patch: a set of memory leak fixes (14.02 KB, patch)
2007-03-21 10:40 EDT, Konstantin Khorenko
no flags Details | Diff

  None (edit)
Description Konstantin Khorenko 2007-03-21 10:40:52 EDT
Description of problem:
SWsoft Virtuozzo/OpenVZ Linux kernel team has found several places in the
"crash" utility code which could produce memory leaks.
The leaks were found by manual code verification, attached patch is intended to
fix them.

Version-Release number of selected component (if applicable):
crash-4.0-3.21
Comment 1 Konstantin Khorenko 2007-03-21 10:40:52 EDT
Created attachment 150584 [details]
patch: a set of memory leak fixes
Comment 2 Dave Anderson 2007-03-21 11:38:10 EDT
> SWsoft Virtuozzo/OpenVZ Linux kernel team has found several places in the
> "crash" utility code which could produce memory leaks.
> The leaks were found by manual code verification, attached patch is
> intended to fix them

I have taken a quick look at your patch, and it appears to 
be concerned with buffers that were allocated with GETBUF(),
and you have added a bunch of associated FREEBUF() calls.

However, if during the course of a crash command GETBUF()
is called, it is not necessary to make associated FREEBUF() calls.  

It was designed that way, because of the setjmp() done by the
various command RESTART() calls, which can be invoked in several places
during a crash command, most notably by error(FATAL, ...) calls.
Therefore, in fatal error situations, the code does not unwind
back through to the original "cmd_xxx()" function.  That being
the case, if malloc() was used to allocate temporary buffers, 
then yes, there would be memory leaks.

To avoid that possibility, the GETBUF() interface was designed
as a self-contained memory allocator.  Although it does offer a
FREEBUF() function, it is not necessary to call it.  That is 
because the crash utility's memory allocator keeps track of all
outstanding GETBUF() calls.  Prior to the *next* crash command,
get_command_line() calls restore_sanity(), which calls free_all_bufs().
The free_all_bufs() frees all of the temporary buffers that were
previously allocated by GETBUF().

I appreciate your looking into this, and if you find a real memory
leak, please report it.

Thanks,
  Dave Anderson

Comment 3 Dave Anderson 2007-03-21 14:18:51 EDT
Hi Konstantin,

I should mention that there is a valid reason for FREEBUF().

The internal allocator maintains 31 static buffers, and if 
all of them get allocated in the course of a crash command,
the allocator then does up to 2000 tracked malloc() calls.
But if a command attempts more than 2031 GETBUF() calls
without doing a FREEBUF(), the command will fail.

In any case, the whole point of the GETBUF() design was to
avoid memory leaks.

Thanks,
  Dave




Note You need to log in before you can comment on or make changes to this bug.