Bug#264884: globfree() double-frees
Hi,
Thanks for your reply.
At Fri, 13 Aug 2004 13:40:58 -0500,
Jeff Licquia wrote:
> > 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
I'm checking this bug; but it's difficult to run LSB test suite.
Which test suite did you try? If you know how to run it, please let
me know.
> 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.
Hmm, weird. I extracted test29 from cvs. After GLOB_ABORTED from
glob(), I checked the result of glob_t value and got as follows:
err: 2 (ABORTED:2) errno: 13
glob: Permission denied
gl_pathc:0, gl_pathv:0x0, gl_offs:0
So, if globfree() is called, it does not occur any problems. Could
you send me the complete test case or confirm these gl_pathv value? I
would like to test your patch and check its correctness.
My test program is (-> is my modification):
-> static void display(glob_t *g)
-> {
-> printf("gl_pathc:%d, gl_pathv:0x%x, gl_offs:%d\n",
-> g->gl_pathc, g->gl_pathv, g->gl_offs);
-> }
private void
test29()
{
...
char *pattern = "dir_1/*able/*";
glob_t glob_b;
glob_b = empty_glob;
errno = 0;
rtval = glob(pattern, GLOB_ERR, NULLFP, &glob_b);
err = errno;
-> if (rtval) {
-> printf("err: %d (ABORTED:%d) errno: %d\n", rtval, GLOB_ABORTED, errno);
-> perror("glob");
-> }
globok = 0;
nfails += chk_sup(pattern, GLOB_ERR, NULLFP, rtval, err);
PATH_FUNC_TRACE;
if (rtval != GLOB_NOSYS || err != ENOSYS)
{
if( rtval != GLOB_ABORTED )
{
nfails++;
xx_rpt(FAILURE);
in_rpt("glob(\"%s\", GLOB_ERR, NULL, pglob) returned %d instead of GLOB_ABORTED",
pattern, rtval);
}
else
PATH_TRACE;
/*
* We cant test the partial result that will be caused by aborting the
* search as there is no way of knowing the order that files and directories
* will be encountered. File creation order is not garuanteed even if it
* is what most systems do.
*/
-> display(&glob_b);
globfree(&glob_b);
...
> > > 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().
Yes, it makes sense. But I still have question about this bug. I
would like to investigate this problem more.
Currently, there's no problem, so I would like to downgrade this bug
to investigate more if there's no comments...
Regards,
-- gotom
Reply to: