On Thu, 2004-07-22 at 06:21, Andrew Josey wrote:
> (from one of my engineers...)
> 
> I believe this is a bug in the glibc globfree() function.
> 
> In version 1.49 of sysdeps/generic/glob.c I see this code in the glob()
> function:
> 
>           status = glob_in_dir (filename, dirs.gl_pathv[i],
>                                 ((flags | GLOB_APPEND)
>                                  & ~(GLOB_NOCHECK | GLOB_NOMAGIC)),
>                                 errfunc, pglob);
>           if (status == GLOB_NOMATCH)
>             /* No matches in this directory.  Try the next.  */
>             continue;
> 
>           if (status != 0)
>             {
>               globfree (&dirs);
>               globfree (pglob);
>               pglob->gl_pathc = 0;
>               return status;
>             }
> 
> When an unreadable directory is encountered, glob_in_dir() returns
> GLOB_ABORTED.  The above code then calls globfree() before returning.
> This is in itself non-compliant, as glob() is supposed to return
> partial results on error.  

In other places in the code, GLOB_APPEND is checked before globfree() is
called.  So perhaps we need to add this check here as well.  (Partial
results seem to be implemented via recursive glob() calls with
GLOB_APPEND set, so this should take care of the problem.)

But, strictly speaking, this is unrelated to the problem at hand.

> However, the double-free problem is because
> the call to globfree() in the above code frees pglob->gl_pathv, then
> when the application calls globfree() that call also tries to free
> pglob->gl_pathv.  It could be fixed by setting pglob->gl_pathv to
> NULL in globfree() after freeing it:
> 
>       void
>       globfree (pglob)
>            register glob_t *pglob;
>       {
>         if (pglob->gl_pathv != NULL)
>           {
>             size_t i;
>             for (i = 0; i < pglob->gl_pathc; ++i)
>               if (pglob->gl_pathv[pglob->gl_offs + i] != NULL)
>                 free ((__ptr_t) pglob->gl_pathv[pglob->gl_offs + i]);
>             free ((__ptr_t) pglob->gl_pathv);
> ADD ->        pglob->gl_pathv = NULL;
>           }
>       }

I suppose it's implicit to the spec that pglob's entire state is
undefined after calling globfree() on it.  (At least, I don't see any
explicit statement to that effect in SUSv3.)

If this is the case, one cannot assume the integrity of any part of
pglob, including gl_pathc.  I therefore tend to agree with Thorsten that
gl_pathc should be checked in globfree and set to zero there as well. 
See:

http://sources.redhat.com/ml/libc-hacker/2002-02/msg00095.html

This would advance the implicit statement in the spec that setting
gl_pathc to 0 is a flag that pglob is invalid.  The only disadvantage
would be the additional conditional test (for gl_pathc) over the above
patch.  But is globfree() really so performance-critical that we can't
afford it?


-- 
To UNSUBSCRIBE, email to [EMAIL PROTECTED]
with a subject of "unsubscribe". Trouble? Contact [EMAIL PROTECTED]

Reply via email to