On Mon, Sep 24, 2018 at 4:36 PM Thomas De Schampheleire <patrickdeping...@gmail.com> wrote: > 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@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?
How about "don't free them at all"? _______________________________________________ busybox mailing list busybox@busybox.net http://lists.busybox.net/mailman/listinfo/busybox