GNU bug report logs - #31439
Possible memory leak in fts.c

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: isedev <at> gmail.com
Subject: bug#31439: closed ([PATCH] fts: avoid a memory leak edge case)
Date: Mon, 14 May 2018 01:52:02 +0000

[Message part 1 (text/plain, inline)]

Your bug report

#31439: Possible memory leak in fts.c

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 31439 <at> debbugs.gnu.org.

-- 
31439: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=31439
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

From: Pádraig Brady <P <at> draigBrady.com>
To: isedev <at> gmail.com, 31439-done <at> debbugs.gnu.org,
 bug-gnulib <bug-gnulib <at> gnu.org>
Subject: [PATCH] fts: avoid a memory leak edge case
Date: Sun, 13 May 2018 18:51:02 -0700

[Message part 3 (text/plain, inline)]

On 12/05/18 18:50, ISE Development wrote:
> Hi,
> 
> I may be wrong but I suspect there is a corner case where fts_close()
> will not free the FTSENT structures correctly if called immediately
> after fts_open().
> 
> After fts_open(), the current entry is a dummy entry created as
> follows:
> 
> if ((sp->fts_cur = fts_alloc(sp, "", 0)) == NULL)
>         goto mem3;
> sp->fts_cur->fts_link = root;
> sp->fts_cur->fts_info = FTS_INIT;
> 
> It would normally be freed during the first invocation of fts_read().
> 
> In fts_close():
> 
> if (sp->fts_cur) {
>         for (p = sp->fts_cur; p->fts_level >= FTS_ROOTLEVEL;) {
>                 freep = p;
>                 p = p->fts_link != NULL ? p->fts_link : p->fts_parent;
>                 free(freep);
>         }
>         free(p);
> }
> 
> However, fts_alloc() does not clear or set fts_level, nor does it zero
> the entire FTSENT structure.
> 
> So as far as I can figure, it is possible for the fts_level of the
> dummy entry to be negative after fts_open() causing fts_close() not to
> free the actual root level entries.

Yes valgrind indicates that fts_level is uninitialized if you
fts_close() right after fts_open().
The attached should fix it up.

thanks!
Pádraig

[fts-dealloc.patch (text/x-patch, attachment)]

[Message part 5 (message/rfc822, inline)]

From: ISE Development <isedev <at> gmail.com>
To: bug-coreutils <at> gnu.org
Subject: Possible memory leak in fts.c
Date: Sun, 13 May 2018 02:50:34 +0100

Hi,

I may be wrong but I suspect there is a corner case where fts_close()
will not free the FTSENT structures correctly if called immediately
after fts_open().

After fts_open(), the current entry is a dummy entry created as
follows:

if ((sp->fts_cur = fts_alloc(sp, "", 0)) == NULL)
        goto mem3;
sp->fts_cur->fts_link = root;
sp->fts_cur->fts_info = FTS_INIT;

It would normally be freed during the first invocation of fts_read().

In fts_close():

if (sp->fts_cur) {
        for (p = sp->fts_cur; p->fts_level >= FTS_ROOTLEVEL;) {
                freep = p;
                p = p->fts_link != NULL ? p->fts_link : p->fts_parent;
                free(freep);
        }
        free(p);
}

However, fts_alloc() does not clear or set fts_level, nor does it zero
the entire FTSENT structure.

So as far as I can figure, it is possible for the fts_level of the
dummy entry to be negative after fts_open() causing fts_close() not to
free the actual root level entries.

-- isedev

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.