[PATCH] Defer SIGCHLD trap handler when waitchld is called from within signal handler
Configuration Information [Automatically generated, do not change]:
Machine: x86_64
OS: linux-gnu
Compiler: gcc
Compilation CFLAGS: -DPROGRAM='bash' -DCONF_HOSTTYPE='x86_64'
-DCONF_OSTYPE='linux-gnu' -DCONF_MACHTYPE='x86_64-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 -DRECYCLES_PIDS
-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector
--param=ssp-buffer-size=4 -m64 -mtune=generic
uname output: Linux localhost 3.1.6-1.fc16.x86_64 #1 SMP Wed Dec 21 22:41:17
UTC 2011 x86_64 x86_64 x86_64 GNU/Linux
Machine Type: x86_64-redhat-linux-gnu
Bash Version: 4.2
Patch Level: 20
Release Status: release
Description:
When a SIGCHLD is received in job control mode and a handler
for the signal is installed, bash calls the trap handler
within the signal handler itself. This is unsafe because the
trap handler function run_sigchld_trap() uses the glibc malloc
functions quite extensively (within the function itself and
also the function it calls, i.e. parse_and_execute() ). This
results in a deadlock and sometimes even a segmentation fault
due to memory corruption.
Repeat-By:
$ cat > foo.sh
#!/bin/sh
check_stop_child_trap() {
echo "child died!"
}
do_something() {
while true; do
true &
done
}
trap check_stop_child_trap SIGCHLD
do_something &
do_something
^d
$ bash
$ . foo.sh
--
The above may either hang or result in a segmentation fault.
Fix:
The patch below fixes this by deferring execution of the trap handler
by adding it to pending_sigs.
diff -pruN bash-4.1/jobs.c bash-4.1.patched/jobs.c
--- bash-4.1/jobs.c 2009-11-30 03:42:05.0 +0530
+++ bash-4.1.patched/jobs.c 2012-03-06 16:44:15.706595703 +0530
@@ -3037,6 +3037,7 @@ waitchld (wpid, block)
PROCESS *child;
pid_t pid;
int call_set_current, last_stopped_job, job, children_exited, waitpid_flags;
+ int called_from_sighand = sigchld;
static int wcontinued = WCONTINUED; /* run-time fix for glibc problem */
call_set_current = children_exited = 0;
@@ -3161,7 +3162,17 @@ waitchld (wpid, block)
longjmp (wait_intr_buf, 1);
}
- run_sigchld_trap (children_exited);
+ /* Queue up the trap handler if we're called directly from within the
+ signal handler. */
+ if (called_from_sighand)
+ {
+ int i = children_exited;
+ interrupt_immediately = 0;
+ while (i--)
+ trap_handler (SIGCHLD);
+ }
+ else
+ run_sigchld_trap (children_exited);
}
/* We have successfully recorded the useful information about this process
[PATCH] Defer SIGCHLD trap handler when waitchld is called from within the signal handler
Configuration Information [Automatically generated, do not change]:
Machine: x86_64
OS: linux-gnu
Compiler: gcc
Compilation CFLAGS: -DPROGRAM='bash' -DCONF_HOSTTYPE='x86_64'
-DCONF_OSTYPE='linux-gnu' -DCONF_MACHTYPE='x86_64-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 -DRECYCLES_PIDS -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2
-fexceptions -fstack-pr\
otector --param=ssp-buffer-size=4 -m64 -mtune=generic
uname output: Linux hostname.removed 3.1.6-1.fc16.x86_64 #1 SMP Wed Dec 21
22:41:17 UTC 2011 x86_64 x86_64 x86_64 GNU/Linux
Machine Type: x86_64-redhat-linux-gnu
Bash Version: 4.2
Patch Level: 20
Release Status: release
Description:
When a SIGCHLD is received in job control mode and a handler for the
signal is installed, bash calls the trap handler within the signal
handler itself. This is unsafe because the trap handler function
run_sigchld_trap() uses the glibc malloc functions quite extensively
(within the function itself and also the function it calls, i.e.
parse_and_execute() ). This results in a deadlock and sometimes even a
segmentation fault due to memory corruption.
Repeat-By:
$ cat > foo.sh
#!/bin/sh
check_stop_child_trap() {
echo "child died!"
}
do_something() {
while true; do
true &
done
}
trap check_stop_child_trap SIGCHLD
do_something &
do_something
^d
$ bash
$ . foo.sh
--
The above may either hang or result in a segmentation fault.
Fix:
The attached patch fixes this by deferring execution of the trap
handler by adding it to pending_sigs.
Regards,
Siddhesh
diff -pruN bash-4.1/jobs.c bash-4.1.patched/jobs.c
--- bash-4.1/jobs.c 2009-11-30 03:42:05.0 +0530
+++ bash-4.1.patched/jobs.c 2012-03-06 16:44:15.706595703 +0530
@@ -3037,6 +3037,7 @@ waitchld (wpid, block)
PROCESS *child;
pid_t pid;
int call_set_current, last_stopped_job, job, children_exited, waitpid_flags;
+ int called_from_sighand = sigchld;
static int wcontinued = WCONTINUED; /* run-time fix for glibc problem */
call_set_current = children_exited = 0;
@@ -3161,7 +3162,17 @@ waitchld (wpid, block)
longjmp (wait_intr_buf, 1);
}
- run_sigchld_trap (children_exited);
+ /* Queue up the trap handler if we're called directly from within the
+ signal handler. */
+ if (called_from_sighand)
+ {
+ int i = children_exited;
+ interrupt_immediately = 0;
+ while (i--)
+ trap_handler (SIGCHLD);
+ }
+ else
+ run_sigchld_trap (children_exited);
}
/* We have successfully recorded the useful information about this process
[PATCH] trap -p does not display ignored signals inherited from parent by a bash process
Hi,
I already reported this in Fedora rawhide
(https://bugzilla.redhat.com/show_bug.cgi?id=572305) This also seems
to be present upstream, so I hope you guys find this useful.
Description of problem:
trap -p doesn't show ignored signals inherited from its parent across an execve
Version-Release number of selected component (if applicable):
bash-4.1.2-3.fc14
How reproducible:
Always
Steps to Reproduce:
1. trap '' SIGINT
2. exec bash
3. trap -p
Actual results:
Nothing is returned as output of trap -p. But at the same time, running cat and
trying to interrupt it shows that the signal is in fact being ignored
Expected results:
The list of ignored signals should be returned
Additional info:
Attached patch ensures that inherited ignored signals are reflected in
trap_list and then subsequently displayed when trap -p is invoked.
--
Siddhesh Poyarekar
http://siddhesh.in
--- a/jobs.c 2009-11-30 03:42:05.0 +0530
+++ b/jobs.c 2010-03-11 00:43:59.0 +0530
@@ -3772,9 +3772,16 @@ set_new_line_discipline (tty)
void
initialize_job_signals ()
{
+ SigHandler *old_int;
+
if (interactive)
{
- set_signal_handler (SIGINT, sigint_sighandler);
+ old_int = set_signal_handler (SIGINT, sigint_sighandler);
+
+ /* If a handler has been modified or inherited, restore the old one */
+ if ( old_int != (SigHandler *)SIG_DFL )
+set_signal_handler (SIGINT, old_int);
+
set_signal_handler (SIGTSTP, SIG_IGN);
set_signal_handler (SIGTTOU, SIG_IGN);
set_signal_handler (SIGTTIN, SIG_IGN);
--- a/trap.c 2010-03-11 00:35:25.0 +0530
+++ b/trap.c 2010-03-11 00:41:31.0 +0530
@@ -142,9 +142,23 @@ initialize_traps ()
for (i = 1; i < NSIG; i++)
{
pending_traps[i] = 0;
- trap_list[i] = (char *)DEFAULT_SIG;
+ /* These will be set by initialize_signals(), but we don't want to show
+ * them in the traps list */
+ if ( i == SIGTSTP || i == SIGTTOU || i == SIGTTIN )
+{
+ trap_list[i] = (char *)DEFAULT_SIG;
+ }
+ else
+{
+ /* Don't assume that the original signal was SIG_DFL */
+ original_signals[i] = (SigHandler *)set_signal_handler (i, SIG_DFL);
+ set_signal_handler (i, original_signals[i]);
+
+ trap_list[i] = (char *)original_signals[i];
+ }
sigmodes[i] = SIG_INHERITED;
- original_signals[i] = IMPOSSIBLE_TRAP_HANDLER;
+ if ( original_signals[i] == SIG_DFL )
+original_signals[i] = IMPOSSIBLE_TRAP_HANDLER;
}
/* Show which signals are treated specially by the shell. */
Re: [PATCH] trap -p does not display ignored signals inherited from parent by a bash process
On Tue, Mar 23, 2010 at 7:46 PM, Eric Blake wrote: > On 03/23/2010 01:29 AM, Siddhesh Poyarekar wrote: >> Steps to Reproduce: >> 1. trap '' SIGINT >> 2. exec bash >> 3. trap -p > > POSIX states: > > "Signals that were ignored on entry to a non-interactive shell cannot be > trapped or reset, although no error need be reported when attempting to > do so. An interactive shell may reset or catch signals ignored on entry." > http://www.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#trap > > I see no bug - bash is within its rights to pretend that an inherited > ignored SIGINT has no trap setting, seeing as how the user cannot modify > that through any use of trap. That is, 'trap -p' is designed to output > the text that will restore traps to their normal state, but since there > is no way to change the state of SIGINT from being ignored, there is > nothing needed in the output. > Thanks for that explanation. I guess we can also detect if a signal is being ignored by trying to set a trap for that signal and verify that it has not been set. This was my original problem and I went about trying to fix it in code rather than reading the docs carefully :) -- Siddhesh Poyarekar http://siddhesh.in
Re: [PATCH] trap -p does not display ignored signals inherited from parent by a bash process
On Thu, Mar 25, 2010 at 7:19 PM, Eric Blake wrote: > Technically, this is only a POSIX requirement on non-interactive shells. > Interactive shells may reset ignored signals on entry, effectively > starting life as if nothing had been inherited ignored. But I don't > know (without looking at the sources) whether bash explicitly modifies > inherited ignored signals in interactive shells. Bash does not do it; ignored signals remain ignored in the subshell. I wanted to know if it would be possible to explicitly document it in the manpage or provide a non-standard extension to display signals that are 'not available to play with'. -- Siddhesh Poyarekar http://siddhesh.in
Re: [PATCH] trap -p does not display ignored signals inherited from parent by a bash process
On Thu, Mar 25, 2010 at 8:13 PM, Eric Blake wrote:
> Subshells are different than new shells on what POSIX requires for
> signal manipulations. Again, quoting POSIX:
>
> "When a subshell is entered, traps that are not being ignored are set to
> the default actions. This does not imply that the trap command cannot be
> used within the subshell to set new traps."
>
> That is, if a non-interactive parent inherited a signal ignored, then
> neither the parent nor the subshell can change it. But if a
> non-interactive parent inherits a default signal, then explicitly
> ignores it, then subshells will inherit it ignored but can still reset
> it back to default.
>
I do this:
In a terminal, execute a script which does basically this:
trap '' SIGTERM
...
bash
According to the above explanation, the resultant bash prompt I get on
execution of this script will allow me to override the ignored SIGTERM
since it was set in its non-interactive parent. But that is not what
happens. I cannot override SIGTERM regardless of whether the parent
shell is interactive or not. But if I replace SIGTERM with SIGHUP, it
works the way you mentioned. The reason for this seems to be the way
the GETORIGSIG macro is defined:
#define GETORIGSIG(sig) \
do { \
original_signals[sig] = (SigHandler *)set_signal_handler (sig, SIG_DFL); \
set_signal_handler (sig, original_signals[sig]); \
if (original_signals[sig] == SIG_IGN) \
sigmodes[sig] |= SIG_HARD_IGNORE; \
} while (0)
So any signal that GETORIGSIG is called for and is inherited as
ignored from parent will automatically be hard ignored. Due to this,
we get automatically hard ignored SIGTERM, SIGQUIT, SIGINT and SIGCHLD
whenever these signals are inherited as ignored.
It doesn't look as though the hard ignore belongs in GETORIGSIG.
--
Siddhesh Poyarekar
http://siddhesh.in
Re: [PATCH] trap -p does not display ignored signals inherited from parent by a bash process
On Fri, Mar 26, 2010 at 2:40 AM, Eric Blake wrote: > Yes. POSIX requires this for non-interactive shells, and does not > forbid it for interactive shells. Technically, this code could be > modified to exempt interactive shells from hard-ignores, or left as is > by treating both classes of shells identically in keeping things > hard-ignored; either approach still meets POSIX. > The code does explicitly hard ignore signals ignored when entering a non-interactive shell; see initialize_terminating_signals() in sig.c. The problem is that in addition to this, it ends up doing a hard ignore for the special signals in GETORIGSIG() -- not for all terminating signals, just the special signals. I know POSIX does not say anything about it, but neither does the man page. The result of this is confusing behaviour. It should either hard ignore _all_ signals or none. I really don't see any reason for a hard ignore for special signals via GETORIGSIG when we already have the stuff in place in initialize_terminating_signals() that does the right thing. What's more, BeOS gets special treatment in all this; its hard ignore flag is removed after GETORIGSIG() for SIGINT because BeOS sets SIGINT to SIG_IGN by default: #if defined (__BEOS__) /* BeOS sets SIGINT to SIG_IGN! */ original_signals[SIGINT] = SIG_DFL; sigmodes[SIGINT] &= ~SIG_HARD_IGNORE; #endif -- Siddhesh Poyarekar http://siddhesh.in
Re: [PATCH] trap -p does not display ignored signals inherited from parent by a bash process
On Fri, Mar 26, 2010 at 11:20 AM, Siddhesh Poyarekar wrote: > The code does explicitly hard ignore signals ignored when entering a > non-interactive shell; see initialize_terminating_signals() in sig.c. > The problem is that in addition to this, it ends up doing a hard > ignore for the special signals in GETORIGSIG() -- not for all > terminating signals, just the special signals. > Oops, forgot to add that GETORIGSIG is called for interactive as well as non-interactive shells. -- Siddhesh Poyarekar http://siddhesh.in
trap output piped to another program gives incomplete results
Configuration Information [Automatically generated, do not change]: Machine: x86_64 OS: linux-gnu Compiler: gcc Compilation CFLAGS: -DPROGRAM='bash' -DCONF_HOSTTYPE='x86_64' -DCONF_OSTYPE='linux-gnu' -DCONF_MACHTYPE='x86_64-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 -DRECYCLES_PIDS -O2 -g -pipe -Wall +-Wp,-D_FORTIFY_SOURCE=2 -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic uname output: Linux spoyarek.pnq.redhat.com 2.6.34-0.16.rc2.git0.fc14.x86_64 #1 SMP Tue Mar 23 02:02:52 UTC 2010 x86_64 x86_64 x86_64 GNU/Linux Machine Type: x86_64-redhat-linux-gnu Bash Version: 4.1 Patch Level: 2 Release Status: release Description: When output of trap is piped to another command, say, less or cat -, it shows only ignored signals. Repeat-By: $ trap '' SIGINT $ trap 'echo hello' SIGHUP $ trap 'echo foobar' SIGTERM $ trap | cat - trap -- '' SIGINT Expected Result: $ trap | cat - trap -- '' SIGINT trap -- 'echo hello' SIGHUP trap -- 'echo foobar' SIGTERM Fix: I've not looked at how this can be fixed yet, but it looks like bash is forking for the trap builtin, which inherits the ignored signals, hence showing only them as active traps into the pipe. The other signals are not inherited and hence do not show up at all. The fix would probably involve either preparing a trap list before forking for the first pipe process or not fork at all in case a trap builtin is called. In either case I suspect the fix will be quite hacky. As an aside, the same behaviour can be seen on ksh as well as csh. Only zsh seems to show the trap list correctly to the pipeline. Regards, Siddhesh -- Siddhesh Poyarekar http://siddhesh.in
Re: trap output piped to another program gives incomplete results
On Sat, Apr 10, 2010 at 8:51 AM, Chet Ramey wrote: > On 3/31/10 1:45 AM, Siddhesh Poyarekar wrote: > >> Bash Version: 4.1 >> Patch Level: 2 >> Release Status: release >> >> Description: >> When output of trap is piped to another command, say, less or >> cat -, it shows only ignored signals. > > As the standard is currently written, this is the correct behavior. > > The standard considers traps in subshells in two places: where it talks > about "subshell environments" and where it describes the behavior of trap. > > The former says only that all commands in a pipeline are run in a subshell > environment, and that in a subshell environment traps "are set to the > default values." The latter says that "when a subshell is entered, traps > that are not being ignored are set to the default actions." This is an > obvious contradiction, and most shells implement the latter. > > Bash and some other shells special-case command substitution, because the > standard includes an example illustrating the use of command substitution > to save and restore traps. This is not univeral -- dash, for instance, > does not do it. > Thanks, will you consider fixing this or accept a patch to fix this? A fix along the lines of the standards change Eric pointed out ought to work I think. I'm not sure at what point the change becomes effective/enforceable. -- Siddhesh Poyarekar http://siddhesh.in
