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 for Android: free password hash cracker in your pocket

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

Date:   Tue, 30 May 2017 19:48:10 -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?

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.

Eric