Bug#264884: globfree() double-frees
On Fri, 2004-08-13 at 07:40, GOTO Masanori wrote:
> At Tue, 10 Aug 2004 14:33:42 -0500,
> Jeff Licquia wrote:
> > Certain kinds of problems in glob() result in a GLOB_ABORTED return
> > value. In these circumstances, the glob_t passed in is likely to
> > contain partial results (per POSIX), and thus, globfree() needs to be
> > called to prevent a memory leak.
>
> What's the essential point of /tset/LSB.os/genuts/glob/T.glob 30 ?
It tests that glob() does the right thing when permissions do not permit
the resolution of a glob request; specifically, it creates a directory
with no rights, and then attempts to do a glob() on it. If glob()
returns GLOB_ABORTED, and the error function is called the right number
of times, the test passes.
The source can be found here:
http://cvs.gforge.freestandards.org/cgi-bin/cvsweb.cgi/tests/lsb-runtime-test/modules/lsb-os/tset/LSB.os/genuts/glob/glob.c?rev=1.1&contenttype=text/x-cvsweb-markup&cvsroot=lsb
Look for the function "test30".
Under current glibc, this test loops infinitely. Eventually, I would
expect it to SIGSEGV, but I have not seem this behavior even when the
test is allowed to run overnight. In other tests I've done, I've seen
the SIGSEGV. Regardless, aborting the test causes the test journal to
report the abort, and test 31 is not run. From debugging, the
double-free occurs at the end of test 29 ("test29"); the test30 freeze
is a delayed reaction to the problem in test29, occurring at the
occasion of the first call to malloc() after test29 ends.
With my patch, the test passes; I've observed run times for all 31 tests
of about two seconds.
> > Unfortunately, glob() itself calls globfree() under certain
> > circumstances. Calling globfree() again (which is legal and in fact
> > mandated under POSIX) causes certain portions of the structure to be
> > double-freed. Under many circumstances, this results in infinite loops
> > or SIGSEGV during the next malloc.
>
> Which documentation (and line number) is "mandated under POSIX"
> described?
Hmm. I thought it was POSIX, but I can only find it in SUSv3.
Anyway, from the description of the glob() call:
"It is the caller's responsibility to create the structure pointed to by
pglob. The glob() function shall allocate other space as needed,
including the memory pointed to by gl_pathv. The globfree() function
shall free any space associated with pglob from a previous call to
glob()."
And:
"If (*errfunc()) is called and returns non-zero, or if the GLOB_ERR flag
is set in flags, glob() shall stop the scan and return GLOB_ABORTED
after setting gl_pathc and gl_pathv in pglob to reflect the paths
already scanned. If GLOB_ERR is not set and either errfunc is a null
pointer or (*errfunc()) returns 0, the error shall be ignored."
Also:
"Note that gl_pathc and gl_pathv have meaning even if glob() fails. This
allows glob() to report partial results in the event of an error.
However, if gl_pathc is 0, gl_pathv is unspecified even if glob() did
not return an error."
Thus, if there is an error, partial results are returned, with memory
allocated that must be freed by globfree(). Strictly speaking, calling
globfree() in this case is not mandated, if you consider a memory leak
acceptable.
Since glob() itself calls globfree() under certain circumstances in
glibc's implementation, globfree() must be able to handle double calls
for the same glob_t, which it does not do without my patch.
It's possible that this problem is made worse by the other glob() issue
I filed at the same time (bug 264887), and that the problem might be
solved by fixing that bug and not this one, but I'm not certain that
glibc's current inability to handle double-globfree() is correct even if
the test is made to pass by fixing 264887. SUSv3 does not contain a
mandate that users must test glob_t->gl_pathc before calling globfree(),
nor does it forbid double-globfree().
Reply to: