Re: PATCH? tracehook_report_clone: fix false positives
From: Roland McGrath
Date: Sun May 31 2009 - 20:23:33 EST
> 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.
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/