[PATCH] UBUNTU: SAUCE: ptrace: restrict ptrace scope to children

[Chase Douglas]

On Wed, 2010-05-26 at 13:50 -0700, Kees Cook wrote:
> On Wed, May 26, 2010 at 04:15:25PM -0400, Chase Douglas wrote:
> > > +	if (ptrace_scope && !capable(CAP_SYS_PTRACE)) {
> > > +		/* require ptrace target be a child of ptracer */
> > > +		struct task_struct *tmp = task;
> > > +		struct task_struct *curtemp = current;
> > 
> > Why create a new variable just to store current? I think it would be
> > more readable to just use current where you use curtemp. I don't think
> > current should change from under you when you're here.
> > 
> > > +		int rc = 0;
> > > +
> > > +		read_lock(&tasklist_lock);
> > > +		while (tmp->pid > 0) {
> > > +			if (tmp == curtemp)
> > > +				break;
> > > +			tmp = tmp->parent;
> > > +		}
> > > +		if (tmp->pid == 0)
> > > +			rc = -EPERM;
> > > +		read_unlock(&tasklist_lock);
> 
> That's a fair point -- I guess we're under lock the entire time.  I'd be
> fine to change it if other people agree it's safe looking.

I don't think the lock here would necessarily affect current; its safe
through a separate mechanism. This code path I assume is called through
a syscall into the kernel. While this is executing, current will always
point to the task that called the syscall. If a context switch happens,
current gets changed to a different process at the same time the
execution pointer is switched to the other process somewhere else in the
kernel or userspace, but when we context switch back to this process to
continue, current once again points to the correct task.

That said, the above is probably an exercise in semantics on a
non-completely-preemptible kernel where we can't context switch while a
lock is held. Our kernels are also only voluntarily preemptible, so
there's no chance the context switch could occur anywhere in this code
anyways.

-- Chase