[prev in list] [next in list] [prev in thread] [next in thread]
List: bash-bug
Subject: Re: Race condition in handling SIGHUP
From: Siteshwar Vashisht <svashisht () redhat ! com>
Date: 2016-04-29 9:12:06
Message-ID: 1644589846.41360029.1461921126286.JavaMail.zimbra () redhat ! com
[Download RAW message or body]
----- 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
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic