Re: [PATCH 4/6] Add dynamic context transition support to SELinux

[Stephen Smalley]

On Thu, 2004-12-02 at 13:34, Chris Wright wrote:
> > +		/* Only allow single threaded processes to change context */
> > +		if (atomic_read(&p->mm->mm_users) != 1) {
> > +			struct task_struct *g, *t;
> > +			struct mm_struct *mm = p->mm;
> > +			read_lock(&tasklist_lock);
> > +			do_each_thread(g, t)
> > +				if (t->mm == mm && t != p) {
> > +					read_unlock(&tasklist_lock);
> > +					return -EPERM;
> > +				}
> > +			while_each_thread(g, t);
> > +			read_unlock(&tasklist_lock);
> 
> That's heavy handed.  Can't you track this at clone time?  Or at least
> do this after the AVC check, so it's not always locking task list.

Hmmm...if the latter is a concern, it seems like there are other cases
that likewise need fixing, e.g. sys_getpriority or sys_setpriority. 
Earlier version of the patch did a simple check of thread_group_empty()
but that didn't catch CLONE_VM w/o CLONE_THREAD, and simple check of
mm_users can produce false positives, e.g. another process reading your
/proc/pid/<xxx> file and holding a reference to the mm temporarily.

We could set a flag in the task security structure upon
selinux_task_create upon CLONE_VM, I suppose, and inherit it in
selinux_task_alloc_security.  That would prohibit any use after you've
ever created a thread, even if none of the other threads are still
around, which is stronger than we need, but probably not an obstacle. 
Is that what you had in mind?
 
-- 
Stephen Smalley <sds@xxxxxxxxxxxxxx>
National Security Agency

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/