linux-kernel - Re: signals: Bug or manpage inconsistency?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 30 May 2017 20:10:36 -0500
From:   ebiederm@...ssion.com (Eric W. Biederman)
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Oleg Nesterov <oleg@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        linux-man@...r.kernel.org, libc-alpha <libc-alpha@...rceware.org>
Subject: Re: signals: Bug or manpage inconsistency?

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

> Thomas Gleixner <tglx@...utronix.de> writes:
>
>> On Tue, 30 May 2017, Linus Torvalds wrote:
>>> On Tue, May 30, 2017 at 10:04 AM, Oleg Nesterov <oleg@...hat.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