Re: [PATCH UPDATED 8/9] ptrace: move JOBCTL_TRAPPING wait to wait(2) and ptrace_check_attach()

[Tejun Heo <tj@kernel.org>]

Hello,

On Mon, May 16, 2011 at 02:11:42PM +0200, Oleg Nesterov wrote:
> Hmm. I didn't even know we have restart_syscall()... It is a bit fragile,
> it assumes recalc_sigpending() is not possible during return from syscall.
> In particular this means recalc_sigpending() must not be called in irq.
> OK, this seems to be true.

Yes, it is quite fragile.  I think we can make it more reliable
instead of each path which might clear TIF_SIGPENDING to be careful
about not hitting it, but that's a separate issue and we need the
syscall restart mechanism anyway for other purposes.

> Anyway, restart_syscall() is not right for do_wait(), especially with the
> next patch. If the caller was woken by the real signal which has a handler,
> we should not restart without SA_RESTART.

I don't think it really matters and might even be incorrect if we do
that.  e.g. we would introduce -EINTR failure to WNOHANG waits.  These
waits and restarts are transient and should be transparent.  Maybe if
the ptracer is ptraced, syscall entry trap can give away that the
syscall was retried when SA_RESTART wasn't set but I'd be happy to
ignore that.

The only case that I think of where this could be visible is sleeping
wait(2), checking the state again after being woken up and gets signal
while waiting for TRAPPING.  In this case, yeap, we should fail with
-EINTR.

Hmmm... so, I suppose, we should either return -ERESTARTNOINTR or
-ERESTARTSYS depending on WNOHANG.  How does that sound?

> It is very hard to review this series. Without the further changes, it is
> not clear why do we need these preparations. IIUC, ptrace_wait_trapping()
> is only needed because we are going to re-trap. Otherwise we could always
> wait in ptrace_attach() afaics.

I think the patches in this series should be able to stand on
themselves.  It's relaxing TRAPPING constraints and making it
generally safer.  With these changes, SEIZE/INTERRUPT/notification
implementation becomes quite straight-forward.

> I am still worried we are loosing the tight control over JOBCTL_TRAPPING.
> 6/9 contributes to this too.

Setting of TRAPPING is scary.  Clearing isn't.  The worst that can
happen is unexpected failure of WNOHANG wait or ptrace request - even
that isn't gonna happen if the application is sensible enough to use
sleeping wait(2) after attaching.

Thanks.

-- 
tejun