[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#264884: globfree() double-frees


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
		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);
		if (rtval != GLOB_NOSYS || err != ENOSYS)
			if( rtval != GLOB_ABORTED )
				in_rpt("glob(\"%s\", GLOB_ERR, NULL, pglob) returned %d instead of GLOB_ABORTED",
					pattern, rtval);
	 * 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);


> > > 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...

-- gotom

Reply to: