Created attachment 6705 [details] 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.
Created attachment 6706 [details] patch w/ malloc() fuzzing, used for testing
Created attachment 6707 [details] test case
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
Created attachment 6708 [details] handle malloc() and realloc() failures in regcomp() patch updated for current glibc git
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
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.
(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.
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.
Did you send this patch to libc-alpha?