Re: [Cscope-devel] cscope aborts at exit due to myexit() recursive call

[Elad L. <e2...@gm...>]

The code is definitely wrong in making non-async-signal-safe calls in
a signal handler. The signal handlers should set some flag and let the
main loop know that it should exit.

--Elad

On Tue, Aug 6, 2024 at 8:45 AM Vladis Dronov <vd...@re...> wrote:
>
> i know this is a dead list, so i'm sorry for a necro posting into a gray
> void.
>
> still, we see cscope aborts at exit due to myexit() recursive call. we have
> 2 cases, see backtraces attached
> (i have no idea if this list@ accepts these attachments). also see the
> following for details:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=2269887#c15 - research
> - an ugly fix
>
> so a research shows that the cscope process attempted a normal exit (by "q"
> or Ctrl-D) calling myexit(0).
> in the middle of it a signal SIGHUP handler was called, which is myexit()
> itself. then glibc detects a
> double-free condition in __GI___libc_free() while performing
> fclose(refsfound). then glibc aborts the process
> with __GI_abort() and the "free(): double free detected in tcache 2"
> message. backtraces we have are:
>
> main.c:847 in main(): /* normal quit */ myexit(0); ->
>   main.c:1079 in myexit(sig=0): freefilelist(); ->
>     dir.c:727 in in freefilelist(): free(srcfiles[--nsrcfiles]); ->
>       malloc.c:3352 in __GI___libc_free() ->
>         <signal handler called> ->
>           main.c:1062 in myexit (sig=1 SIGHUP ): fclose(refsfound); ->
>             iofclose.c:74 in _IO_new_fclose() -> ... ->
>               malloc.c:3391 in __GI___libc_free() ->
>                 malloc_printerr("free(): double free detected in tcache 2")
> -> __GI_abort()
>
> main.c:847 in main(): /* normal quit */ myexit(0); ->
>   main.c:1066 in myexit(sig=0): unlink(temp1); ->
>     unlink.c:28 __unlink (name=<temp1> "/tmp/cscope.699047/cscope.1"); ->
>       <signal handler called> ->
>         main.c:1062 in myexit (sig=1 SIGHUP ): fclose(refsfound); ->
>           iofclose.c:74 in _IO_new_fclose() -> ... ->
>             malloc.c:3391 in __GI___libc_free() ->
>               malloc_printerr("free(): double free detected in tcache 2")
> -> __GI_abort()
>
> in a short word, a process abort is caused by myexit() interrupted by a
> signal which handler is myexit() itself.
>
> note, free() and fclose() are NOT async-signal-safe, see "man 7
> signal-safety". calling an async-signal-unsafe
> function from a signal handler is perfectly legal unless the signal
> interrupted another async-signal-unsafe function.
> so i'm not sure about the unlink() case, unlink() is async-signal-safe.
> still, the double-free condition is still
> there, as we can see from the backtrace.
>
> i'm not going to fix the whole signal handling in cscope, as it is broken.
> 1) myexit() calls functions which are
> not async-signal-safe 2) signal() infrastructure (which is broken) is used
> instead of sigaction(). i'll fix only
> this specific case where myexit() is interrupted by a call to myexit()
> itself by resetting signal handlers to
> default ones. this may be not entirely correct but whatever.
>
> Best regards,
> Vladis
>
> ---
>
> diff -up ./src/main.c.orig ./src/main.c
> --- ./src/main.c.orig 2024-08-04 16:49:08.723525637 +0200
> +++ ./src/main.c 2024-08-04 16:53:19.967862016 +0200
> @@ -1056,11 +1056,18 @@ Please see the manpage for more informat
>  void
>  myexit(int sig)
>  {
> + /* reset signal handlers to default ones, so myexit() is not called
> + * recursively as a signal handler during a normal exit */
> + signal(SIGINT, SIG_DFL);
> + signal(SIGQUIT, SIG_DFL);
> + signal(SIGHUP, SIG_DFL);
> + signal(SIGTERM, SIG_DFL);
> +
>   /* HBB 20010313; close file before unlinking it. Unix may not care
>   * about that, but DOS absolutely needs it */
>   if (refsfound != NULL)
>   fclose(refsfound);
> -
> +
>   /* remove any temporary files */
>   if (temp1[0] != '\0') {
>   unlink(temp1);
> _______________________________________________
> Cscope-devel mailing list
> Csc...@li...
> https://lists.sourceforge.net/lists/listinfo/cscope-devel