lkml.org 
[lkml]   [2010]   [Jun]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2/2] Yama: add PTRACE exception tracking
    Quoting Kees Cook (kees.cook@canonical.com):
    > Some application suites have external crash handlers that depend on
    > being able to use PTRACE to generate crash reports (KDE, Chromium, etc).
    > Since the inferior process generally knows the PID of the debugger,
    > it can use PR_SET_PTRACER to allow a specific PID and its descendants
    > to perform the PTRACE instead of only a direct ancestor.
    >
    > Signed-off-by: Kees Cook <kees.cook@canonical.com>
    > ---

    Hi Kees - very nice, overall. One little note though:

    > @@ -32,27 +204,20 @@ static int yama_ptrace_access_check(struct task_struct *child,
    > {
    > int rc;
    >
    > + /* If standard caps disallows it, so does Yama. We should
    > + * should only tighten restrictions further.
    > + */
    > rc = cap_ptrace_access_check(child, mode);

    This means that if capable(CAP_SYS_PTRACE) we'll always shortcut
    here, so

    > - if (rc != 0)
    > + if (rc)
    > return rc;
    >
    > /* require ptrace target be a child of ptracer on attach */
    > - if (mode == PTRACE_MODE_ATTACH && ptrace_scope &&
    > - !capable(CAP_SYS_PTRACE)) {
    > - struct task_struct *walker = child;
    > -
    > - rcu_read_lock();
    > - read_lock(&tasklist_lock);
    > - while (walker->pid > 0) {
    > - if (walker == current)
    > - break;
    > - walker = walker->real_parent;
    > - }
    > - if (walker->pid == 0)
    > - rc = -EPERM;
    > - read_unlock(&tasklist_lock);
    > - rcu_read_unlock();
    > - }
    > + if (mode == PTRACE_MODE_ATTACH &&
    > + ptrace_scope &&
    > + !capable(CAP_SYS_PTRACE) &&

    You don't need the CAP_SYS_PTRACE check here AFAICS.

    > + !task_is_descendant(current, child) &&
    > + !ptracer_exception_found(current, child))
    > + rc = -EPERM;
    >
    > if (rc) {
    > char name[sizeof(current->comm)];
    > @@ -170,6 +335,8 @@ static struct security_operations yama_ops = {
    > .ptrace_access_check = yama_ptrace_access_check,
    > .inode_follow_link = yama_inode_follow_link,
    > .path_link = yama_path_link,
    > + .task_prctl = yama_task_prctl,
    > + .task_free = yama_task_free,
    > };
    >
    > #ifdef CONFIG_SYSCTL
    > @@ -221,6 +388,8 @@ static __init int yama_init(void)
    >
    > printk(KERN_INFO "Yama: becoming mindful.\n");
    >
    > + spin_lock_init(&ptracer_relations_lock);
    > +
    > if (register_security(&yama_ops))
    > panic("Yama: kernel registration failed.\n");
    >
    > --
    > 1.7.1
    >
    >
    > --
    > Kees Cook
    > Ubuntu Security Team
    > --
    > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
    > the body of a message to majordomo@vger.kernel.org
    > More majordomo info at http://vger.kernel.org/majordomo-info.html


    \
     
     \ /
      Last update: 2010-06-30 05:57    [W:0.115 / U:60.224 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site