Re: Updated auth patch for 2.4.12 - kernel/ptrace.c

offerat_private

* frm chrisat_private "10/16/2001 07:34:42 PM -0700" | sed '1,$s/^/* /'
*
* 
*> ===== kernel/ptrace.c 1.6 vs edited =====
*> @@ -27,20 +27,24 @@
*>  		goto bad;
*>  	if (!task->mm)
*>  		goto bad;
*> +	/* messier than it should be to ensure we have a code path identical
*> to  +	 * that pre-LSM */
*> +	retval = 0;
*>  	if(((current->uid != task->euid) ||
*>  	    (current->uid != task->suid) ||
*>  	    (current->uid != task->uid) ||
*>   	    (current->gid != task->egid) ||
*>   	    (current->gid != task->sgid) ||
*>   	    (current->gid != task->gid)) && !capable(CAP_SYS_PTRACE))
*> -		goto bad;
*> -	rmb();
*> -	if (!task->mm->dumpable && !capable(CAP_SYS_PTRACE))
*> -		goto bad;
*> +		retval = -EPERM;
*> +	if ( ! retval )
*> +		rmb();
*> +	if (!retval && !task->mm->dumpable && !capable(CAP_SYS_PTRACE))
*> +		retval = -EPERM;
* 
* now this is looking very sketchy.  to be safe, i'd not make the rmb()
* conditional, as it is required to order reads for race condition
* elimination.  (yes, ptrace is _always_ a headache!)  perhaps nesting
* conditional would be better here instead of multiple use of !retval.

From Greg's comments from a much earlier patch, I got the impression that
for ptrace it was more important to follow the pre-LSM code order as it is
very race sensitive.

The question is "is it okay to call rmb() even if you fail the first
permission check ?"

* and this is another example of the blurred lines between functional and
* access control logic... if (task->ptrace & PT_PTRACED).  do you really
* want to hand over a task that is already being traced to our hook?  i
* don't think it is appropriate.

I agree. Here's what I'd propose.

===== ptrace.c 1.6 vs edited =====

--- 1.6/kernel/ptrace.c Sun Sep 23 17:21:24 2001
+++ edited/ptrace.c     Fri Oct 19 07:41:01 2001
@@ -27,20 +27,24 @@
                goto bad;
        if (!task->mm)
                goto bad;
+       /* messier than it should be to ensure we have a code path
identical to 
+        * that pre-LSM */
+       retval = 0;
        if(((current->uid != task->euid) ||
            (current->uid != task->suid) ||
            (current->uid != task->uid) ||
            (current->gid != task->egid) ||
            (current->gid != task->sgid) ||
            (current->gid != task->gid)) && !capable(CAP_SYS_PTRACE))
-               goto bad;
-       rmb();
-       if (!task->mm->dumpable && !capable(CAP_SYS_PTRACE))
-               goto bad;
+               retval = -EPERM;
+       if ( ! retval )
+               rmb();
+       if (!retval && !task->mm->dumpable && !capable(CAP_SYS_PTRACE))
+               retval = -EPERM;
        /* the same process cannot be attached many times */
        if (task->ptrace & PT_PTRACED)
                goto bad;
-       retval = security_ops->ptrace(current, task);
+       retval = security_ops->ptrace(retval, current, task);
        if (retval)
                goto bad;


richard.

-- 
-----------------------------------------------------------------------
Richard Offer                     Technical Lead, Trust Technology, SGI
"Specialization is for insects"
___________________________________________On sabatical Nov 8 -> Nov 30


_______________________________________________
linux-security-module mailing list
linux-security-moduleat_private
http://mail.wirex.com/mailman/listinfo/linux-security-module
