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

[Vladimir Zapolskiy <vzapolskiy@gmail.com>]

On Mon, Jul 18, 2011 at 8:54 PM, Oleg Nesterov <oleg@redhat.com> wrote:
> On 07/15, Vladimir Zapolskiy wrote:
>>
>> Such an event allows to create a simple automated userspace mechanism
>> to be aware about processes connecting to others, therefore predefined
>> process policies can be applied to them if needed.
>
> I'd wish I could understand this ;) IOW, I still do not understand why
> this is useful, but this doesn't matter. Since Evgeniy acked this patch,
> I'll apply it to ptrace tree.
>

Originally the idea comes from a desire to classify development tools
like strace or gdb in the same way as a tracee process, for instance
put them to a tracee's current cgroup partition, which might be done
automatically in user-space by some process "track and manage" daemon.

>
> Can't resist, a couple of very minor/cosmetics nits. Just because I am
> blighter ;)
>
>> +void proc_ptrace_connector(struct task_struct *task, int which_id);
>
> "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.

>> -     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.

>
> But once again, this is very minor and cosmetic. I am going to apply
> the patch as is unless you send v3 quickly.
>

Thank you a lot, especially for comments, but I think this v2 is good
enough to be applied :) And if there are more users of that proc
connector extension in future, it might be needed to cut some sharp
corners later, and may be even add implicit detach reporting :)

--
With best wishes,
Vladimir