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

From: richard offer (offerat_private)
Date: Fri Oct 19 2001 - 07:46:23 PDT

  • Next message: richard offer: "Re: Updated auth patch for 2.4.12 - kernel/module.c"

    * 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
    



    This archive was generated by hypermail 2b30 : Fri Oct 19 2001 - 07:47:43 PDT