Re: [PATCH v2] connector: add an event for monitoring process tracers

On 07/18, Vladimir Zapolskiy wrote:

On Mon, Jul 18, 2011 at 8:54 PM, Oleg Nesterov [off-list ref] wrote:

quoted

"which_id" doesn't match "ptrace_id" used elsewhere. And PTRACE_ATTACH
instead of simple boolean looks as if you are going to add more ptrace
events, but I guess this won't happen.

^
I'd like to preserve that variant, in my opinion its just a bit more
undisguised version rather than bare true/false.

OK. Although this "else return" in proc_ptrace_connector() looks like
the "hide the potentional error" to me.

quoted

quoted

-     if (!retval)
+     if (!retval) {
              wait_on_bit(&task->jobctl, JOBCTL_TRAPPING_BIT,
                          ptrace_trapping_sleep_fn, TASK_UNINTERRUPTIBLE);
+             proc_ptrace_connector(task, PTRACE_ATTACH);
+     }

OK, but it is a bit strange we are waiting for STOPPED/TRACED transition
before we report PROC_EVENT_PTRACE. Perhaps it makes more sense to
call proc_ptrace_connector() first, this also decreases the probability
PTRACE_ATTACH will be reported after PROC_EVENT_EXIT.

Yes, there is a difference. But as far as there is no guaranteed
serialization in proc connector event reports, user-space process
trackers should be designed to operate correctly having in mind
possible event reordering.

Yes, but I didn't really mean the correctness. I meant, this looks
confusing, as if this wait_on_bit() has something to do with attach.
Likewise I do not understand why proc_exec_connector() is called
after ptrace_event() which can sleep unpredictably long.



Nevermind, applied.

Oleg.