linux-kernel - Re: [PATCH 3/3] ptrace_untrace: fix the SIGNAL_STOP

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]

Message-ID: <20090209030953.GA30787@redhat.com>
Date:	Mon, 9 Feb 2009 04:09:53 +0100
From:	Oleg Nesterov <oleg@...hat.com>
To:	Roland McGrath <roland@...hat.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Jerome Marchand <jmarchan@...hat.com>,
	Denys Vlasenko <dvlasenk@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/3] ptrace_untrace: fix the SIGNAL_STOP_STOPPED check

On 02/08, Roland McGrath wrote:
>
> Yes, I believe this is correct.  It matches the flip side of the
> bookkeeping where we adjust group_stop_count when going into TASK_TRACED
> (ptrace_stop).  I think it warrants a comment with your change, saying that
> treating group_stop_count as "we should be already stopped" is consistent
> with decrementing an active group_stop_count when we enter TASK_TRACED.

Yes, I tried to make the comment, but failed.

Because we have another case. The group stop is in progress, and some
thread T does do_signal_stop()->finish_stop(). It is TASK_STOPPED.
Now we do PTRACE_ATTACH + PTRACE_DETACH. And the second sys_ptrace()
changes T->state to TASK_TRACED.

And. It it also possible that we ptrace the single sub-thread, then
the group stop starts. The first thread which enters do_signal_stop()
will not count the TASK_TRACED child, so it should stay stopped.

> > 	- if the process/thread was traced, SIGNAL_STOP_STOPPED
> > 	  does not necessary means this thread group is stopped.
> >
> > 	- ptrace breaks the bookkeeping of ->group_stop_count.
>
> SIGNAL_STOP_STOPPED is only set when all live threads in the group are in
> either TASK_TRACED or TASK_STOPPED.  PTRACE_DETACH respects this and this
> it stopped.  However, PTRACE_CONT et al (ptrace_resume) do not respect it
> and can resume an individual thread regardless of SIGNAL_STOP_STOPPED.
> That's what you mean here, right?

Yes. (and of course, we don't even need threads to hit this problem).

> > (the comment above ptrace_untrace() doesn't look exactly right too).
>
> How so?

Perhaps this is just my misunderstanding, but

	/*
	 * Turn a tracing stop into a normal stop now, since with no tracer there
	 * would be no way to wake it up with SIGCONT or SIGKILL.

This looks as if we always do /TRACED/STOPPED/ unconditionally.

	                                                           If there was a
	 * signal sent that would resume the child, but didn't because it was in
	 * TASK_TRACED, resume it now.

No, we resume it not because it may have signals, and we don't even check
it has pending signals.

	 * Requires that irqs be disabled.
	 */

this is correct ;)

Oleg.

--
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/