Hello,
Recently we came across a bug in bash which was introduced by below patch :
http://git.savannah.gnu.org/cgit/bash.git/commit/?id=d5d00961
In bash 4.2 this could lead to a race condition. 'yy_readline_get ()' function
sets 'terminate_immediately' to 1 before calling readline() at [1].
If shell receives SIGHUP and executes 'termsig_handler ()' before setting
'terminate_immediately' back to 0 [2], we will end up at [3] (considering
'RL_ISSTATE (RL_STATE_READCMD)' evaluates to 0 when readline is not waiting to
read command from user), and ~/.bash_history will not be updated.
We started seeing this bug after above mentioned patch was backported to RHEL
6. Sometimes ~/.bash_history is not updated when user executes 'reboot' command
in a ssh session. This is caused by race condition in handling SIGHUP.
While this issue was fixed by backporting somes changes (See attached patch)
from [4] to bash-4.2 or older versions, there is still a corner case which may
cause race condition in handling SIGHUP in current upstream.
'bash_tilde_expand ()' function sets 'terminate_immediately' to 1 at [5]
If SIGHUP is received and termsig_handler () gets executed before reaching [6],
~/.bash_history will not be updated. This can happen in a scenario where user
is running ssh session and requests for expansion for '~', if an admin issues
'reboot' command at the same time then ~/.bash_history may not be updated.
I have 2 questions about how current upstream handles this condition :
1. Why 'terminate_immediately' is set to 1 at [7]?
2. Why 'RL_ISSTATE (RL_STATE_READCMD)' is being checked at [8]? This will
evaluate to 0 if readline is not waiting to read a command from user. I believe
this check can be removed.
--------------------
[1] http://git.savannah.gnu.org/cgit/bash.git/tree/parse.y?id=bash-4.2#n1441
[2] http://git.savannah.gnu.org/cgit/bash.git/tree/parse.y?id=bash-4.2#n1446
[3]
http://git.savannah.gnu.org/cgit/bash.git/tree/sig.c?id=d5d0096115d0d484fd669ad170498962ea45e841#n513
[4]
http://git.savannah.gnu.org/cgit/bash.git/commit/?id=ac50fbac377e32b98d2de396f016ea81e8ee9961
[5] http://git.savannah.gnu.org/cgit/bash.git/tree/general.c#n994
[6] http://git.savannah.gnu.org/cgit/bash.git/tree/general.c#n1004
[7] http://git.savannah.gnu.org/cgit/bash.git/tree/general.c#n994
[8] http://git.savannah.gnu.org/cgit/bash.git/tree/sig.c#n524
--
Siteshwar Vashisht
Software Engineer
From 1ecc0ff050d74aef548677fb97b2113b0920c711 Mon Sep 17 00:00:00 2001
From: Siteshwar Vashisht <svashi...@redhat.com>
Date: Mon, 25 Apr 2016 14:04:10 +0530
Subject: [PATCH] Do not set terminate_immediately while reading input
---
builtins/read.def | 3 +--
parse.y | 5 +----
y.tab.c | 5 +----
3 files changed, 3 insertions(+), 10 deletions(-)
diff --git a/builtins/read.def b/builtins/read.def
index 1ef9142..2dcaf35 100644
--- a/builtins/read.def
+++ b/builtins/read.def
@@ -455,7 +455,6 @@ read_builtin (list)
of the unwind-protect stack after the realloc() works right. */
add_unwind_protect (xfree, input_string);
interrupt_immediately++;
- terminate_immediately++;
unbuffered_read = (nchars > 0) || (delim != '\n') || input_is_pipe;
@@ -512,6 +511,7 @@ read_builtin (list)
if (retval <= 0)
{
+ CHECK_TERMSIG;
eof = 1;
break;
}
@@ -616,7 +616,6 @@ add_char:
zsyncfd (fd);
interrupt_immediately--;
- terminate_immediately--;
discard_unwind_frame ("read_builtin");
retval = eof ? EXECUTION_FAILURE : EXECUTION_SUCCESS;
diff --git a/parse.y b/parse.y
index 9a78d0c..0b9f9ff 100644
--- a/parse.y
+++ b/parse.y
@@ -1440,12 +1440,11 @@ yy_readline_get ()
old_sigint = (SigHandler *)set_signal_handler (SIGINT, sigint_sighandler);
interrupt_immediately++;
}
- terminate_immediately = 1;
current_readline_line = readline (current_readline_prompt ?
current_readline_prompt : "");
- terminate_immediately = 0;
+ CHECK_TERMSIG;
if (signal_is_ignored (SIGINT) == 0 && old_sigint)
{
interrupt_immediately--;
@@ -1606,13 +1605,11 @@ yy_stream_get ()
if (interactive)
{
interrupt_immediately++;
- terminate_immediately++;
}
result = getc_with_restart (bash_input.location.file);
if (interactive)
{
interrupt_immediately--;
- terminate_immediately--;
}
}
return (result);
diff --git a/y.tab.c b/y.tab.c
index d702554..563d312 100644
--- a/y.tab.c
+++ b/y.tab.c
@@ -3757,12 +3757,11 @@ yy_readline_get ()
old_sigint = (SigHandler *)set_signal_handler (SIGINT, sigint_sighandler);
interrupt_immediately++;
}
- terminate_immediately = 1;
current_readline_line = readline (current_readline_prompt ?
current_readline_prompt : "");
- terminate_immediately = 0;
+ CHECK_TERMSIG;
if (signal_is_ignored (SIGINT) == 0 && old_sigint)
{
interrupt_immediately--;
@@ -3923,13 +3922,11 @@ yy_stream_get ()
if (interactive)
{
interrupt_immediately++;
- terminate_immediately++;
}
result = getc_with_restart (bash_input.location.file);
if (interactive)
{
interrupt_immediately--;
- terminate_immediately--;
}
}
return (result);
--
2.5.5
