Re: [PATCH 02/11] ptrace: implement PTRACE_SEIZE

[Tejun Heo <tj@kernel.org>]

Hey, Oleg.

On Tue, May 10, 2011 at 03:20:01PM +0200, Oleg Nesterov wrote:
> > Hmmm... if killed, the tracee would go through __ptrace_unlink() which
> > always clears JOBCTL_TRAP_MASK which includes JOBCTL_TRAP_SEIZE.  What
> > am I missing?
> 
> No. If killed, the tracee becomes a zombie (but see below). __ptrace_unlink()
> won't be called until the tracee does wait().

Ah, right.

> Yes, and thus the tracee will loop forever until the tracer detaches, so
> it can't even become a zombie. And the tracer can't detach via ptrace().

I think this basically is the same problem as the using TRAPPING on
PTRACE_DETACH one.  Do you recall the original version where TRAPPING
was cleared on kill via clear_pending instead of at the end of
do_signal_stop()?  I think that's what we should do.  We should be
clearing these conditions when the actions which should override these
conditions happen instead of trying to put them in the tracee
expecting control to be at certain places and to me
task_clear_jobctl_stop_pending() seems to be the best place to do
that.  We might need to rename it and maybe add a parameter tho.

How does it sound to you?  It would solve the TRAPPING issue too.

> > > I never understood why ptrace_stop()->sigkill_pending() logic, I think
> > > we should check fatal_signal_pending() unconditionally.
> >
> > Probably the best way to do it is adding fatal_signal_pending() into
> > may_ptrace_stop() so that the failure path shares most of the code
> > path instead of doing quick dirty exit?  It's just nasty to have early
> > exit above there and walking through the normal code path wouldn't
> > hurt SIGKILL either.
> 
> Yes, probably. But, apart from the user-visible change which needs another
> discussion, there are more problems. fatal_signal_pending() can "wrongly"
> return false if the caller is tracehook_report_exit(). OTOH, it can correctly
> return true (exec), but we probably want to stop. Let's discuss this later,
> I think this has nothing to do with your patches. The main problem is not
> fatal_signal_pending(), we need the better definition of explicit SIGKILL
> behaviour. Not for ptrace only, there are other issues.

Alright, let's put it aside for now.

Thank you.

-- 
tejun