Re: Race condition in handling SIGHUP

[Siteshwar Vashisht]

----- Original Message -----
> From: "Chet Ramey" <chet.ramey@case.edu>
> To: "Siteshwar Vashisht" <svashish@redhat.com>, bug-bash@gnu.org
> Cc: dkaspar@redhat.com, "chet ramey" <chet.ramey@case.edu>
> Sent: Thursday, April 28, 2016 6:19:17 PM
> Subject: Re: Race condition in handling SIGHUP
> 
> On 4/27/16 6:04 AM, Siteshwar Vashisht wrote:
> 
> > 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]?
> 
> Because systems using a networked password database can hang at a priority
> that doesn't interrupt the system call when a SIGHUP arrives.  The handler
> gets run, but the system call gets restarted.  Setting
> terminate_immediately was a way to get around that.
> 
> That's probably not as necessary as it once was, so we can try removing
> that code from bash_tilde_expand.
> 
> > 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.
> 
> The checks have to handle all the places terminate_immediately is being
> set.  However, you need to notice how terminate_immediately can be set if
> a terminating signal arrives twice before it's handled.  You don't want to
> try and save the history, which involves memory allocation and multiple
> system and C library calls, from a signal handler context.
> 
> That code is written the way it is to accommodate the much more common
> case of users exiting a shell by hitting the `close' button on their
> terminal window, which causes the terminal emulator to send one or more
> SIGHUPs to the shell process, usually while readline is active.  You want
> shell to try and save the history in this case, since that's what users
> expect.

if (interactive_shell == 0 || interactive == 0 || (sig != SIGHUP && sig != 
SIGTERM) || no_line_editing || (RL_ISSTATE (RL_STATE_READCMD) == 0))

This condition means if shell is waiting for a command to finish execution 
(readline is not waiting for user input) and the terminating signal arrives 
more than once (may be through hitting 'close' button in terminal) 
~/.bash_history will not be saved.

> 
> Chet
> --
> ``The lyf so short, the craft so long to lerne.'' - Chaucer
>                ``Ars longa, vita brevis'' - Hippocrates
> Chet Ramey, ITS, CWRU    chet@case.edu    http://cnswww.cns.cwru.edu/~chet/
> 
> 

-- 
--
Siteshwar Vashisht
Software Engineer