Re: [PATCH 03/10] ptrace: implement PTRACE_SEIZE

From: Tejun Heo <tj@kernel.org>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Pedro Alves <pedro@codesourcery.com>,
	Denys Vlasenko <vda.linux@googlemail.com>,
	jan.kratochvil@redhat.com, linux-kernel@vger.kernel.org,
	torvalds@linux-foundation.org, akpm@linux-foundation.org,
	indan@nul.nu, bdonlan@gmail.com
Subject: Re: [PATCH 03/10] ptrace: implement PTRACE_SEIZE
Date: Thu, 26 May 2011 11:10:41 +0200	[thread overview]
Message-ID: <20110526091041.GC9715@htj.dyndns.org> (raw)
In-Reply-To: <20110525181856.GA16575@redhat.com>

Hey,

On Wed, May 25, 2011 at 08:18:56PM +0200, Oleg Nesterov wrote:
> > Now that thinking more about it, TRAP_STOP (INTERRUPT trap) would
> > probably be better.  I'll think more about it.  For fork, it doesn't
> > really matter but deliverying SIGSTOP on CLONE isn't too good.  From
> > user's POV, TRAP_STOP should work too, right?
> 
> I am a bit confused, sorry if I missed/misunderstood the context...
> 
> I thought, we already discussed this. Obviously, PT_SEIZED should be
> inherited by the child during the auto-attach (and it is indeed copied
> by ptrace_init_task). Why should we send SIGSTOP to the child then?
> Even in the fork case, this leads to the same problems as the current
> PTRACE_ATTACH has with the bogus SIGSTOP it sends. I think TRAP_STOP
> is the only sane option, no?

I forgot about that and was thinking about events reported by
SIGTRAPs.  :)

Yes, in this case, TRAP_STOP is the only sane solution.

> Btw. Speaking of SEIZE->execvd->INTERRUPT which makes the tracee see
> a SIGTRAP. Stupid question. Perhaps PTRACE_SEIZE should set
> PT_TRACESYSGOOD | PT_TRACE_EXEC along with PT_SEIZED automatically?
> PT_SEIZED implies the new behaviour anyway.

Yeap, it makes sense to set them by default.  I was thinking about
defining a new PTRACE_EVENT_* rather than using 0x80 tho.  That way we
should be able to distinguish between enter and exit too.

This reminded me of another issue.  Currently we have two different
methods of reporting debug events.  One is direct ptrace event trap,
which ends up in ptrace_stop() from the event site and thus is
synchronous.  The other is sending SIGTRAP, literally.  Besides the
usual problems related with implicitly sending signals, this also
makes it difficult to determine which event is happening exactly and
makes the whole signal delivery/injection thing much worse.

I suspect that all ptrace traps probably started as actual SIGTRAPs
and so all the events were reported via signal delivery path and so
the signal "injection" worked; however, now we have synchronous traps
and some SIGTRAPs, which are confusing and unreliable.  I don't see
any reason why the events which are currently reported via SIGTRAPs
can't be reported via ptrace traps with unique PTRACE_EVENT_*.  I
don't think we can take synchronous traps directly but we can schedule
them, record relevant information in preallocated area and just set
TIF_SIGPENDING and let the signal delivery path report the trap.  This
will make all the events reliable and easily distinguishible and we
can fix the SIGTRAP signal problem.  What do you think?

Thanks.

-- 
tejun