Messages in this thread | | | Date | Tue, 29 Jun 2010 22:56:09 -0500 | From | "Serge E. Hallyn" <> | Subject | Re: [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
| |