sed segfault after commit 9c47c43e0736 with FEATURE_CLEAN_UP
Thomas De Schampheleire
patrickdepinguin at gmail.com
Mon Sep 24 14:38:09 UTC 2018
Hi,
If FEATURE_CLEAN_UP is enabled, following reproduction test case
causes a segfault:
+testing "sed uses previous regexp test2" \
+ "sed -n '/foo/ { //p }'" \
+ "" \
+ "" \
+ "q\nw\ne\nr\n"
+
(note: the test case passes because the output is correct; it seems
the test suite does not catch segfaults)
The issue is bisected to commit 9c47c43e0736 which adds support for
'//' to indicate the previous regex. The change in question is:
- temp = copy_parsing_escapes(pos, next);
- *regex = xzalloc(sizeof(regex_t));
- xregcomp(*regex, temp, G.regex_type);
- free(temp);
+ if (next != 0) {
+ temp = copy_parsing_escapes(pos, next);
+ G.previous_regex_ptr = *regex =
xzalloc(sizeof(regex_t));
+ xregcomp(*regex, temp, G.regex_type);
+ free(temp);
+ } else {
+ *regex = G.previous_regex_ptr;
+ if (!G.previous_regex_ptr)
+ bb_error_msg_and_die("no previous regexp");
+ }
In the original code, *regex was always a newly allocated pointer.
In the new code, there are cases where *regex (mapping to
sed_cmd.beg_match in the caller) is not a newly allocated pointer, but
the same as the previously allocated one.
When sed finishes, and sed_free_and_close_stuff() is called, there is
an iteration over the different sed_cmd structures. Following code:
if (sed_cmd->beg_match) {
regfree(sed_cmd->beg_match);
free(sed_cmd->beg_match);
}
will work fine in the first iteration that touches the repeated
pointer, but will fail in 'regfree' on the next iteration, where the
beg_match pointer points to the same value as was already freed.
Backtrace is:
Program received signal SIGSEGV, Segmentation fault.
free_token (node=0x198) at regcomp.c:3808
3808 regcomp.c: No such file or directory.
(gdb) bt
#0 free_token (node=0x198) at regcomp.c:3808
#1 0xf6fe474c in free_dfa_content (dfa=0x712220) at regcomp.c:598
#2 0xf6ff1200 in __regfree (preg=0x712068) at regcomp.c:647
#3 0x000d53dc in sed_free_and_close_stuff () at editors/sed.c:184
#4 0xf6f6be94 in __run_exit_handlers (status=0x0, listp=0xf70744b4
<__exit_funcs>,
run_list_atexit=run_list_atexit at entry=0x1) at exit.c:82
#5 0xf6f6bee8 in __GI_exit (status=<optimized out>) at exit.c:104
#6 0x0001a194 in xfunc_die () at libbb/xfunc_die.c:20
#7 0x000196b0 in run_applet_no_and_exit (applet_no=0xa9,
name=0xfff7ce6a "sed", argv=0xfff7bf14)
at libbb/appletlib.c:952
#8 0x00019718 in run_applet_and_exit (name=0xfff7ce6a "sed",
argv=0xfff7bf14) at libbb/appletlib.c:968
#9 0x000197fc in main (argc=0x4, argv=0xfff7bf14) at libbb/appletlib.c:1076
I'm not sure how to fix this:
* either keep a list in sed_free_and_close_stuff of pointers already freed
* or create a separate list of pointers-to-free when allocating
pointers rather than assuming that all values in beg_match need to be
freed
* or add a flag to indicate this special case in sed_cmd, which would
need some more communication between get_address and its caller.
* something else?
Best regards,
Thomas
More information about the busybox
mailing list