• Bug#264884: globfree() double-frees

    From Jeff Licquia@1:229/2 to All on Tue Aug 10 22:00:13 2004
    XPost: linux.debian.maint.glibc
    From: [email protected]

    Package: libc6
    Version: 2.3.2.ds1-13
    Severity: serious
    Tags: patch

    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.

    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.

    The best way to fix it is for globfree() to do housekeeping on the
    glob_t it's freeing, by setting gl_pathc to 0 and gl_pathv to NULL.
    Then, when globfree() is called the second time, it knows to do
    nothing. A patch to that effect is attached (in debian/patches form).

    (This is the same bug, basically, as 260767, except that the source of
    the double-free I complained about has now been discovered.)

    The severity is serious because this bug causes the LSB tests to hang, specifically /tset/LSB.os/genuts/glob/T.glob 30.


    #! /bin/sh -e

    # All lines beginning with `# DP:' are a description of the patch.
    # DP: Description: Patch to make globfree() clear pglob->gl_pathc
    # DP: Related bugs:
    # DP: Dpatch author:
    # DP: Patch author: Jeff Licquia <[email protected]>
    # DP: Upstream status: Not submitted
    # DP: Status Details:
    # DP: Date: 2004-07-22

    PATCHLEVEL=1

    if [ $# -ne 2 ]; then
    echo >&2 "`basename $0`: script expects -patch|-unpatch as argument"
    exit 1
    fi
    case "$1" in
    -patch) patch -d "$2" -f --no-backup-if-mismatch -p$PATCHLEVEL < $0;;
    -unpatch) patch -d "$2" -f --no-backup-if-mismatch -R -p$PATCHLEVEL < $0;;
    *)
    echo >&2 "`basename $0`: script expects -patch|-unpatch as argument"
    exit 1
    esac
    exit 0

    # append the patch here and adjust the -p? flag in the patch calls.
    --- glibc-2.3.2-old/sysdeps/generic/glob.c 2004-07-26 17:49:07.000000000 -0400
    +++ glibc-2.3.2/sysdeps/generic/glob.c 2004-07-26 17:51:14.000000000 -0400
    @@ -1105,6 +1105,8 @@
    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);
    + pglob->gl_pathc = 0;
    + pglob->gl_pathv = NULL;
    }
    }
    #if defined _LIBC && !defined globfree

    --- SoupGate-Win32 v1.05
    * Origin: you cannot sedate... all the things you hate (1:229/2)
  • From GOTO Masanori@1:229/2 to Jeff Licquia on Fri Aug 13 15:00:14 2004
    XPost: linux.debian.maint.glibc
    From: [email protected]

    Hi,

    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 ?

    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?

    Regards,
    -- gotom


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

    --- SoupGate-Win32 v1.05
    * Origin: you cannot sedate... all the things you hate (1:229/2)
  • From Jeff Licquia@1:229/2 to GOTO Masanori on Fri Aug 13 21:00:26 2004
    XPost: linux.debian.maint.glibc
    From: [email protected]

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



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

    --- SoupGate-Win32 v1.05
    * Origin: you cannot sedate... all the things you hate (1:229/2)