Re: [PATCH] NULL dereference on EOF in heredoc with history on

[Lubomir Rintel]

On Sun, 2009-03-22 at 13:03 -0400, Chet Ramey wrote:
> Lubomir Rintel wrote:
> 
> > Bash Version: 4.0
> > Patch Level: 10
> > Release Status: release
> > 
> > Description:
> >     If the heredoc input is terminted with end-of-file by pressing ^D
> >     while history is on, bash enters an infinite loop (due to NULL
> >     dereference along with a rather sick SIGSEGV handling, which will
> >     probably be subject to a subsequent patch).
> 
> The SIGSEGV just needs to be handled immediately, the problem of doing
> "too much" in a signal handler be damned:
> 
> *** ../bash-4.0-patched/sig.c 2009-01-04 14:32:41.000000000 -0500
> --- sig.c     2009-03-21 14:31:30.000000000 -0400
> ***************
> *** 449,452 ****
> --- 449,457 ----
>        int sig;
>   {
> +   /* If we get called twice with the same signal before handling it,
> +      terminate right away. */
> +   if (sig == terminating_signal)
> +     terminate_immediately = 1;
> +
>     terminating_signal = sig;

I was thinking about distinguishing between signals that are triggered
by the instruction at the handler's return address and the asynchronous
ones somehow, but clearly your solution seems much more clever.

Still I'm not sure if this is not a regression. I can imagine valid use
cases where you deal with lot of signals, while sometimes not being able
to have your portion of CPU share served. Just dying is probably not
acceptable there. I'm attaching my change merged with yours, just in
case.

--- bash-4.0.orig/sig.c 2009-01-04 20:32:41.000000000 +0100
+++ bash-4.0/sig.c      2009-03-22 18:51:01.021314888 +0100
@@ -448,6 +448,51 @@ sighandler
 termsig_sighandler (sig)
      int sig;
 {
+  /* If we get called twice with the same signal before handling it,
+     there's a good chance we're stuck in an endless loop, due to
+     returning to the same instruction that trigger the signal.
+     Terminate right away, unless the signal is on the whitelist of
+     signals that are known to be always delivered asynchronously.  */
+  if (
+#ifdef SIGHUP
+    sig != SIGHUP &&
+#endif
+#ifdef SIGINT
+    sig != SIGINT &&
+#endif
+#ifdef SIGDANGER
+    sig != SIGDANGER &&
+#endif
+#ifdef SIGPIPE
+    sig != SIGPIPE &&
+#endif
+#ifdef SIGALRM
+    sig != SIGALRM &&
+#endif
+#ifdef SIGTERM
+    sig != SIGTERM &&
+#endif
+#ifdef SIGXCPU
+    sig != SIGXCPU &&
+#endif
+#ifdef SIGXFSZ
+    sig != SIGXFSZ &&
+#endif
+#ifdef SIGVTALRM
+    sig != SIGVTALRM &&
+#endif
+#ifdef SIGLOST
+    sig != SIGLOST &&
+#endif
+#ifdef SIGUSR1
+    sig != SIGUSR1 &&
+#endif
+#ifdef SIGUSR2
+    sig != SIGUSR2 &&
+#endif
+    sig == terminating_signal
+  ) terminate_immediately = 1;
+
   terminating_signal = sig;
 
   /* XXX - should this also trigger when interrupt_immediately is set? */

-- 
Lubomir Rintel <lkundrak@v3.sk>