Messages in this thread |  | | From | Roland McGrath <> | | Subject | Re: PATCH? tracehook_report_clone: fix false positives | | Date | Sun, 31 May 2009 17:22:26 -0700 (PDT) |
| |
> Firtsly, I don't understand CLONE_PTRACE check. Suppose that untraced > task does clone(CLONE_PTRACE). In that case we create the untraced > child (this is correct) but still we send SIGSTOP. > > I do not really know if this bug or not, but this doesn't look right. > At least this should be commented, imho. And, looking at 2.6.26, I think > the behaviour was different before tracehooks. > > So, I assume this is bug for now.
You're right. CLONE_PTRACE when not traced will misbehave (not that anyone ever uses it). The old code just checked child->ptrace, and that is fine to do again now. I probably changed that thinking it had a race--which it does--with asynchronous PTRACE_ATTACH after an untraced fork. But that is a harmless race as you explained.
ACK on the 2.6.30 patch attached.
> So, I am going to send the patch below. But this leads to another question: > should not we move these sigaddset() + set_tsk_thread_flag() into > ptrace_init_task() ?
It might make sense to consolidate them. But note that ptrace_attach() uses send_sig_info(). With SEND_SIG_FORCED, this does almost nothing more than sigaddset() (i.e. no queue entry). But it does do prepare_signal(), which will clear any pending SIGCONTs. It's possible that something in userland manages to rely on that behavior for the asynchronous attach case (unrelated to startup-time races). It wouldn't hurt for the creation-time case to use send_sig_info() too, though it would go through a bunch more code to do nothing effectual but sigaddset() in the end.
Thanks, Roland
|  |