[PATCH] NULL dereference on EOF in heredoc with history on
Configuration Information [Automatically generated, do not change]:
Machine: i386
OS: linux-gnu
Compiler: gcc
Compilation CFLAGS: -DPROGRAM='bash' -DCONF_HOSTTYPE='i386'
-DCONF_OSTYPE='linux-gnu' -DCONF_MACHTYPE='i386-redhat-linux-gnu'
-DCONF_VENDOR='redhat' -DLOCALEDIR='/usr/share/locale' -DPACKAGE='bash' -DSHELL
-DHAVE_CONFIG_H -I. -I. -I./include -I./lib -D_GNU_SOURCE
-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -O2 -g -pipe -Wall
-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m32 -march=i586 -mtune=generic
-fasynchronous-unwind-tables
uname output: Linux bimbo.lan 2.6.29-0.237.rc7.git4.fc11.i586 #1 SMP Wed Mar 11
18:55:21 EDT 2009 i686 i686 i386 GNU/Linux
Machine Type: i386-redhat-linux-gnu
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).
Repeat-By:
A simple test case is attached. Here's how to reproduce the problem
in an interactive shell session:
$ set -o history
$ true < ^D
... (looping)
Fix:
diff -urp bash-4.0.orig/parse.y bash-4.0/parse.y
--- bash-4.0.orig/parse.y 2009-03-21 16:52:00.937619544 +0100
+++ bash-4.0/parse.y2009-03-21 16:54:12.990620212 +0100
@@ -1879,7 +1879,7 @@ read_secondary_line (remove_quoted_newli
prompt_again ();
ret = read_a_line (remove_quoted_newline);
#if defined (HISTORY)
- if (remember_on_history && (parser_state & PST_HEREDOC))
+ if (ret && remember_on_history && (parser_state & PST_HEREDOC))
{
/* To make adding the the here-document body right, we need to rely
on history_delimiting_chars() returning \n for the first line of
diff -urp bash-4.0.orig/tests/heredoc.right bash-4.0/tests/heredoc.right
--- bash-4.0.orig/tests/heredoc.right 2009-02-19 21:25:53.0 +0100
+++ bash-4.0/tests/heredoc.right2009-03-21 16:57:29.827625342 +0100
@@ -58,6 +58,7 @@ qux
bar
qux
-./heredoc.tests: line 96: warning: here-document at line 94 delimited by
end-of-file (wanted `EOF')
+OK: unterminated heredoc spanning end-of-file dealt with correctly
+./heredoc.tests: line 107: warning: here-document at line 105 delimited by
end-of-file (wanted `EOF')
hi
there
diff -urp bash-4.0.orig/tests/heredoc.tests bash-4.0/tests/heredoc.tests
--- bash-4.0.orig/tests/heredoc.tests 2009-02-19 21:25:45.0 +0100
+++ bash-4.0/tests/heredoc.tests2009-03-21 16:57:21.024630685 +0100
@@ -88,6 +88,17 @@ ${THIS_SH} -c 'type fff'
${THIS_SH} ./heredoc1.sub
+# If the above doesn't finish in 4 seconds, then we're
+# almost certainly stuck in an infinite loop
+
+echo 'true </dev/null 2>&1 &
+PID=$!
+sleep 4
+kill -KILL $! 2>/dev/null &&
+ echo 'FAIL: unterminated heredoc spanning end-of-file probably stuck in an
endless loop' ||
+ echo 'OK: unterminated heredoc spanning end-of-file dealt with correctly'
+wait 2>/dev/null
+
# check that end of file delimits a here-document
# THIS MUST BE LAST!
--
Lubomir Rintel
Re: [PATCH] NULL dereference on EOF in heredoc with history on
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.0 -0500
> --- sig.c 2009-03-21 14:31:30.0 -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.0 +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
