On 10/27, Srikar Dronamraju wrote:
>
> * Oleg Nesterov <o...@redhat.com> [2009-10-26 04:28:46]:
>
> > @@ -169,9 +164,9 @@ static inline void ptrace_init_task(stru
> >     INIT_LIST_HEAD(&child->ptraced);
> >     child->parent = child->real_parent;
> >     child->ptrace = 0;
> > -   if (unlikely(ptrace)) {
> > +   if (unlikely(ptrace) && (current->ptrace & PT_PTRACED)) {
>
> Any reason for directly accessing current->ptrace instead of accessing
> thro task_ptrace(current) ?

Yes, we can use task_ptrace(), but we don't use this helper in ptrace.h.

In fact it should die. Initially it was introduced for tracehooks, then
it was overloaded (by me).

Currently it is used to read task->ptrace flags, and to check the task
is traced, iow to check task_ptrace() != 0. Not good.

> Also would something like this suffice.
>  +    if (unlikely(ptrace) && (task_ptrace(current)) {

No. The whole point (apart from cleanup) is to add PT_PTRACED check.

Currently, task_ptrace() != 0 means that PT_PTRACED must be set, so
we could use task_ptrace() != 0.

But with utrace-ptrace, task_ptrace() != 0 means the task is ptraced,
while PT_PTRACED is not set. IOW, PT_PTRACED is only true when
!CONFIG_UTRACE.

With utrace-ptrace, ptrace_init_task() must not auto-attach the new
child, that is why we check PT_PTRACED.

Please note that, once the old implementation is removed (when
CONFIG_UTRACE goes away) we can just kill allmost all tracehooks
from do_fork() path. They are only needed for the old implementation.

> i.e the first patch in this series removes checks to see if ptrace field
> is set with PT_TRACED.
> "task_ptrace() != 0 if and only if PT_PTRACED bit is set, kill
> some PT_PTRACED checks in tracehook.h."

Yes, but see above.

And the first patch ensures that, say, tracehook_tracer_task() works
correctly regardless of CONFIG_UTRACE.

Again, this all is only needed because we have two implementations
until CONFIG_UTRACE goes away.

Oleg.

Reply via email to