Messages in this thread | | | Date | Mon, 16 May 2011 15:36:44 +0200 | From | Tejun Heo <> | Subject | Re: [PATCH UPDATED 8/9] ptrace: move JOBCTL_TRAPPING wait to wait(2) and ptrace_check_attach() |
| |
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
| |