[Bug regex/14780] New: [PATCH] handle malloc() and realloc() failures in regcomp()

[Bug regex/14780] New: [PATCH] handle malloc() and realloc() failures in regcomp()

[makovick at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14780

             Bug #: 14780
           Summary: [PATCH] handle malloc() and realloc() failures in
                    regcomp()
           Product: glibc
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: regex
        AssignedTo: unassigned@sourceware.org
        ReportedBy: makovick@gmail.com
                CC: drepper.fsp@gmail.com
    Classification: Unclassified


Created attachment 6705
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6705
Patch for regcomp.c

Hi,

currently, regcomp() misses a lot of checks for memory allocation
failures, and it also does not properly release memory on error paths.
This means a malloc error usually causes either a SEGV or a memory
leak.

The attached patch (regex.diff) adds the return value checks and
memory deallocation on failures.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug regex/14780] [PATCH] handle malloc() and realloc() failures in regcomp()

[makovick at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14780

--- Comment #1 from Jindrich Makovicka <makovick at gmail dot com> 2012-10-28 13:27:55 UTC ---
Created attachment 6706
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6706
patch w/ malloc() fuzzing, used for testing

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug regex/14780] [PATCH] handle malloc() and realloc() failures in regcomp()

[makovick at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14780

--- Comment #2 from Jindrich Makovicka <makovick at gmail dot com> 2012-10-28 13:28:28 UTC ---
Created attachment 6707
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6707
test case

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug regex/14780] [PATCH] handle malloc() and realloc() failures in regcomp()

[makovick at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14780

--- Comment #3 from Jindrich Makovicka <makovick at gmail dot com> 2012-10-28 13:28:54 UTC ---
I have been debugging this issue by fuzzing re_malloc() and
re_realloc(), making them randomly return NULL. The patch with added
fuzzing is attached as regex-fuzzed.diff . testcase.c has been used to
exercise the modified regcomp().
Memory violations or leaks have been tested using valgrind: valgrind
--leak-check=full --show-reachable=yes --trace-children=yes
./testrun.sh ./testcase

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug regex/14780] [PATCH] handle malloc() and realloc() failures in regcomp()

[makovick at gmail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14780

Jindrich Makovicka <makovick at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #6705|0                           |1
        is obsolete|                            |

--- Comment #4 from Jindrich Makovicka <makovick at gmail dot com> 2012-10-28 14:42:10 UTC ---
Created attachment 6708
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6708
handle malloc() and realloc() failures in regcomp()

patch updated for current glibc git

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug regex/14780] [PATCH] handle malloc() and realloc() failures in regcomp()

[siddhesh at redhat dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=14780

Siddhesh Poyarekar <siddhesh at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |siddhesh at redhat dot com

--- Comment #5 from Siddhesh Poyarekar <siddhesh at redhat dot com> 2013-01-17 14:10:10 UTC ---
Thanks for the patch.  Please use the following wiki document as a guideline:

http://sourceware.org/glibc/wiki/Contribution%20checklist

and post your patch for review on the libc-alpha mailing list:

http://www.gnu.org/software/libc/development.html

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug regex/14780] [PATCH] handle malloc() and realloc() failures in regcomp()

[bugdal at aerifal dot cx]

https://sourceware.org/bugzilla/show_bug.cgi?id=14780

Rich Felker <bugdal at aerifal dot cx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugdal at aerifal dot cx

--- Comment #6 from Rich Felker <bugdal at aerifal dot cx> ---
Are there any actual cases where malloc failure is not checked? I reviewed
regcomp.c briefly and it seems the result is eventually (just not immediately)
checked before use. However, there are major leaks when malloc has failed,
since multiple results are checked together and no effort is made to free the
ones that did succeed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug regex/14780] [PATCH] handle malloc() and realloc() failures in regcomp()

[makovick at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=14780

--- Comment #7 from Jindrich Makovicka <makovick at gmail dot com> ---
(In reply to Rich Felker from comment #6)
> Are there any actual cases where malloc failure is not checked? I reviewed
> regcomp.c briefly and it seems the result is eventually (just not
> immediately) checked before use. However, there are major leaks when malloc
> has failed, since multiple results are checked together and no effort is
> made to free the ones that did succeed.

I do not really recall anymore if there _really_ was a segfault, or it was only
caused when I tried to free such partially compiled regex using regfree(). But
you can insert the fuzzing code from the first patch, consisting of xxmalloc
and xxrealloc from regcomp.c and #defines from regcomp.h, and run the attached
testcase with, say, 100000 iterations and look what happens.

The memory leaks are obviously real, and were the main reason I was looking
into this.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug regex/14780] [PATCH] handle malloc() and realloc() failures in regcomp()

[bugdal at aerifal dot cx]

https://sourceware.org/bugzilla/show_bug.cgi?id=14780

--- Comment #8 from Rich Felker <bugdal at aerifal dot cx> ---
If regcomp() returned failure, passing the regex to regfree() is invalid, so
crashes there would not be a bug. In any case, the memory leaks are a bug and
should be fixed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug regex/14780] [PATCH] handle malloc() and realloc() failures in regcomp()

[neleai at seznam dot cz]

https://sourceware.org/bugzilla/show_bug.cgi?id=14780

Ondrej Bilka <neleai at seznam dot cz> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |WAITING
                 CC|                            |neleai at seznam dot cz

--- Comment #9 from Ondrej Bilka <neleai at seznam dot cz> ---
Did you send this patch to libc-alpha?

-- 
You are receiving this mail because:
You are on the CC list for the bug.