[PATCH 09] move ->ptrace == 0 checks to ptrace_attach_task()

[Oleg Nesterov]

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.