signals: Bug or manpage inconsistency?

signals: Bug or manpage inconsistency?

[Thomas Gleixner]

While trying to address the longstanding FIXME in the posix timer code
related to ignored signals, I stumbled over the following issue:

I blocked the signal of the timer, then installed the SIG_IGN handler,
created and started the timer. After a short sleep the timer has fired
several times, but it's still ignored AND blocked.

Calling sigpending() after that has the timer signal set. See test case
below.

But 'man sigpending' says:

   "If a signal is both blocked and has a disposition of "ignored", it is _not_
    added to the mask of pending signals when generated."

So something is clearly wrong here.

The same happens with sigwait() while the signal is still blocked and
ignored, it returns with that signal number and has the signal dequeued.


The whole blocked vs. ignored handling is inconsistent both in the posix
spec and in the kernel.

The only thing vs. ignored signals what the spec mandates is:

 SIG_IGN:

 Delivery of the signal shall have no effect on the process.

 ...

 Setting a signal action to SIG_IGN for a signal that is pending shall
 cause the pending signal to be discarded, whether or not it is blocked.

 ...

 Any queued values pending shall be discarded and the resources used to
 queue them shall be released and made available to queue other signals.

That's exactly what the kernel does in do_sigaction().

And for everything else the spec is blurry:

  If the action associated with a blocked signal is to ignore the signal
  and if that signal is generated for the process, it is unspecified
  whether the signal is discarded immediately upon generation or remains
  pending.

So the kernel has chosen to keep them pending for whatever reasons, which
does not make any sense to me, but there is probably a historic reason.

The commit which added the queuing of blocked and ignored signals is in the
history tree with a pretty useless changelog.

https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git

 commit 98fc8ab9e74389e0c7001052597f61336dc62833
 Author: Linus Torvalds <torvalds@penguin.transmeta.com>
 Date:   Tue Feb 11 20:49:03 2003 -0800

     Don't wake up processes unnecessarily for ignored signals

It rewrites sig_ignored() and adds the following to it:

+       /*
+        * Blocked signals are never ignored, since the
+        * signal handler may change by the time it is
+        * unblocked.
+        */
+       if (sigismember(&t->blocked, sig))
+               return 0;

I have no idea how that is related to $subject of the commit and why this
decision was made.

Linus, any recollection?

IMO, it's perfectly reasonable to discard ignored signals even when the
signal is in the blocked mask. When its unblocked and SIG_IGN is replaced
then the next signal will be delivered. But hell knows, how much user space
depends on this weird behaviour by now.

Thanks,

	tglx
	
8<-------------

#define _GNU_SOURCE
#include <stdint.h>
#include <stdio.h>
#include <unistd.h>
#include <signal.h>
#include <string.h>
#include <time.h>

#include <sys/types.h>
#include <sys/time.h>
#include <sys/syscall.h>

int main(void)
{
	struct itimerspec tspec;
	struct sigevent sigev;
	struct sigaction action;
	int signal, sig = SIGALRM;
	sigset_t set, pend;
	timer_t timerid;

	sigemptyset(&set);
	sigaddset(&set, sig);
	sigprocmask(SIG_BLOCK, &set, NULL);

	memset(&action, 0, sizeof(action));
	action.sa_handler = SIG_IGN;
	sigaction(sig, &action, NULL);

	memset(&sigev, 0, sizeof(sigev));
	sigev.sigev_notify = SIGEV_SIGNAL;
	sigev.sigev_signo = sig;
	sigev.sigev_value.sival_ptr = &timerid;
	timer_create(CLOCK_REALTIME, &sigev, &timerid);

	tspec.it_interval.tv_sec = 0;
	tspec.it_interval.tv_nsec = 100 * 1e6;

	tspec.it_value.tv_sec = 0;
	tspec.it_value.tv_nsec = 100 * 1e6;

	timer_settime(timerid, 0, &tspec, NULL);

	sleep(1);

	sigpending(&pend);
	if (sigismember(&pend, sig)) {
		/* This is reached */
		printf("Timer signal pending 1\n");
		sigwait(&set, &signal);
		printf("sigwait signal: %d\n", signal);
	}

	sleep(1);

	printf("Unblock\n");
	sigprocmask(SIG_UNBLOCK, &set, NULL);

	sigpending(&pend);
	if (sigismember(&pend, sig)) {
		/* This is not reached */
		printf("Timer signal pending 2\n");
		sigwait(&set, &signal);
		printf("sigwait signal: %d\n", signal);
	}

	sleep(1);

	sigpending(&pend);
	if (sigismember(&pend, sig)) {
		/* This is not reached */
		printf("Timer signal pending 3\n");
		sigwait(&set, &signal);
		printf("sigwait signal: %d\n", signal);
	}

	timer_delete(timerid);
	return 0;
}

Re: signals: Bug or manpage inconsistency?

[Thomas Gleixner]

On Tue, 30 May 2017, Thomas Gleixner wrote:
> The commit which added the queuing of blocked and ignored signals is in the
> history tree with a pretty useless changelog.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
> 
>  commit 98fc8ab9e74389e0c7001052597f61336dc62833
>  Author: Linus Torvalds <torvalds-ZsETY1VsSgIuF7duwytmx4H6Mc4MB0Vx@public.gmane.org>
>  Date:   Tue Feb 11 20:49:03 2003 -0800
> 
>      Don't wake up processes unnecessarily for ignored signals
> 
> It rewrites sig_ignored() and adds the following to it:
> 
> +       /*
> +        * Blocked signals are never ignored, since the
> +        * signal handler may change by the time it is
> +        * unblocked.
> +        */
> +       if (sigismember(&t->blocked, sig))
> +               return 0;
> 
> I have no idea how that is related to $subject of the commit and why this
> decision was made.
> 
> Linus, any recollection?

So I found at least some explanation by studying the spec some more.

There are two variants of ignored signals:

  1) handler is SIG_IGN

  2) handler is SIG_DFL and default action is 'ignore'

     These are the signals in SIG_KERNEL_IGNORE_MASK 

     #define SIG_KERNEL_IGNORE_MASK (\
       	rt_sigmask(SIGCONT)   |  rt_sigmask(SIGCHLD)   | \
        rt_sigmask(SIGWINCH)  |  rt_sigmask(SIGURG)    )

     These signals are not allowed to be discarded when the signal is
     blocked.

So my understanding of the spec is:

  #1 Can discard the signals as long as SIG_IGN is set whether the signal
     is blocked or not

  #2 Must queue them if the signal is blocked, otherwise discard

I changed the logic according to this with the patch below and a quick test
run of lpt and glibc test cases produces no failures.

Thoughts?

Thanks,

	tglx

8<--------------------
Subject: signals: Reduce scope of blocked signals in sig_handler_ignored()
From: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
Date: Tue, 30 May 2017 18:01:33 +0200

Add proper changelog and a big fat comment in the code.

Not-yet-signed-off-by: Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org>
---
 kernel/signal.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Index: b/kernel/signal.c
===================================================================
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -70,6 +70,13 @@ static int sig_handler_ignored(void __us
 		(handler == SIG_DFL && sig_kernel_ignore(sig));
 }
 
+static int sig_handler_is_sigign(struct task_struct *t, int sig)
+{
+	void __user *handler = sig_handler(t, sig);
+
+	return handler == SIG_IGN;
+}
+
 static int sig_task_ignored(struct task_struct *t, int sig, bool force)
 {
 	void __user *handler;
@@ -91,7 +98,7 @@ static int sig_ignored(struct task_struc
 	 * unblocked.
 	 */
 	if (sigismember(&t->blocked, sig) || sigismember(&t->real_blocked, sig))
-		return 0;
+		return sig_handler_is_sigign(t, sig);
 
 	if (!sig_task_ignored(t, sig, force))
 		return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: signals: Bug or manpage inconsistency?

[Thomas Gleixner]

On Tue, 30 May 2017, Thomas Gleixner wrote:
> The commit which added the queuing of blocked and ignored signals is in the
> history tree with a pretty useless changelog.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
> 
>  commit 98fc8ab9e74389e0c7001052597f61336dc62833
>  Author: Linus Torvalds <torvalds@penguin.transmeta.com>
>  Date:   Tue Feb 11 20:49:03 2003 -0800
> 
>      Don't wake up processes unnecessarily for ignored signals
> 
> It rewrites sig_ignored() and adds the following to it:
> 
> +       /*
> +        * Blocked signals are never ignored, since the
> +        * signal handler may change by the time it is
> +        * unblocked.
> +        */
> +       if (sigismember(&t->blocked, sig))
> +               return 0;
> 
> I have no idea how that is related to $subject of the commit and why this
> decision was made.
> 
> Linus, any recollection?

So I found at least some explanation by studying the spec some more.

There are two variants of ignored signals:

  1) handler is SIG_IGN

  2) handler is SIG_DFL and default action is 'ignore'

     These are the signals in SIG_KERNEL_IGNORE_MASK 

     #define SIG_KERNEL_IGNORE_MASK (\
       	rt_sigmask(SIGCONT)   |  rt_sigmask(SIGCHLD)   | \
        rt_sigmask(SIGWINCH)  |  rt_sigmask(SIGURG)    )

     These signals are not allowed to be discarded when the signal is
     blocked.

So my understanding of the spec is:

  #1 Can discard the signals as long as SIG_IGN is set whether the signal
     is blocked or not

  #2 Must queue them if the signal is blocked, otherwise discard

I changed the logic according to this with the patch below and a quick test
run of lpt and glibc test cases produces no failures.

Thoughts?

Thanks,

	tglx

8<--------------------
Subject: signals: Reduce scope of blocked signals in sig_handler_ignored()
From: Thomas Gleixner <tglx@linutronix.de>
Date: Tue, 30 May 2017 18:01:33 +0200

Add proper changelog and a big fat comment in the code.

Not-yet-signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 kernel/signal.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Index: b/kernel/signal.c
===================================================================
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -70,6 +70,13 @@ static int sig_handler_ignored(void __us
 		(handler == SIG_DFL && sig_kernel_ignore(sig));
 }
 
+static int sig_handler_is_sigign(struct task_struct *t, int sig)
+{
+	void __user *handler = sig_handler(t, sig);
+
+	return handler == SIG_IGN;
+}
+
 static int sig_task_ignored(struct task_struct *t, int sig, bool force)
 {
 	void __user *handler;
@@ -91,7 +98,7 @@ static int sig_ignored(struct task_struc
 	 * unblocked.
 	 */
 	if (sigismember(&t->blocked, sig) || sigismember(&t->real_blocked, sig))
-		return 0;
+		return sig_handler_is_sigign(t, sig);
 
 	if (!sig_task_ignored(t, sig, force))
 		return 0;

Re: signals: Bug or manpage inconsistency?

[Oleg Nesterov]

On 05/30, Thomas Gleixner wrote:
>
> So I found at least some explanation by studying the spec some more.
>
> There are two variants of ignored signals:
>
>   1) handler is SIG_IGN
>
>   2) handler is SIG_DFL and default action is 'ignore'

Yes, and note that sys_rt_sigaction() discard the pending signal in both cases.
So even with this change the logic won't look 100% consistent.

I can't comment, I never tried to understand the rationality behind the current
behaviour. But at least the sending path should never drop a blocked SIG_DFL
signal, there is no other way to ensure you won't miss a signal during exec.

> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -70,6 +70,13 @@ static int sig_handler_ignored(void __us
>  		(handler == SIG_DFL && sig_kernel_ignore(sig));
>  }
>
> +static int sig_handler_is_sigign(struct task_struct *t, int sig)
> +{
> +	void __user *handler = sig_handler(t, sig);
> +
> +	return handler == SIG_IGN;
> +}
> +
>  static int sig_task_ignored(struct task_struct *t, int sig, bool force)
>  {
>  	void __user *handler;
> @@ -91,7 +98,7 @@ static int sig_ignored(struct task_struc
>  	 * unblocked.
>  	 */
>  	if (sigismember(&t->blocked, sig) || sigismember(&t->real_blocked, sig))
> -		return 0;
> +		return sig_handler_is_sigign(t, sig);

we can probably make a simpler change, but this doesn't matter.

Obviously this is a user-visible change and it can break something. Say, an
application does sigwaitinfo(SIGCHLD) and SIGCHLD is ignored (SIG_IGN), this
will no longer work.

I won't argue, but perhaps it is too late change this historical behaviour.




Although perhaps we can cleanup do_sigtimedwait() for the start. ->real_blocked
doesn't look nice. I think we can replace it with task->sigwait_mask and then
change sig_handler() to do

	sigismember(sigwait_mask, sig) ? SIG_ERR :
		t->sighand->action[sig - 1].sa.sa_handler;

this needs other changes, say, sig_fatal() will need to use sig_handler() too.
Then it would be more safe to drop the SIG_IGN signals unconditionally.

Oleg.

Re: signals: Bug or manpage inconsistency?

[Linus Torvalds]

On Tue, May 30, 2017 at 10:04 AM, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
> I can't comment, I never tried to understand the rationality behind the current
> behaviour. But at least the sending path should never drop a blocked SIG_DFL
> signal, there is no other way to ensure you won't miss a signal during exec.

Note that both SIG_DFL _and_ SIG_IGN are possible after exec, so if
you don't want to drop particular signals to the new process (which
may then add its own handler and want them), using the signal blocked
mask is the rigth thing to do for both of them,

SIG_IGN doesn't mean "ignore signal forever". It means "ignore signals
right now", and I think that our current signal blocking semantics are
likely the correct ones, exactly because it means "when you start
blocking signals, the kernel will not drop them".

There is no difference wrt SIG_DFL and SIG_IGN in this sense.

> Obviously this is a user-visible change and it can break something. Say, an
> application does sigwaitinfo(SIGCHLD) and SIGCHLD is ignored (SIG_IGN), this
> will no longer work.

That's an interesting special case. Yes, SIG_IGN actually has magical
properties wrt SIGCHLD. It basically means the opposite of ignoring
it, it's an "implicit signal handler".  So I could imagine people
using SIG_IGN to avoid the signal handler, but then block SIG_CHLD and
using sigwait() for it.

That sounds nonportable as hell, but I could imagine people doing it
because it happens to work.

So again, I really wouldn't want to change existing semantics unless
there is a big real reason for it. Our current semantics are not
wrong.

                Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: signals: Bug or manpage inconsistency?

[Linus Torvalds]

On Tue, May 30, 2017 at 10:04 AM, Oleg Nesterov <oleg@redhat.com> wrote:
>
> I can't comment, I never tried to understand the rationality behind the current
> behaviour. But at least the sending path should never drop a blocked SIG_DFL
> signal, there is no other way to ensure you won't miss a signal during exec.

Note that both SIG_DFL _and_ SIG_IGN are possible after exec, so if
you don't want to drop particular signals to the new process (which
may then add its own handler and want them), using the signal blocked
mask is the rigth thing to do for both of them,

SIG_IGN doesn't mean "ignore signal forever". It means "ignore signals
right now", and I think that our current signal blocking semantics are
likely the correct ones, exactly because it means "when you start
blocking signals, the kernel will not drop them".

There is no difference wrt SIG_DFL and SIG_IGN in this sense.

> Obviously this is a user-visible change and it can break something. Say, an
> application does sigwaitinfo(SIGCHLD) and SIGCHLD is ignored (SIG_IGN), this
> will no longer work.

That's an interesting special case. Yes, SIG_IGN actually has magical
properties wrt SIGCHLD. It basically means the opposite of ignoring
it, it's an "implicit signal handler".  So I could imagine people
using SIG_IGN to avoid the signal handler, but then block SIG_CHLD and
using sigwait() for it.

That sounds nonportable as hell, but I could imagine people doing it
because it happens to work.

So again, I really wouldn't want to change existing semantics unless
there is a big real reason for it. Our current semantics are not
wrong.

                Linus

Re: signals: Bug or manpage inconsistency?

[Oleg Nesterov]

On 05/30, Linus Torvalds wrote:
>
> On Tue, May 30, 2017 at 10:04 AM, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> >
> > I can't comment, I never tried to understand the rationality behind the current
> > behaviour. But at least the sending path should never drop a blocked SIG_DFL
> > signal, there is no other way to ensure you won't miss a signal during exec.
>
> Note that both SIG_DFL _and_ SIG_IGN are possible after exec,

Yes, if it was already ignored before exec. But ignoring the compatibility the
only important case is when it is SIG_DFL because of flush_signal_handlers().

> SIG_IGN doesn't mean "ignore signal forever". It means "ignore signals
> right now", and I think that our current signal blocking semantics are
> likely the correct ones,

I am not saying it is incorrect, but I agree with Thomas in that this
sigismember(t->blocked) in sig_ignored() doesn't look really nice.

> exactly because it means "when you start
> blocking signals, the kernel will not drop them".

if the process is singe-threaded or the signal is private, or it is blocked
by all threads. Otherwise it will wakeup another thread for no reason, the
signal will be dropped in get_signal().

And again, this doesn't look consistent with do_sigaction(). It even has a
comment which explains that we want to flush the ignored signals, blocked
or not.

Nevermind, I am not trying to argue, and

> So again, I really wouldn't want to change existing semantics unless
> there is a big real reason for it. Our current semantics are not
> wrong.

I certainly agree.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: signals: Bug or manpage inconsistency?

[Oleg Nesterov]

On 05/30, Linus Torvalds wrote:
>
> On Tue, May 30, 2017 at 10:04 AM, Oleg Nesterov <oleg@redhat.com> wrote:
> >
> > I can't comment, I never tried to understand the rationality behind the current
> > behaviour. But at least the sending path should never drop a blocked SIG_DFL
> > signal, there is no other way to ensure you won't miss a signal during exec.
>
> Note that both SIG_DFL _and_ SIG_IGN are possible after exec,

Yes, if it was already ignored before exec. But ignoring the compatibility the
only important case is when it is SIG_DFL because of flush_signal_handlers().

> SIG_IGN doesn't mean "ignore signal forever". It means "ignore signals
> right now", and I think that our current signal blocking semantics are
> likely the correct ones,

I am not saying it is incorrect, but I agree with Thomas in that this
sigismember(t->blocked) in sig_ignored() doesn't look really nice.

> exactly because it means "when you start
> blocking signals, the kernel will not drop them".

if the process is singe-threaded or the signal is private, or it is blocked
by all threads. Otherwise it will wakeup another thread for no reason, the
signal will be dropped in get_signal().

And again, this doesn't look consistent with do_sigaction(). It even has a
comment which explains that we want to flush the ignored signals, blocked
or not.

Nevermind, I am not trying to argue, and

> So again, I really wouldn't want to change existing semantics unless
> there is a big real reason for it. Our current semantics are not
> wrong.

I certainly agree.

Oleg.

Re: signals: Bug or manpage inconsistency?

[Thomas Gleixner]

On Tue, 30 May 2017, Linus Torvalds wrote:
> On Tue, May 30, 2017 at 10:04 AM, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > Obviously this is a user-visible change and it can break something. Say, an
> > application does sigwaitinfo(SIGCHLD) and SIGCHLD is ignored (SIG_IGN), this
> > will no longer work.
> 
> That's an interesting special case. Yes, SIG_IGN actually has magical
> properties wrt SIGCHLD. It basically means the opposite of ignoring
> it, it's an "implicit signal handler".  So I could imagine people
> using SIG_IGN to avoid the signal handler, but then block SIG_CHLD and
> using sigwait() for it.
> 
> That sounds nonportable as hell, but I could imagine people doing it
> because it happens to work.

Just that it does not work. See do_notify_parent()

	if (!tsk->ptrace && sig == SIGCHLD &&
	    (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN ||
	     (psig->action[SIGCHLD-1].sa.sa_flags & SA_NOCLDWAIT))) {
		/*
		 * We are exiting and our parent doesn't care.  POSIX.1
		 * defines special semantics for setting SIGCHLD to SIG_IGN
		 * or setting the SA_NOCLDWAIT flag: we should be reaped
		 * automatically and not left for our parent's wait4 call.
		 * Rather than having the parent do it as a magic kind of
		 * signal handler, we just set this to tell do_exit that we
		 * can be cleaned up without becoming a zombie.  Note that
		 * we still call __wake_up_parent in this case, because a
		 * blocked sys_wait4 might now return -ECHILD.
		 *
		 * Whether we send SIGCHLD or not for SA_NOCLDWAIT
		 * is implementation-defined: we do (if you don't want
		 * it, just use SIG_IGN instead).
		 */
		autoreap = true;
		if (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN)
			sig = 0;
	}
        if (valid_signal(sig) && sig)
                __group_send_sig_info(sig, &info, tsk->parent);

So if the oarent has SIG_IGN we do not send a signal at all. So it's not a
really interesting special case and the magic properties are not that magic
either. Test case below. The parent waits forever.

Thanks,

	tglx
---

#include <unistd.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/wait.h>

int main(void)
{
	struct sigaction action;
	sigset_t set;
	int signum;

	sigemptyset(&set);
	sigaddset (&set, SIGCHLD);

	memset(&action, 0, sizeof(action));
	action.sa_handler = SIG_IGN;
	sigaction(SIGCHLD, &action, NULL);

	sigprocmask(SIG_BLOCK, &set, NULL);

	if (fork() == 0) {
		sleep(1);
		printf("Child exiting\n");
		exit(0);
	}

	sigwait(&set, &signum);
	printf("Parent exiting\n");
	return 0;
}



--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: signals: Bug or manpage inconsistency?

[Thomas Gleixner]

On Tue, 30 May 2017, Linus Torvalds wrote:
> On Tue, May 30, 2017 at 10:04 AM, Oleg Nesterov <oleg@redhat.com> wrote:
> > Obviously this is a user-visible change and it can break something. Say, an
> > application does sigwaitinfo(SIGCHLD) and SIGCHLD is ignored (SIG_IGN), this
> > will no longer work.
> 
> That's an interesting special case. Yes, SIG_IGN actually has magical
> properties wrt SIGCHLD. It basically means the opposite of ignoring
> it, it's an "implicit signal handler".  So I could imagine people
> using SIG_IGN to avoid the signal handler, but then block SIG_CHLD and
> using sigwait() for it.
> 
> That sounds nonportable as hell, but I could imagine people doing it
> because it happens to work.

Just that it does not work. See do_notify_parent()

	if (!tsk->ptrace && sig == SIGCHLD &&
	    (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN ||
	     (psig->action[SIGCHLD-1].sa.sa_flags & SA_NOCLDWAIT))) {
		/*
		 * We are exiting and our parent doesn't care.  POSIX.1
		 * defines special semantics for setting SIGCHLD to SIG_IGN
		 * or setting the SA_NOCLDWAIT flag: we should be reaped
		 * automatically and not left for our parent's wait4 call.
		 * Rather than having the parent do it as a magic kind of
		 * signal handler, we just set this to tell do_exit that we
		 * can be cleaned up without becoming a zombie.  Note that
		 * we still call __wake_up_parent in this case, because a
		 * blocked sys_wait4 might now return -ECHILD.
		 *
		 * Whether we send SIGCHLD or not for SA_NOCLDWAIT
		 * is implementation-defined: we do (if you don't want
		 * it, just use SIG_IGN instead).
		 */
		autoreap = true;
		if (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN)
			sig = 0;
	}
        if (valid_signal(sig) && sig)
                __group_send_sig_info(sig, &info, tsk->parent);

So if the oarent has SIG_IGN we do not send a signal at all. So it's not a
really interesting special case and the magic properties are not that magic
either. Test case below. The parent waits forever.

Thanks,

	tglx
---

#include <unistd.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/wait.h>

int main(void)
{
	struct sigaction action;
	sigset_t set;
	int signum;

	sigemptyset(&set);
	sigaddset (&set, SIGCHLD);

	memset(&action, 0, sizeof(action));
	action.sa_handler = SIG_IGN;
	sigaction(SIGCHLD, &action, NULL);

	sigprocmask(SIG_BLOCK, &set, NULL);

	if (fork() == 0) {
		sleep(1);
		printf("Child exiting\n");
		exit(0);
	}

	sigwait(&set, &signum);
	printf("Parent exiting\n");
	return 0;
}

Re: signals: Bug or manpage inconsistency?

[Eric W. Biederman]

Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> writes:

> On Tue, 30 May 2017, Linus Torvalds wrote:
>> On Tue, May 30, 2017 at 10:04 AM, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> > Obviously this is a user-visible change and it can break something. Say, an
>> > application does sigwaitinfo(SIGCHLD) and SIGCHLD is ignored (SIG_IGN), this
>> > will no longer work.
>> 
>> That's an interesting special case. Yes, SIG_IGN actually has magical
>> properties wrt SIGCHLD. It basically means the opposite of ignoring
>> it, it's an "implicit signal handler".  So I could imagine people
>> using SIG_IGN to avoid the signal handler, but then block SIG_CHLD and
>> using sigwait() for it.
>> 
>> That sounds nonportable as hell, but I could imagine people doing it
>> because it happens to work.
>
> Just that it does not work. See do_notify_parent()
>
> 	if (!tsk->ptrace && sig == SIGCHLD &&
> 	    (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN ||
> 	     (psig->action[SIGCHLD-1].sa.sa_flags & SA_NOCLDWAIT))) {
> 		/*
> 		 * We are exiting and our parent doesn't care.  POSIX.1
> 		 * defines special semantics for setting SIGCHLD to SIG_IGN
> 		 * or setting the SA_NOCLDWAIT flag: we should be reaped
> 		 * automatically and not left for our parent's wait4 call.
> 		 * Rather than having the parent do it as a magic kind of
> 		 * signal handler, we just set this to tell do_exit that we
> 		 * can be cleaned up without becoming a zombie.  Note that
> 		 * we still call __wake_up_parent in this case, because a
> 		 * blocked sys_wait4 might now return -ECHILD.
> 		 *
> 		 * Whether we send SIGCHLD or not for SA_NOCLDWAIT
> 		 * is implementation-defined: we do (if you don't want
> 		 * it, just use SIG_IGN instead).
> 		 */
> 		autoreap = true;
> 		if (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN)
> 			sig = 0;
> 	}
>         if (valid_signal(sig) && sig)
>                 __group_send_sig_info(sig, &info, tsk->parent);
>
> So if the oarent has SIG_IGN we do not send a signal at all. So it's not a
> really interesting special case and the magic properties are not that magic
> either. Test case below. The parent waits forever.

Which would suggests that to be consistent we should ignore
blocks for other signals on send when the signal handler is SIG_IGN.

Hmm.

For blocked signals because there is only one siginfo ever allocated
as I read it the code naturally blocks the signal until it is
dequeued and rearmed.

I suspect what you want to do is a little more in the magic
dequeue_signal for timers and look if the signal handler
is SIG_IGN.  I think the clean solution would be to
treat timers whose signal handler is SIG_IGN as blocked
signals and simply not dequeue them.

If they are not dequeued they won't reschedule and won't restart.
Then when the signal handler finally changes you immediately get
one pending signal and then the timers fire normally.

That gets tricky though because the signal numbers are not dedicated
to posix timers.

It might instead require noting that the handler is SIG_IGN when
dequeued and simply disabled the timer.  With an enable that kicks
in when someone calls sigaction and changes the handler.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: signals: Bug or manpage inconsistency?

[Eric W. Biederman]

Thomas Gleixner <tglx@linutronix.de> writes:

> On Tue, 30 May 2017, Linus Torvalds wrote:
>> On Tue, May 30, 2017 at 10:04 AM, Oleg Nesterov <oleg@redhat.com> wrote:
>> > Obviously this is a user-visible change and it can break something. Say, an
>> > application does sigwaitinfo(SIGCHLD) and SIGCHLD is ignored (SIG_IGN), this
>> > will no longer work.
>> 
>> That's an interesting special case. Yes, SIG_IGN actually has magical
>> properties wrt SIGCHLD. It basically means the opposite of ignoring
>> it, it's an "implicit signal handler".  So I could imagine people
>> using SIG_IGN to avoid the signal handler, but then block SIG_CHLD and
>> using sigwait() for it.
>> 
>> That sounds nonportable as hell, but I could imagine people doing it
>> because it happens to work.
>
> Just that it does not work. See do_notify_parent()
>
> 	if (!tsk->ptrace && sig == SIGCHLD &&
> 	    (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN ||
> 	     (psig->action[SIGCHLD-1].sa.sa_flags & SA_NOCLDWAIT))) {
> 		/*
> 		 * We are exiting and our parent doesn't care.  POSIX.1
> 		 * defines special semantics for setting SIGCHLD to SIG_IGN
> 		 * or setting the SA_NOCLDWAIT flag: we should be reaped
> 		 * automatically and not left for our parent's wait4 call.
> 		 * Rather than having the parent do it as a magic kind of
> 		 * signal handler, we just set this to tell do_exit that we
> 		 * can be cleaned up without becoming a zombie.  Note that
> 		 * we still call __wake_up_parent in this case, because a
> 		 * blocked sys_wait4 might now return -ECHILD.
> 		 *
> 		 * Whether we send SIGCHLD or not for SA_NOCLDWAIT
> 		 * is implementation-defined: we do (if you don't want
> 		 * it, just use SIG_IGN instead).
> 		 */
> 		autoreap = true;
> 		if (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN)
> 			sig = 0;
> 	}
>         if (valid_signal(sig) && sig)
>                 __group_send_sig_info(sig, &info, tsk->parent);
>
> So if the oarent has SIG_IGN we do not send a signal at all. So it's not a
> really interesting special case and the magic properties are not that magic
> either. Test case below. The parent waits forever.

Which would suggests that to be consistent we should ignore
blocks for other signals on send when the signal handler is SIG_IGN.

Hmm.

For blocked signals because there is only one siginfo ever allocated
as I read it the code naturally blocks the signal until it is
dequeued and rearmed.

I suspect what you want to do is a little more in the magic
dequeue_signal for timers and look if the signal handler
is SIG_IGN.  I think the clean solution would be to
treat timers whose signal handler is SIG_IGN as blocked
signals and simply not dequeue them.

If they are not dequeued they won't reschedule and won't restart.
Then when the signal handler finally changes you immediately get
one pending signal and then the timers fire normally.

That gets tricky though because the signal numbers are not dedicated
to posix timers.

It might instead require noting that the handler is SIG_IGN when
dequeued and simply disabled the timer.  With an enable that kicks
in when someone calls sigaction and changes the handler.

Eric

Re: signals: Bug or manpage inconsistency?

[Eric W. Biederman]

ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman) writes:

> Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> writes:
>
>> On Tue, 30 May 2017, Linus Torvalds wrote:
>>> On Tue, May 30, 2017 at 10:04 AM, Oleg Nesterov <oleg-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>>> > Obviously this is a user-visible change and it can break something. Say, an
>>> > application does sigwaitinfo(SIGCHLD) and SIGCHLD is ignored (SIG_IGN), this
>>> > will no longer work.
>>> 
>>> That's an interesting special case. Yes, SIG_IGN actually has magical
>>> properties wrt SIGCHLD. It basically means the opposite of ignoring
>>> it, it's an "implicit signal handler".  So I could imagine people
>>> using SIG_IGN to avoid the signal handler, but then block SIG_CHLD and
>>> using sigwait() for it.
>>> 
>>> That sounds nonportable as hell, but I could imagine people doing it
>>> because it happens to work.
>>
>> Just that it does not work. See do_notify_parent()
>>
>> 	if (!tsk->ptrace && sig == SIGCHLD &&
>> 	    (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN ||
>> 	     (psig->action[SIGCHLD-1].sa.sa_flags & SA_NOCLDWAIT))) {
>> 		/*
>> 		 * We are exiting and our parent doesn't care.  POSIX.1
>> 		 * defines special semantics for setting SIGCHLD to SIG_IGN
>> 		 * or setting the SA_NOCLDWAIT flag: we should be reaped
>> 		 * automatically and not left for our parent's wait4 call.
>> 		 * Rather than having the parent do it as a magic kind of
>> 		 * signal handler, we just set this to tell do_exit that we
>> 		 * can be cleaned up without becoming a zombie.  Note that
>> 		 * we still call __wake_up_parent in this case, because a
>> 		 * blocked sys_wait4 might now return -ECHILD.
>> 		 *
>> 		 * Whether we send SIGCHLD or not for SA_NOCLDWAIT
>> 		 * is implementation-defined: we do (if you don't want
>> 		 * it, just use SIG_IGN instead).
>> 		 */
>> 		autoreap = true;
>> 		if (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN)
>> 			sig = 0;
>> 	}
>>         if (valid_signal(sig) && sig)
>>                 __group_send_sig_info(sig, &info, tsk->parent);
>>
>> So if the oarent has SIG_IGN we do not send a signal at all. So it's not a
>> really interesting special case and the magic properties are not that magic
>> either. Test case below. The parent waits forever.
>
> Which would suggests that to be consistent we should ignore
> blocks for other signals on send when the signal handler is SIG_IGN.
>
> Hmm.
>
> For blocked signals because there is only one siginfo ever allocated
> as I read it the code naturally blocks the signal until it is
> dequeued and rearmed.
>
> I suspect what you want to do is a little more in the magic
> dequeue_signal for timers and look if the signal handler
> is SIG_IGN.  I think the clean solution would be to
> treat timers whose signal handler is SIG_IGN as blocked
> signals and simply not dequeue them.
>
> If they are not dequeued they won't reschedule and won't restart.
> Then when the signal handler finally changes you immediately get
> one pending signal and then the timers fire normally.
>
> That gets tricky though because the signal numbers are not dedicated
> to posix timers.
>
> It might instead require noting that the handler is SIG_IGN when
> dequeued and simply disabled the timer.  With an enable that kicks
> in when someone calls sigaction and changes the handler.

The point my tired brain is making is that I don't think you actually
care about SIG_IGN vs blocked signals.

Sigh.  But then again you have two places to worry about blocked
signals.  From send_siqueue telling you the signals are ignored
and from signals being dequeued and ignored with dequeue_signal
in get_signal, do_sigtimedwait, and signalfd_dequeue.

Now I see why you are asking about semantics.  If send_siqueue could
always look at SIG_IGN you would only have one spot to worry about.

However the big practical question is if you can block those signals
and pick them up with sigtimedwait or with signalfd.  It looks like
you can today as neither sigtimedwait or signalfd care if the signal
handler can be set to SIG_IGN.

Which means you wind up having 4 places you need to deal with ignored
signals.  send_sigque, get_signal, do_sigtimedwait, and
signalfd_dequeue.  It feels like it would be nice to move the timer
requeue out of dequeue_signal and into it's callers, with an appropriate
set of helpers.

Sigh.

I hope that helps a little.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: signals: Bug or manpage inconsistency?

[Eric W. Biederman]

ebiederm@xmission.com (Eric W. Biederman) writes:

> Thomas Gleixner <tglx@linutronix.de> writes:
>
>> On Tue, 30 May 2017, Linus Torvalds wrote:
>>> On Tue, May 30, 2017 at 10:04 AM, Oleg Nesterov <oleg@redhat.com> wrote:
>>> > Obviously this is a user-visible change and it can break something. Say, an
>>> > application does sigwaitinfo(SIGCHLD) and SIGCHLD is ignored (SIG_IGN), this
>>> > will no longer work.
>>> 
>>> That's an interesting special case. Yes, SIG_IGN actually has magical
>>> properties wrt SIGCHLD. It basically means the opposite of ignoring
>>> it, it's an "implicit signal handler".  So I could imagine people
>>> using SIG_IGN to avoid the signal handler, but then block SIG_CHLD and
>>> using sigwait() for it.
>>> 
>>> That sounds nonportable as hell, but I could imagine people doing it
>>> because it happens to work.
>>
>> Just that it does not work. See do_notify_parent()
>>
>> 	if (!tsk->ptrace && sig == SIGCHLD &&
>> 	    (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN ||
>> 	     (psig->action[SIGCHLD-1].sa.sa_flags & SA_NOCLDWAIT))) {
>> 		/*
>> 		 * We are exiting and our parent doesn't care.  POSIX.1
>> 		 * defines special semantics for setting SIGCHLD to SIG_IGN
>> 		 * or setting the SA_NOCLDWAIT flag: we should be reaped
>> 		 * automatically and not left for our parent's wait4 call.
>> 		 * Rather than having the parent do it as a magic kind of
>> 		 * signal handler, we just set this to tell do_exit that we
>> 		 * can be cleaned up without becoming a zombie.  Note that
>> 		 * we still call __wake_up_parent in this case, because a
>> 		 * blocked sys_wait4 might now return -ECHILD.
>> 		 *
>> 		 * Whether we send SIGCHLD or not for SA_NOCLDWAIT
>> 		 * is implementation-defined: we do (if you don't want
>> 		 * it, just use SIG_IGN instead).
>> 		 */
>> 		autoreap = true;
>> 		if (psig->action[SIGCHLD-1].sa.sa_handler == SIG_IGN)
>> 			sig = 0;
>> 	}
>>         if (valid_signal(sig) && sig)
>>                 __group_send_sig_info(sig, &info, tsk->parent);
>>
>> So if the oarent has SIG_IGN we do not send a signal at all. So it's not a
>> really interesting special case and the magic properties are not that magic
>> either. Test case below. The parent waits forever.
>
> Which would suggests that to be consistent we should ignore
> blocks for other signals on send when the signal handler is SIG_IGN.
>
> Hmm.
>
> For blocked signals because there is only one siginfo ever allocated
> as I read it the code naturally blocks the signal until it is
> dequeued and rearmed.
>
> I suspect what you want to do is a little more in the magic
> dequeue_signal for timers and look if the signal handler
> is SIG_IGN.  I think the clean solution would be to
> treat timers whose signal handler is SIG_IGN as blocked
> signals and simply not dequeue them.
>
> If they are not dequeued they won't reschedule and won't restart.
> Then when the signal handler finally changes you immediately get
> one pending signal and then the timers fire normally.
>
> That gets tricky though because the signal numbers are not dedicated
> to posix timers.
>
> It might instead require noting that the handler is SIG_IGN when
> dequeued and simply disabled the timer.  With an enable that kicks
> in when someone calls sigaction and changes the handler.

The point my tired brain is making is that I don't think you actually
care about SIG_IGN vs blocked signals.

Sigh.  But then again you have two places to worry about blocked
signals.  From send_siqueue telling you the signals are ignored
and from signals being dequeued and ignored with dequeue_signal
in get_signal, do_sigtimedwait, and signalfd_dequeue.

Now I see why you are asking about semantics.  If send_siqueue could
always look at SIG_IGN you would only have one spot to worry about.

However the big practical question is if you can block those signals
and pick them up with sigtimedwait or with signalfd.  It looks like
you can today as neither sigtimedwait or signalfd care if the signal
handler can be set to SIG_IGN.

Which means you wind up having 4 places you need to deal with ignored
signals.  send_sigque, get_signal, do_sigtimedwait, and
signalfd_dequeue.  It feels like it would be nice to move the timer
requeue out of dequeue_signal and into it's callers, with an appropriate
set of helpers.

Sigh.

I hope that helps a little.

Eric

Re: signals: Bug or manpage inconsistency?

On Tue, May 30, 2017 at 6:21 AM, Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> wrote:
>
> Linus, any recollection?
>
> IMO, it's perfectly reasonable to discard ignored signals even when the
> signal is in the blocked mask. When its unblocked and SIG_IGN is replaced
> then the next signal will be delivered. But hell knows, how much user space
> depends on this weird behaviour by now.

Is there any real reason you care? Because clearly we're doing what
POSIX allows, and I'd be nervous about changing existing behavior.

There are various races wrt signals that happen particularly around
fork/exec, and the way that programs handle those races is to block
signals. I don't know that anybody cares about the exact semantics of
this, but I could *imagine* that they do.

Our current behavior is actually very nice: blocking a signal
basically guarantees that you're now "atomic" wrt that signal. You
won't lose signaling events after the blocking, unless you explicitly
throw them away.

So I would suggest *not* changing the semantics unless you have a
major real reason for wanting to do that.

                     Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: signals: Bug or manpage inconsistency?

[Linus Torvalds]

On Tue, May 30, 2017 at 6:21 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> Linus, any recollection?
>
> IMO, it's perfectly reasonable to discard ignored signals even when the
> signal is in the blocked mask. When its unblocked and SIG_IGN is replaced
> then the next signal will be delivered. But hell knows, how much user space
> depends on this weird behaviour by now.

Is there any real reason you care? Because clearly we're doing what
POSIX allows, and I'd be nervous about changing existing behavior.

There are various races wrt signals that happen particularly around
fork/exec, and the way that programs handle those races is to block
signals. I don't know that anybody cares about the exact semantics of
this, but I could *imagine* that they do.

Our current behavior is actually very nice: blocking a signal
basically guarantees that you're now "atomic" wrt that signal. You
won't lose signaling events after the blocking, unless you explicitly
throw them away.

So I would suggest *not* changing the semantics unless you have a
major real reason for wanting to do that.

                     Linus

Re: signals: Bug or manpage inconsistency?

[Thomas Gleixner]

On Tue, 30 May 2017, Linus Torvalds wrote:
> On Tue, May 30, 2017 at 6:21 AM, Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> wrote:
> >
> > Linus, any recollection?
> >
> > IMO, it's perfectly reasonable to discard ignored signals even when the
> > signal is in the blocked mask. When its unblocked and SIG_IGN is replaced
> > then the next signal will be delivered. But hell knows, how much user space
> > depends on this weird behaviour by now.
> 
> Is there any real reason you care? Because clearly we're doing what
> POSIX allows, and I'd be nervous about changing existing behavior.
> 
> There are various races wrt signals that happen particularly around
> fork/exec, and the way that programs handle those races is to block
> signals. I don't know that anybody cares about the exact semantics of
> this, but I could *imagine* that they do.
> 
> Our current behavior is actually very nice: blocking a signal
> basically guarantees that you're now "atomic" wrt that signal. You
> won't lose signaling events after the blocking, unless you explicitly
> throw them away.

Right, blocking signals which are not set to SIG_IGN makes perfectly sense. The
SIG_IGN case is what bothers me.

If you do

   ignore(sig);
			<- Signal discarded

   sigpending() -> empty

   block(sig);
			<- Signal queued

   sigpending() -> queued signal marked pending

   unblock(sig);
			<- Queued signal dequeued and discarded due to SIG_IGN

   sigpending() -> empty

> So I would suggest *not* changing the semantics unless you have a
> major real reason for wanting to do that.

The reason why I'm looking into that is the silly case with posix interval
timers dealing with ignored signals. We have to keep these timers self
rearming because nothing rearms them when SIG_IGN is lifted. That means
it's a nice way to keep the system busy and the process which armed the
timer is never waking up. We have a crude 'rate limit' of the interval to
one jiffie in place which prevents a unpriviledged DOS attack by arming the
timer with a very small interval. But that turns that mechanism into a
battery drain which is not obvious to figure out.

When I was working on a mitigation for that, which of course involves
fiddling with the signal code, I noticed that issue with sigpending() which
returns the blocked _and_ ignored signal as pending despite the man page
claiming otherwise. That made me look deeper and wonder about the rather
strange and inconsistent semantics of all of this.

Thanks,

	tglx


--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: signals: Bug or manpage inconsistency?

[Thomas Gleixner]

On Tue, 30 May 2017, Linus Torvalds wrote:
> On Tue, May 30, 2017 at 6:21 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> > Linus, any recollection?
> >
> > IMO, it's perfectly reasonable to discard ignored signals even when the
> > signal is in the blocked mask. When its unblocked and SIG_IGN is replaced
> > then the next signal will be delivered. But hell knows, how much user space
> > depends on this weird behaviour by now.
> 
> Is there any real reason you care? Because clearly we're doing what
> POSIX allows, and I'd be nervous about changing existing behavior.
> 
> There are various races wrt signals that happen particularly around
> fork/exec, and the way that programs handle those races is to block
> signals. I don't know that anybody cares about the exact semantics of
> this, but I could *imagine* that they do.
> 
> Our current behavior is actually very nice: blocking a signal
> basically guarantees that you're now "atomic" wrt that signal. You
> won't lose signaling events after the blocking, unless you explicitly
> throw them away.

Right, blocking signals which are not set to SIG_IGN makes perfectly sense. The
SIG_IGN case is what bothers me.

If you do

   ignore(sig);
			<- Signal discarded

   sigpending() -> empty

   block(sig);
			<- Signal queued

   sigpending() -> queued signal marked pending

   unblock(sig);
			<- Queued signal dequeued and discarded due to SIG_IGN

   sigpending() -> empty

> So I would suggest *not* changing the semantics unless you have a
> major real reason for wanting to do that.

The reason why I'm looking into that is the silly case with posix interval
timers dealing with ignored signals. We have to keep these timers self
rearming because nothing rearms them when SIG_IGN is lifted. That means
it's a nice way to keep the system busy and the process which armed the
timer is never waking up. We have a crude 'rate limit' of the interval to
one jiffie in place which prevents a unpriviledged DOS attack by arming the
timer with a very small interval. But that turns that mechanism into a
battery drain which is not obvious to figure out.

When I was working on a mitigation for that, which of course involves
fiddling with the signal code, I noticed that issue with sigpending() which
returns the blocked _and_ ignored signal as pending despite the man page
claiming otherwise. That made me look deeper and wonder about the rather
strange and inconsistent semantics of all of this.

Thanks,

	tglx

Re: signals: Bug or manpage inconsistency?

[Linus Torvalds]

On Tue, May 30, 2017 at 12:35 PM, Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> wrote:
> Right, blocking signals which are not set to SIG_IGN makes perfectly sense. The
> SIG_IGN case is what bothers me.

The thing is, the SIG_IGN may not *remain* a SIG_IGN.

Put another way, let's say that you are a process that uses signals
for some event thing. You have a signal handler for handling the
event, but it may be that you also have a mode in which you are in
some kind of "don't care" phase, and you don't want to be woken up or
have that signal handler even be invoked, so you set the signal to
SIG_IGN.

But then when you want to start listening to the event again, you
might need to set up data structures for the signal handler, and what
you do is

 - block the signal to avoid all race conditions with things that
might come in before you're ready

 - set up your data structures

 - set the new signal handler

 - unblock the signal to make it all "live".

and notice that there's a race condition there: you need to set up
your data structures in order to take the signal sanely, but maybe
because of how you structured things, the act of setting up your data
structures is also what may make the signal start happening!

And when your data structures are set up, you do *not* want to drop
new signals that are starting to come in!

Note how you could not set the new signal handler before your data
structures were ready (because then you'd crash or not be able to
handle them), but also note how you can not just ignore signals.

That's what I meant by the nice "atomic" behavior of blocking the
signal.  With the signal blocked, it now doesn't matter if a signal
happens during that "not completely ready phase" or not.

And notice how SIG_IGN is just one _part_ of that "not completely ready" phase.

Does anybody do this? I really don't know. Maybe not.

But our current code _allows_ you to do it, and actually feels like a
much better model than the one you are trying to argue for.

So I literally think that what we do now is objectively better than
something that ignores the blocking, and just always ignores a signal.

Exactly because that can fundamentally race with the whole "we're now
going to unblock the signal" phase.

With the "signal is blocked means that it gets queued" model, you can
handle that race cleanly.

> The reason why I'm looking into that is the silly case with posix interval
> timers dealing with ignored signals. We have to keep these timers self
> rearming because nothing rearms them when SIG_IGN is lifted.

Why not do SIG_IGN specially at signal generation time, the way
SIGCHLD does. If SIG_IGN is set, you re-arm immediately but do not
actually deliver the signal. It will automatically be re-delivered
next time around (assuming people have by then installed a real signal
handler).

That sounds very much like the SIGCHLD case.

            Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: signals: Bug or manpage inconsistency?

[Linus Torvalds]

On Tue, May 30, 2017 at 12:35 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> Right, blocking signals which are not set to SIG_IGN makes perfectly sense. The
> SIG_IGN case is what bothers me.

The thing is, the SIG_IGN may not *remain* a SIG_IGN.

Put another way, let's say that you are a process that uses signals
for some event thing. You have a signal handler for handling the
event, but it may be that you also have a mode in which you are in
some kind of "don't care" phase, and you don't want to be woken up or
have that signal handler even be invoked, so you set the signal to
SIG_IGN.

But then when you want to start listening to the event again, you
might need to set up data structures for the signal handler, and what
you do is

 - block the signal to avoid all race conditions with things that
might come in before you're ready

 - set up your data structures

 - set the new signal handler

 - unblock the signal to make it all "live".

and notice that there's a race condition there: you need to set up
your data structures in order to take the signal sanely, but maybe
because of how you structured things, the act of setting up your data
structures is also what may make the signal start happening!

And when your data structures are set up, you do *not* want to drop
new signals that are starting to come in!

Note how you could not set the new signal handler before your data
structures were ready (because then you'd crash or not be able to
handle them), but also note how you can not just ignore signals.

That's what I meant by the nice "atomic" behavior of blocking the
signal.  With the signal blocked, it now doesn't matter if a signal
happens during that "not completely ready phase" or not.

And notice how SIG_IGN is just one _part_ of that "not completely ready" phase.

Does anybody do this? I really don't know. Maybe not.

But our current code _allows_ you to do it, and actually feels like a
much better model than the one you are trying to argue for.

So I literally think that what we do now is objectively better than
something that ignores the blocking, and just always ignores a signal.

Exactly because that can fundamentally race with the whole "we're now
going to unblock the signal" phase.

With the "signal is blocked means that it gets queued" model, you can
handle that race cleanly.

> The reason why I'm looking into that is the silly case with posix interval
> timers dealing with ignored signals. We have to keep these timers self
> rearming because nothing rearms them when SIG_IGN is lifted.

Why not do SIG_IGN specially at signal generation time, the way
SIGCHLD does. If SIG_IGN is set, you re-arm immediately but do not
actually deliver the signal. It will automatically be re-delivered
next time around (assuming people have by then installed a real signal
handler).

That sounds very much like the SIGCHLD case.

            Linus

Re: signals: Bug or manpage inconsistency?

[Thomas Gleixner]

On Tue, 30 May 2017, Linus Torvalds wrote:
> On Tue, May 30, 2017 at 12:35 PM, Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> wrote:
> > The reason why I'm looking into that is the silly case with posix interval
> > timers dealing with ignored signals. We have to keep these timers self
> > rearming because nothing rearms them when SIG_IGN is lifted.
> 
> Why not do SIG_IGN specially at signal generation time, the way
> SIGCHLD does. If SIG_IGN is set, you re-arm immediately but do not
> actually deliver the signal. It will automatically be re-delivered
> next time around (assuming people have by then installed a real signal
> handler).

The rearming is exactly the issue. Assume the following:

	t.interval.tv_nsec = 1;
	t.interval.tv_sec = 0;

	timer_set(timerid, 0, &t, NULL);

This creates a 1 nanosecond periodic timer, which is silly to begin with,
but allowed. In the normal case this is automatically rate limited by:

  expire -> queue signal -> wakeup task -> dequeue signal -> rearm

So the scheduler controls how much CPU this task gets because the signal
must be dequeued to rearm the timer. If it's the only task on the system it
will still hog the CPU, but that's the same as if you do while(1).

In the SIG_IGN case it is not, because we need to rearm automatically. So
with that 1 nsec interval you created a DOS attack because the system is
busy expiring and rearming the timer.

The mitigation we have in place is to rate limit that rearming to one
jiffie, so the DOS won't happen.

But that's just a stupid hack. The proper solution would be to rearm the
timer at the point where the SIG_IGN is replaced or for that matter the
ignored signal is blocked.

I'm fine with the current behaviour vs. blocking the ignored signal, I
still think it's inconsistent, but that could be debated forever.

Though it needs to be documented somewhere proper and the man page of
sigpending() needs to be fixed so the next person looking into this starts
scratching his head and asks the same questions again.

So with the existing blocking ignored signal semantics the DOS is mitigated
as well, because the signal is queued and the rearming happens when the
signal is dequeued as in the normal case above, through unblocking or
sigwait().

Now there is a subtle issue with this. The following code sequence will
stop the timer forever:

     block(sig);

	timer expires -> signal is queued -> timer is stopped

     ignore(sig);

	pending signal is discarded

     install_handler(sig);
     unblock(sig);

     Neither the handler install nor the unblocking will restart the timer.

I think I have an idea how to handle both cases proper but I certainly
wanted to have clarity about the semantics before starting.

Thanks,

	tglx






--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: signals: Bug or manpage inconsistency?

[Thomas Gleixner]

On Tue, 30 May 2017, Linus Torvalds wrote:
> On Tue, May 30, 2017 at 12:35 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > The reason why I'm looking into that is the silly case with posix interval
> > timers dealing with ignored signals. We have to keep these timers self
> > rearming because nothing rearms them when SIG_IGN is lifted.
> 
> Why not do SIG_IGN specially at signal generation time, the way
> SIGCHLD does. If SIG_IGN is set, you re-arm immediately but do not
> actually deliver the signal. It will automatically be re-delivered
> next time around (assuming people have by then installed a real signal
> handler).

The rearming is exactly the issue. Assume the following:

	t.interval.tv_nsec = 1;
	t.interval.tv_sec = 0;

	timer_set(timerid, 0, &t, NULL);

This creates a 1 nanosecond periodic timer, which is silly to begin with,
but allowed. In the normal case this is automatically rate limited by:

  expire -> queue signal -> wakeup task -> dequeue signal -> rearm

So the scheduler controls how much CPU this task gets because the signal
must be dequeued to rearm the timer. If it's the only task on the system it
will still hog the CPU, but that's the same as if you do while(1).

In the SIG_IGN case it is not, because we need to rearm automatically. So
with that 1 nsec interval you created a DOS attack because the system is
busy expiring and rearming the timer.

The mitigation we have in place is to rate limit that rearming to one
jiffie, so the DOS won't happen.

But that's just a stupid hack. The proper solution would be to rearm the
timer at the point where the SIG_IGN is replaced or for that matter the
ignored signal is blocked.

I'm fine with the current behaviour vs. blocking the ignored signal, I
still think it's inconsistent, but that could be debated forever.

Though it needs to be documented somewhere proper and the man page of
sigpending() needs to be fixed so the next person looking into this starts
scratching his head and asks the same questions again.

So with the existing blocking ignored signal semantics the DOS is mitigated
as well, because the signal is queued and the rearming happens when the
signal is dequeued as in the normal case above, through unblocking or
sigwait().

Now there is a subtle issue with this. The following code sequence will
stop the timer forever:

     block(sig);

	timer expires -> signal is queued -> timer is stopped

     ignore(sig);

	pending signal is discarded

     install_handler(sig);
     unblock(sig);

     Neither the handler install nor the unblocking will restart the timer.

I think I have an idea how to handle both cases proper but I certainly
wanted to have clarity about the semantics before starting.

Thanks,

	tglx

Re: signals: Bug or manpage inconsistency?

[Michael Kerrisk (man-pages)]

Hi Thomas,

On 05/30/2017 03:21 PM, Thomas Gleixner wrote:
> While trying to address the longstanding FIXME in the posix timer code
> related to ignored signals, I stumbled over the following issue:
> 
> I blocked the signal of the timer, then installed the SIG_IGN handler,
> created and started the timer. After a short sleep the timer has fired
> several times, but it's still ignored AND blocked.
> 
> Calling sigpending() after that has the timer signal set. See test case
> below.
> 
> But 'man sigpending' says:
> 
>    "If a signal is both blocked and has a disposition of "ignored", it is _not_
>     added to the mask of pending signals when generated."
> 
> So something is clearly wrong here.

Yes. I confirm the behavior you saw with your test program. And, I'm a little
surprised by the man page text (mainly because I wrote it :-}).

Here's what I understand as longstanding UNIX behavior: if a blocked signal
is pending, and then we set the disposition to SIG_IGN, the signal is
removed the pending set.

The reason I'm surprised about the man page text is because I think I wrote
it (in 2013) while thinking about that case. I don't recall ever much thinking
about the case "signal is generated while it is blocked and its disposition
has been explicitly set to SIG_IGN" (and the commit message that goes with
the man page change isn't really enlightening as to my thinking either...)

> The same happens with sigwait() while the signal is still blocked and
> ignored, it returns with that signal number and has the signal dequeued.

Yes. And one can also see that the signal is pending in the ShdPnd field
of /proc/PID/status.

> The whole blocked vs. ignored handling is inconsistent both in the posix
> spec and in the kernel.
>
> The only thing vs. ignored signals what the spec mandates is:
> 
>  SIG_IGN:
> 
>  Delivery of the signal shall have no effect on the process.
> 
>  ...
> 
>  Setting a signal action to SIG_IGN for a signal that is pending shall
>  cause the pending signal to be discarded, whether or not it is blocked.

Yes, that's the bit I knew.
 
>  ...
> 
>  Any queued values pending shall be discarded and the resources used to
>  queue them shall be released and made available to queue other signals.
> 
> That's exactly what the kernel does in do_sigaction().
> 
> And for everything else the spec is blurry:
> 
>   If the action associated with a blocked signal is to ignore the signal
>   and if that signal is generated for the process, it is unspecified
>   whether the signal is discarded immediately upon generation or remains
>   pending.
> 
> So the kernel has chosen to keep them pending for whatever reasons, which
> does not make any sense to me, but there is probably a historic reason.
> 
> The commit which added the queuing of blocked and ignored signals is in the
> history tree with a pretty useless changelog.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
> 
>  commit 98fc8ab9e74389e0c7001052597f61336dc62833
>  Author: Linus Torvalds <torvalds-ZsETY1VsSgIuF7duwytmx4H6Mc4MB0Vx@public.gmane.org>
>  Date:   Tue Feb 11 20:49:03 2003 -0800
> 
>      Don't wake up processes unnecessarily for ignored signals
> 
> It rewrites sig_ignored() and adds the following to it:
> 
> +       /*
> +        * Blocked signals are never ignored, since the
> +        * signal handler may change by the time it is
> +        * unblocked.
> +        */
> +       if (sigismember(&t->blocked, sig))
> +               return 0;
> 
> I have no idea how that is related to $subject of the commit and why this
> decision was made.
> 
> Linus, any recollection?
> 
> IMO, it's perfectly reasonable to discard ignored signals even when the
> signal is in the blocked mask. When its unblocked and SIG_IGN is replaced
> then the next signal will be delivered. But hell knows, how much user space
> depends on this weird behaviour by now.

Clearly, something needs to be fixed in the man page. I want to do
a little more investigation first though.

Cheers,

Michael

> 
> Thanks,
> 
> 	tglx
> 	
> 8<-------------
> 
> #define _GNU_SOURCE
> #include <stdint.h>
> #include <stdio.h>
> #include <unistd.h>
> #include <signal.h>
> #include <string.h>
> #include <time.h>
> 
> #include <sys/types.h>
> #include <sys/time.h>
> #include <sys/syscall.h>
> 
> int main(void)
> {
> 	struct itimerspec tspec;
> 	struct sigevent sigev;
> 	struct sigaction action;
> 	int signal, sig = SIGALRM;
> 	sigset_t set, pend;
> 	timer_t timerid;
> 
> 	sigemptyset(&set);
> 	sigaddset(&set, sig);
> 	sigprocmask(SIG_BLOCK, &set, NULL);
> 
> 	memset(&action, 0, sizeof(action));
> 	action.sa_handler = SIG_IGN;
> 	sigaction(sig, &action, NULL);
> 
> 	memset(&sigev, 0, sizeof(sigev));
> 	sigev.sigev_notify = SIGEV_SIGNAL;
> 	sigev.sigev_signo = sig;
> 	sigev.sigev_value.sival_ptr = &timerid;
> 	timer_create(CLOCK_REALTIME, &sigev, &timerid);
> 
> 	tspec.it_interval.tv_sec = 0;
> 	tspec.it_interval.tv_nsec = 100 * 1e6;
> 
> 	tspec.it_value.tv_sec = 0;
> 	tspec.it_value.tv_nsec = 100 * 1e6;
> 
> 	timer_settime(timerid, 0, &tspec, NULL);
> 
> 	sleep(1);
> 
> 	sigpending(&pend);
> 	if (sigismember(&pend, sig)) {
> 		/* This is reached */
> 		printf("Timer signal pending 1\n");
> 		sigwait(&set, &signal);
> 		printf("sigwait signal: %d\n", signal);
> 	}
> 
> 	sleep(1);
> 
> 	printf("Unblock\n");
> 	sigprocmask(SIG_UNBLOCK, &set, NULL);
> 
> 	sigpending(&pend);
> 	if (sigismember(&pend, sig)) {
> 		/* This is not reached */
> 		printf("Timer signal pending 2\n");
> 		sigwait(&set, &signal);
> 		printf("sigwait signal: %d\n", signal);
> 	}
> 
> 	sleep(1);
> 
> 	sigpending(&pend);
> 	if (sigismember(&pend, sig)) {
> 		/* This is not reached */
> 		printf("Timer signal pending 3\n");
> 		sigwait(&set, &signal);
> 		printf("sigwait signal: %d\n", signal);
> 	}
> 
> 	timer_delete(timerid);
> 	return 0;
> }
> 
> 
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: signals: Bug or manpage inconsistency?

[Michael Kerrisk (man-pages)]

Hi Thomas,

On 05/30/2017 03:21 PM, Thomas Gleixner wrote:
> While trying to address the longstanding FIXME in the posix timer code
> related to ignored signals, I stumbled over the following issue:
> 
> I blocked the signal of the timer, then installed the SIG_IGN handler,
> created and started the timer. After a short sleep the timer has fired
> several times, but it's still ignored AND blocked.
> 
> Calling sigpending() after that has the timer signal set. See test case
> below.
> 
> But 'man sigpending' says:
> 
>    "If a signal is both blocked and has a disposition of "ignored", it is _not_
>     added to the mask of pending signals when generated."
> 
> So something is clearly wrong here.

Yes. I confirm the behavior you saw with your test program. And, I'm a little
surprised by the man page text (mainly because I wrote it :-}).

Here's what I understand as longstanding UNIX behavior: if a blocked signal
is pending, and then we set the disposition to SIG_IGN, the signal is
removed the pending set.

The reason I'm surprised about the man page text is because I think I wrote
it (in 2013) while thinking about that case. I don't recall ever much thinking
about the case "signal is generated while it is blocked and its disposition
has been explicitly set to SIG_IGN" (and the commit message that goes with
the man page change isn't really enlightening as to my thinking either...)

> The same happens with sigwait() while the signal is still blocked and
> ignored, it returns with that signal number and has the signal dequeued.

Yes. And one can also see that the signal is pending in the ShdPnd field
of /proc/PID/status.

> The whole blocked vs. ignored handling is inconsistent both in the posix
> spec and in the kernel.
>
> The only thing vs. ignored signals what the spec mandates is:
> 
>  SIG_IGN:
> 
>  Delivery of the signal shall have no effect on the process.
> 
>  ...
> 
>  Setting a signal action to SIG_IGN for a signal that is pending shall
>  cause the pending signal to be discarded, whether or not it is blocked.

Yes, that's the bit I knew.
 
>  ...
> 
>  Any queued values pending shall be discarded and the resources used to
>  queue them shall be released and made available to queue other signals.
> 
> That's exactly what the kernel does in do_sigaction().
> 
> And for everything else the spec is blurry:
> 
>   If the action associated with a blocked signal is to ignore the signal
>   and if that signal is generated for the process, it is unspecified
>   whether the signal is discarded immediately upon generation or remains
>   pending.
> 
> So the kernel has chosen to keep them pending for whatever reasons, which
> does not make any sense to me, but there is probably a historic reason.
> 
> The commit which added the queuing of blocked and ignored signals is in the
> history tree with a pretty useless changelog.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
> 
>  commit 98fc8ab9e74389e0c7001052597f61336dc62833
>  Author: Linus Torvalds <torvalds@penguin.transmeta.com>
>  Date:   Tue Feb 11 20:49:03 2003 -0800
> 
>      Don't wake up processes unnecessarily for ignored signals
> 
> It rewrites sig_ignored() and adds the following to it:
> 
> +       /*
> +        * Blocked signals are never ignored, since the
> +        * signal handler may change by the time it is
> +        * unblocked.
> +        */
> +       if (sigismember(&t->blocked, sig))
> +               return 0;
> 
> I have no idea how that is related to $subject of the commit and why this
> decision was made.
> 
> Linus, any recollection?
> 
> IMO, it's perfectly reasonable to discard ignored signals even when the
> signal is in the blocked mask. When its unblocked and SIG_IGN is replaced
> then the next signal will be delivered. But hell knows, how much user space
> depends on this weird behaviour by now.

Clearly, something needs to be fixed in the man page. I want to do
a little more investigation first though.

Cheers,

Michael

> 
> Thanks,
> 
> 	tglx
> 	
> 8<-------------
> 
> #define _GNU_SOURCE
> #include <stdint.h>
> #include <stdio.h>
> #include <unistd.h>
> #include <signal.h>
> #include <string.h>
> #include <time.h>
> 
> #include <sys/types.h>
> #include <sys/time.h>
> #include <sys/syscall.h>
> 
> int main(void)
> {
> 	struct itimerspec tspec;
> 	struct sigevent sigev;
> 	struct sigaction action;
> 	int signal, sig = SIGALRM;
> 	sigset_t set, pend;
> 	timer_t timerid;
> 
> 	sigemptyset(&set);
> 	sigaddset(&set, sig);
> 	sigprocmask(SIG_BLOCK, &set, NULL);
> 
> 	memset(&action, 0, sizeof(action));
> 	action.sa_handler = SIG_IGN;
> 	sigaction(sig, &action, NULL);
> 
> 	memset(&sigev, 0, sizeof(sigev));
> 	sigev.sigev_notify = SIGEV_SIGNAL;
> 	sigev.sigev_signo = sig;
> 	sigev.sigev_value.sival_ptr = &timerid;
> 	timer_create(CLOCK_REALTIME, &sigev, &timerid);
> 
> 	tspec.it_interval.tv_sec = 0;
> 	tspec.it_interval.tv_nsec = 100 * 1e6;
> 
> 	tspec.it_value.tv_sec = 0;
> 	tspec.it_value.tv_nsec = 100 * 1e6;
> 
> 	timer_settime(timerid, 0, &tspec, NULL);
> 
> 	sleep(1);
> 
> 	sigpending(&pend);
> 	if (sigismember(&pend, sig)) {
> 		/* This is reached */
> 		printf("Timer signal pending 1\n");
> 		sigwait(&set, &signal);
> 		printf("sigwait signal: %d\n", signal);
> 	}
> 
> 	sleep(1);
> 
> 	printf("Unblock\n");
> 	sigprocmask(SIG_UNBLOCK, &set, NULL);
> 
> 	sigpending(&pend);
> 	if (sigismember(&pend, sig)) {
> 		/* This is not reached */
> 		printf("Timer signal pending 2\n");
> 		sigwait(&set, &signal);
> 		printf("sigwait signal: %d\n", signal);
> 	}
> 
> 	sleep(1);
> 
> 	sigpending(&pend);
> 	if (sigismember(&pend, sig)) {
> 		/* This is not reached */
> 		printf("Timer signal pending 3\n");
> 		sigwait(&set, &signal);
> 		printf("sigwait signal: %d\n", signal);
> 	}
> 
> 	timer_delete(timerid);
> 	return 0;
> }
> 
> 
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: signals: Bug or manpage inconsistency?

[Eric W. Biederman]

Thomas Gleixner <tglx-hfZtesqFncYOwBW4kG4KsQ@public.gmane.org> writes:

> While trying to address the longstanding FIXME in the posix timer code
> related to ignored signals, I stumbled over the following issue:
>
> I blocked the signal of the timer, then installed the SIG_IGN handler,
> created and started the timer. After a short sleep the timer has fired
> several times, but it's still ignored AND blocked.
>
> Calling sigpending() after that has the timer signal set. See test case
> below.
>
> But 'man sigpending' says:
>
>    "If a signal is both blocked and has a disposition of "ignored", it is _not_
>     added to the mask of pending signals when generated."
>
> So something is clearly wrong here.
>
> The same happens with sigwait() while the signal is still blocked and
> ignored, it returns with that signal number and has the signal dequeued.
>
>
> The whole blocked vs. ignored handling is inconsistent both in the posix
> spec and in the kernel.
>
> The only thing vs. ignored signals what the spec mandates is:
>
>  SIG_IGN:
>
>  Delivery of the signal shall have no effect on the process.
>
>  ...
>
>  Setting a signal action to SIG_IGN for a signal that is pending shall
>  cause the pending signal to be discarded, whether or not it is blocked.
>
>  ...
>
>  Any queued values pending shall be discarded and the resources used to
>  queue them shall be released and made available to queue other signals.
>
> That's exactly what the kernel does in do_sigaction().
>
> And for everything else the spec is blurry:
>
>   If the action associated with a blocked signal is to ignore the signal
>   and if that signal is generated for the process, it is unspecified
>   whether the signal is discarded immediately upon generation or remains
>   pending.
>
> So the kernel has chosen to keep them pending for whatever reasons, which
> does not make any sense to me, but there is probably a historic reason.
>
> The commit which added the queuing of blocked and ignored signals is in the
> history tree with a pretty useless changelog.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
>
>  commit 98fc8ab9e74389e0c7001052597f61336dc62833
>  Author: Linus Torvalds <torvalds-ZsETY1VsSgIuF7duwytmx4H6Mc4MB0Vx@public.gmane.org>
>  Date:   Tue Feb 11 20:49:03 2003 -0800
>
>      Don't wake up processes unnecessarily for ignored signals
>
> It rewrites sig_ignored() and adds the following to it:
>
> +       /*
> +        * Blocked signals are never ignored, since the
> +        * signal handler may change by the time it is
> +        * unblocked.
> +        */
> +       if (sigismember(&t->blocked, sig))
> +               return 0;
>
> I have no idea how that is related to $subject of the commit and why this
> decision was made.
>
> Linus, any recollection?
>
> IMO, it's perfectly reasonable to discard ignored signals even when the
> signal is in the blocked mask. When its unblocked and SIG_IGN is replaced
> then the next signal will be delivered. But hell knows, how much user space
> depends on this weird behaviour by now.

I just looked through the history and the commit you point to looks like
it was either code motion or a regression fix.  The change to ignore
blocked signals actually came in between 1.2 and 2.0.  It looks like the
relevant diff was:

commit 886bad3fe1fe0c67208f15a02047e450e30f2b3a
Author: Linus Torvalds <torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>
Date:   Mon Apr 1 16:00:00 1996 -0800

    Linux version 1.3.82

diff --git a/kernel/exit.c b/kernel/exit.c
index 2b8e6d13ba1f..329d0b36bb08 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -23,25 +23,28 @@ extern void kerneld_exit(void);
 
 int getrusage(struct task_struct *, int, struct rusage *);
 
-static int generate(unsigned long sig, struct task_struct * p)
+static inline void generate(unsigned long sig, struct task_struct * p)
 {
        unsigned long mask = 1 << (sig-1);
        struct sigaction * sa = sig + p->sig->action - 1;
 
-       /* always generate signals for traced processes ??? */
-       if (!(p->flags & PF_PTRACED)) {
+       /*
+        * Optimize away the signal, if it's a signal that can
+        * be handled immediately (ie non-blocked and untraced)
+        * and that is ignored (either explicitly or by default)
+        */
+       if (!(mask & p->blocked) && !(p->flags & PF_PTRACED)) {
                /* don't bother with ignored signals (but SIGCHLD is special) */
                if (sa->sa_handler == SIG_IGN && sig != SIGCHLD)
-                       return 0;
+                       return;
                /* some signals are ignored by default.. (but SIGCONT already did its deed) */
                if ((sa->sa_handler == SIG_DFL) &&
                    (sig == SIGCONT || sig == SIGCHLD || sig == SIGWINCH || sig == SIGURG))
-                       return 0;
+                       return;
        }
        p->signal |= mask;
        if (p->state == TASK_INTERRUPTIBLE && (p->signal & ~p->blocked))
                wake_up_process(p);
-       return 1;
 }
 
 int send_sig(unsigned long sig,struct task_struct * p,int priv)



Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-man" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: signals: Bug or manpage inconsistency?

[Eric W. Biederman]

Thomas Gleixner <tglx@linutronix.de> writes:

> While trying to address the longstanding FIXME in the posix timer code
> related to ignored signals, I stumbled over the following issue:
>
> I blocked the signal of the timer, then installed the SIG_IGN handler,
> created and started the timer. After a short sleep the timer has fired
> several times, but it's still ignored AND blocked.
>
> Calling sigpending() after that has the timer signal set. See test case
> below.
>
> But 'man sigpending' says:
>
>    "If a signal is both blocked and has a disposition of "ignored", it is _not_
>     added to the mask of pending signals when generated."
>
> So something is clearly wrong here.
>
> The same happens with sigwait() while the signal is still blocked and
> ignored, it returns with that signal number and has the signal dequeued.
>
>
> The whole blocked vs. ignored handling is inconsistent both in the posix
> spec and in the kernel.
>
> The only thing vs. ignored signals what the spec mandates is:
>
>  SIG_IGN:
>
>  Delivery of the signal shall have no effect on the process.
>
>  ...
>
>  Setting a signal action to SIG_IGN for a signal that is pending shall
>  cause the pending signal to be discarded, whether or not it is blocked.
>
>  ...
>
>  Any queued values pending shall be discarded and the resources used to
>  queue them shall be released and made available to queue other signals.
>
> That's exactly what the kernel does in do_sigaction().
>
> And for everything else the spec is blurry:
>
>   If the action associated with a blocked signal is to ignore the signal
>   and if that signal is generated for the process, it is unspecified
>   whether the signal is discarded immediately upon generation or remains
>   pending.
>
> So the kernel has chosen to keep them pending for whatever reasons, which
> does not make any sense to me, but there is probably a historic reason.
>
> The commit which added the queuing of blocked and ignored signals is in the
> history tree with a pretty useless changelog.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
>
>  commit 98fc8ab9e74389e0c7001052597f61336dc62833
>  Author: Linus Torvalds <torvalds@penguin.transmeta.com>
>  Date:   Tue Feb 11 20:49:03 2003 -0800
>
>      Don't wake up processes unnecessarily for ignored signals
>
> It rewrites sig_ignored() and adds the following to it:
>
> +       /*
> +        * Blocked signals are never ignored, since the
> +        * signal handler may change by the time it is
> +        * unblocked.
> +        */
> +       if (sigismember(&t->blocked, sig))
> +               return 0;
>
> I have no idea how that is related to $subject of the commit and why this
> decision was made.
>
> Linus, any recollection?
>
> IMO, it's perfectly reasonable to discard ignored signals even when the
> signal is in the blocked mask. When its unblocked and SIG_IGN is replaced
> then the next signal will be delivered. But hell knows, how much user space
> depends on this weird behaviour by now.

I just looked through the history and the commit you point to looks like
it was either code motion or a regression fix.  The change to ignore
blocked signals actually came in between 1.2 and 2.0.  It looks like the
relevant diff was:

commit 886bad3fe1fe0c67208f15a02047e450e30f2b3a
Author: Linus Torvalds <torvalds@linux-foundation.org>
Date:   Mon Apr 1 16:00:00 1996 -0800

    Linux version 1.3.82

diff --git a/kernel/exit.c b/kernel/exit.c
index 2b8e6d13ba1f..329d0b36bb08 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -23,25 +23,28 @@ extern void kerneld_exit(void);
 
 int getrusage(struct task_struct *, int, struct rusage *);
 
-static int generate(unsigned long sig, struct task_struct * p)
+static inline void generate(unsigned long sig, struct task_struct * p)
 {
        unsigned long mask = 1 << (sig-1);
        struct sigaction * sa = sig + p->sig->action - 1;
 
-       /* always generate signals for traced processes ??? */
-       if (!(p->flags & PF_PTRACED)) {
+       /*
+        * Optimize away the signal, if it's a signal that can
+        * be handled immediately (ie non-blocked and untraced)
+        * and that is ignored (either explicitly or by default)
+        */
+       if (!(mask & p->blocked) && !(p->flags & PF_PTRACED)) {
                /* don't bother with ignored signals (but SIGCHLD is special) */
                if (sa->sa_handler == SIG_IGN && sig != SIGCHLD)
-                       return 0;
+                       return;
                /* some signals are ignored by default.. (but SIGCONT already did its deed) */
                if ((sa->sa_handler == SIG_DFL) &&
                    (sig == SIGCONT || sig == SIGCHLD || sig == SIGWINCH || sig == SIGURG))
-                       return 0;
+                       return;
        }
        p->signal |= mask;
        if (p->state == TASK_INTERRUPTIBLE && (p->signal & ~p->blocked))
                wake_up_process(p);
-       return 1;
 }
 
 int send_sig(unsigned long sig,struct task_struct * p,int priv)



Eric