Message-ID: <87y3tcgpil.fsf@xmission.com>
Date:   Thu, 01 Jun 2017 02:01:38 -0500
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Oleg Nesterov <oleg@...hat.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        linux-man@...r.kernel.org, libc-alpha@...rceware.org
Subject: Re: signals: Bug or manpage inconsistency?

Thomas Gleixner <tglx@...utronix.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@...guin.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@...ux-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

Powered by blists - more mailing lists