Hi Srikar,
On 08/21, Srikar Dronamraju wrote:
> >
> > --- PU/kernel/ptrace.c~09_MV_PTRACE_CK 2009-08-19 16:49:25.000000000
> > +0200
> > +++ PU/kernel/ptrace.c 2009-08-20 20:04:59.000000000 +0200
> > @@ -471,35 +471,47 @@ static int ptrace_attach_task(struct tas
> > {
> > struct utrace_engine *engine;
> > unsigned long events;
> > + int err = -EPERM;
> >
> > engine = utrace_attach_task(tracee, UTRACE_ATTACH_CREATE |
> > UTRACE_ATTACH_EXCLUSIVE |
> > UTRACE_ATTACH_MATCH_OPS,
> > &ptrace_utrace_ops, NULL);
> > if (unlikely(IS_ERR(engine))) {
> > - int err = PTR_ERR(engine);
> > - if (err != -ESRCH && err != -ERESTARTNOINTR)
> > - err = -EPERM ;
> > + if (PTR_ERR(engine) == -ESRCH ||
> > + PTR_ERR(engine) == -ERESTARTNOINTR)
> > + err = PTR_ERR(engine);
> > return err;
> > }
> > +
> > /*
> > - * We need QUIESCE for resume handling, CLONE to check
> > - * for CLONE_PTRACE, other events are always reported.
> > - */
> > - events = UTRACE_EVENT(QUIESCE) | UTRACE_EVENT(CLONE) |
> > - UTRACE_EVENT(EXEC) | UTRACE_EVENT_SIGNAL_ALL;
> > - /*
> > - * It can fail only if the tracee is dead, the caller
> > - * must notice this before setting PT_PTRACED.
> > + * Check ->ptrace to make sure we don't race with the previous
> > + * tracer which didn't finish detach. If it is clear, nobody
> > + * can change it except us due to UTRACE_ATTACH_EXCLUSIVE.
> > */
> > - utrace_set_events(tracee, engine, events);
>
>
> > + if (unlikely(tracee->ptrace)) {
>
> Any reason why we cant move this check before we do utrace_attach_task?
Please note the comment, this check relies on UTRACE_ATTACH_EXCLUSIVE
above. Once we see ->ptrace = 0 after utrace_attach_task(), nobody
can change ->ptrace.
Now suppose ptrace_attach_task() does
if (tracee->ptrace)
return;
/* --- WINDOW --- */
engine = utrace_attach_task();
if (IS_ERR(engine))
return;
...complete attach...
In that WINDOW, another tracer can attach to this tracee, then start
the detach and remove the engine, but do not call __ptrace_unlink() yet.
Oleg.
