Yama: turn process ancestry check into function
in [Kernel]
|
Prev: x86, paravirt: Add a global synchronization point for pvclock
Next: CFQ: Don't store left slice when slice used up or for a idle workload
From: Tetsuo Handa on 13 Jul 2010 20:20 Kees Cook wrote: > +static int task_is_descendant(struct task_struct *parent, > + struct task_struct *child) > +{ > + int rc = 0; > + struct task_struct *walker = child; > + > + if (!parent || !child) > + return 0; parent (== current) is !NULL and child (in original code) is !NULL. You can remove this check unless you are planning to call this function from other places. > + if (mode == PTRACE_MODE_ATTACH && > + ptrace_scope && > + !task_is_descendant(current, child) && > + !capable(CAP_SYS_PTRACE)) > + rc = -EPERM; I don't know how heavy capable(CAP_SYS_PTRACE) is. But checking !capable(CAP_SYS_PTRACE) before !task_is_descendant(current, child) might be lighter. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Kees Cook on 13 Jul 2010 20:40 On Wed, Jul 14, 2010 at 09:19:09AM +0900, Tetsuo Handa wrote: > Kees Cook wrote: > > +static int task_is_descendant(struct task_struct *parent, > > + struct task_struct *child) > > +{ > > + int rc = 0; > > + struct task_struct *walker = child; > > + > > + if (!parent || !child) > > + return 0; > > parent (== current) is !NULL and > child (in original code) is !NULL. > You can remove this check unless you are planning to call > this function from other places. I'd like the flexibility to call it with NULLs. But yes, at present, it never will be NULL. > > + if (mode == PTRACE_MODE_ATTACH && > > + ptrace_scope && > > + !task_is_descendant(current, child) && > > + !capable(CAP_SYS_PTRACE)) > > + rc = -EPERM; > > I don't know how heavy capable(CAP_SYS_PTRACE) is. > But checking !capable(CAP_SYS_PTRACE) before > !task_is_descendant(current, child) might be lighter. That's the order I had before, but in looking at some of the other code, it seemed like moving it to the end made more logical sense. Since checking PTRACE attach isn't a common or time-sensitive operation, I figured trying to tune it wasn't critical. -Kees -- Kees Cook Ubuntu Security Team -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Serge E. Hallyn on 13 Jul 2010 22:20 Quoting Kees Cook (kees.cook(a)canonical.com): > On Wed, Jul 14, 2010 at 09:19:09AM +0900, Tetsuo Handa wrote: > > > + if (mode == PTRACE_MODE_ATTACH && > > > + ptrace_scope && > > > + !task_is_descendant(current, child) && > > > + !capable(CAP_SYS_PTRACE)) > > > + rc = -EPERM; > > > > I don't know how heavy capable(CAP_SYS_PTRACE) is. > > But checking !capable(CAP_SYS_PTRACE) before > > !task_is_descendant(current, child) might be lighter. > > That's the order I had before, but in looking at some of the other code, it > seemed like moving it to the end made more logical sense. Since checking > PTRACE attach isn't a common or time-sensitive operation, I figured trying > to tune it wasn't critical. Yes the reason to keep it like this is that capable(CAP_SYS_PTRACE) will set PF_SUPERPRIV if it passes. You don't want to do that unless the capability was actually required. -serge -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: James Morris on 20 Jul 2010 19:10
On Tue, 13 Jul 2010, Kees Cook wrote: > This cleans up the ancestry check by separating it out into its own > function, which makes the code a bit more readable. Additionally, it > now checks for and uses the thread group leader since otherwise we may > end up missing potential matches on attaches to threads. > > Signed-off-by: Kees Cook <kees.cook(a)canonical.com> Applied to git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/security-testing-2.6#next -- James Morris <jmorris(a)namei.org> -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |