linux-kernel - Re: [PATCH] [RFC] fix missed SIGCONT cases

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] [day] [month] [year] [list]

Message-Id: <20080306105043.8FD982700FD@magilla.localdomain>
Date:	Thu,  6 Mar 2008 02:50:43 -0800 (PST)
From:	Roland McGrath <roland@...hat.com>
To:	Oleg Nesterov <oleg@...sign.ru>
Cc:	Jiri Kosina <jkosina@...e.cz>,
	Davide Libenzi <davidel@...ilserver.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] [RFC] fix missed SIGCONT cases

> Actually, it is easy to re-use signal_struct->flags from the very beginning,
> no need to introduce the new flag temporary, see the attached patches. I also
> have a couple of simple cleanups on top of them (use cached ->signal value).

Looks like a good start.

> Perhaps, we can add the new helper which sets SIGNAL_GROUP_EXIT but preserves
> SIGNAL_CLD_MASK, but I don't think this is really needed.

Actually, I think it is.  This made think of another pedantic concern.
(Not that I'm sure we get this right now anyway.)

Say a child was previously stopped, parent had seen CLD_STOPPED and done
WSTOPPED wait.  Now we send SIGCONT followed shortly by SIGTERM (unhandled
fatal signal).  In the interval before the SIGTERM has yet been dequeued
(nor SIGKILLs sent to other threads by __group_complete_signal), the parent
does waitpid(pid, WSTOPPED|WCONTINUED|WEXITED|WNOHANG).  It should see one
of: WIFSTOPPED (still); WIFSIGNALED (dead); WIFCONTINUED.  Instead, waitpid
will return 0 (it's running, not stopped, not continued).  This says
SIGNAL_STOP_CONTINUED ought to function as normal (i.e. WCONTINUED polling)
until we're actually ready for wait to report the process as dead.

We already break this pedantic nit because flags = SIGNAL_GROUP_EXIT clears
SIGNAL_STOP_CONTINUED.  But if we clean up all flags usage, I think we
should make it preserve SIGNAL_STOP_CONTINUED.

I'll admit it's thin since a SIGKILL not yet dequeued or in the middle of
delivery brings the task to running so WNOHANG wait won't report it as
still stopped, won't report it for WCONTINUED at all, and won't report it
as dead yet.  But if we were to make wait key on SIGNAL_STOP_STOPPED, as is
probably necessary to do WSTOPPED waits for processes with a zombie
group_leader, then this too could be made consistent.  (Leave the
SIGNAL_STOP_STOPPED bit set throughout, so WSTOPPED|WNOHANG waits continues
to report it as stopped until it actually dies.)

> @@ -1761,6 +1757,19 @@ int get_signal_to_deliver(siginfo_t *inf
>  
>  relock:
>  	spin_lock_irq(&current->sighand->siglock);
> +
> +	if (unlikely(current->signal->flags & SIGNAL_CLD_MASK)) {
> +		int why = (current->signal->flags & SIGNAL_STOP_CONTINUED)

This should have a comment before the block.  Explain that it is finishing
the report set up by handle_stop_signal while we were in TASK_STOPPED in a
previous iteration here inside get_signal_to_deliver.

Thanks,
Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/