From df26c40e567356caeefe2861311e19c54444d917 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Mon, 26 Jun 2006 00:25:59 -0700 Subject: [PATCH] proc: Cleanup proc_fd_access_allowed In process of getting proc_fd_access_allowed to work it has developed a few warts. In particular the special case that always allows introspection and the special case to allow inspection of kernel threads. The special case for introspection is needed for /proc/self/mem. The special case for kernel threads really should be overridable by security modules. So consolidate these checks into ptrace.c:may_attach(). The check to always allow introspection is trivial. The check to allow access to kernel threads, and zombies is a little trickier. mem_read and mem_write already verify an mm exists so it isn't needed twice. proc_fd_access_allowed only doesn't want a check to verify task->mm exits, s it prevents all access to kernel threads. So just move the task->mm check into ptrace_attach where it is needed for practical reasons. I did a quick audit and none of the security modules in the kernel seem to care if they are passed a task without an mm into security_ptrace. So the above move should be safe and it allows security modules to come up with more restrictive policy. Signed-off-by: Eric W. Biederman Cc: Stephen Smalley Cc: Chris Wright Cc: James Morris Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- kernel/ptrace.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) (limited to 'kernel/ptrace.c') diff --git a/kernel/ptrace.c b/kernel/ptrace.c index 921c22ad16e4..6252d2fa2bf3 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -120,8 +120,18 @@ int ptrace_check_attach(struct task_struct *child, int kill) static int may_attach(struct task_struct *task) { - if (!task->mm) - return -EPERM; + /* May we inspect the given task? + * This check is used both for attaching with ptrace + * and for allowing access to sensitive information in /proc. + * + * ptrace_attach denies several cases that /proc allows + * because setting up the necessary parent/child relationship + * or halting the specified task is impossible. + */ + int dumpable = 0; + /* Don't let security modules deny introspection */ + if (task == current) + return 0; if (((current->uid != task->euid) || (current->uid != task->suid) || (current->uid != task->uid) || @@ -130,7 +140,9 @@ static int may_attach(struct task_struct *task) (current->gid != task->gid)) && !capable(CAP_SYS_PTRACE)) return -EPERM; smp_rmb(); - if (!task->mm->dumpable && !capable(CAP_SYS_PTRACE)) + if (task->mm) + dumpable = task->mm->dumpable; + if (!dumpable && !capable(CAP_SYS_PTRACE)) return -EPERM; return security_ptrace(current, task); @@ -176,6 +188,8 @@ repeat: goto repeat; } + if (!task->mm) + goto bad; /* the same process cannot be attached many times */ if (task->ptrace & PT_PTRACED) goto bad; -- cgit v1.2.3 From d5f70c00ad24cd1158d3678b44ff969b4c971d49 Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Mon, 26 Jun 2006 00:26:07 -0700 Subject: [PATCH] coredump: kill ptrace related stuff With this patch zap_process() sets SIGNAL_GROUP_EXIT while sending SIGKILL to the thread group. This means that a TASK_TRACED task 1. Will be awakened by signal_wake_up(1) 2. Can't sleep again via ptrace_notify() 3. Can't go to do_signal_stop() after return from ptrace_stop() in get_signal_to_deliver() So we can remove all ptrace related stuff from coredump path. Signed-off-by: Oleg Nesterov Cc: "Eric W. Biederman" Cc: Roland McGrath Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/exec.c | 30 +++++------------------------- include/linux/ptrace.h | 1 - kernel/ptrace.c | 3 ++- kernel/signal.c | 35 ++++++++++++++++++++++++++++++----- 4 files changed, 37 insertions(+), 32 deletions(-) (limited to 'kernel/ptrace.c') diff --git a/fs/exec.c b/fs/exec.c index a5c51646d1ad..b58ba7d127e0 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1368,12 +1368,14 @@ static void format_corename(char *corename, const char *pattern, long signr) *out_ptr = 0; } -static void zap_process(struct task_struct *start, int *ptraced) +static void zap_process(struct task_struct *start) { struct task_struct *t; unsigned long flags; spin_lock_irqsave(&start->sighand->siglock, flags); + start->signal->flags = SIGNAL_GROUP_EXIT; + start->signal->group_stop_count = 0; t = start; do { @@ -1381,22 +1383,17 @@ static void zap_process(struct task_struct *start, int *ptraced) t->mm->core_waiters++; sigaddset(&t->pending.signal, SIGKILL); signal_wake_up(t, 1); - - if (unlikely(t->ptrace) && - unlikely(t->parent->mm == t->mm)) - *ptraced = 1; } } while ((t = next_thread(t)) != start); spin_unlock_irqrestore(&start->sighand->siglock, flags); } -static void zap_threads (struct mm_struct *mm) +static void zap_threads(struct mm_struct *mm) { struct task_struct *g, *p; struct task_struct *tsk = current; struct completion *vfork_done = tsk->vfork_done; - int traced = 0; /* * Make sure nobody is waiting for us to release the VM, @@ -1413,29 +1410,12 @@ static void zap_threads (struct mm_struct *mm) do { if (p->mm) { if (p->mm == mm) - zap_process(p, &traced); + zap_process(p); break; } } while ((p = next_thread(p)) != g); } read_unlock(&tasklist_lock); - - if (unlikely(traced)) { - /* - * We are zapping a thread and the thread it ptraces. - * If the tracee went into a ptrace stop for exit tracing, - * we could deadlock since the tracer is waiting for this - * coredump to finish. Detach them so they can both die. - */ - write_lock_irq(&tasklist_lock); - do_each_thread(g,p) { - if (mm == p->mm && p != tsk && - p->ptrace && p->parent->mm == mm) { - __ptrace_detach(p, 0); - } - } while_each_thread(g,p); - write_unlock_irq(&tasklist_lock); - } } static void coredump_wait(struct mm_struct *mm) diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h index ee918bc6e18c..8b2749a259dc 100644 --- a/include/linux/ptrace.h +++ b/include/linux/ptrace.h @@ -88,7 +88,6 @@ extern int ptrace_readdata(struct task_struct *tsk, unsigned long src, char __us extern int ptrace_writedata(struct task_struct *tsk, char __user *src, unsigned long dst, int len); extern int ptrace_attach(struct task_struct *tsk); extern int ptrace_detach(struct task_struct *, unsigned int); -extern void __ptrace_detach(struct task_struct *, unsigned int); extern void ptrace_disable(struct task_struct *); extern int ptrace_check_attach(struct task_struct *task, int kill); extern int ptrace_request(struct task_struct *child, long request, long addr, long data); diff --git a/kernel/ptrace.c b/kernel/ptrace.c index 6252d2fa2bf3..335c5b932e14 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -214,7 +214,7 @@ out: return retval; } -void __ptrace_detach(struct task_struct *child, unsigned int data) +static inline void __ptrace_detach(struct task_struct *child, unsigned int data) { child->exit_code = data; /* .. re-parent .. */ @@ -233,6 +233,7 @@ int ptrace_detach(struct task_struct *child, unsigned int data) ptrace_disable(child); write_lock_irq(&tasklist_lock); + /* protect against de_thread()->release_task() */ if (child->ptrace) __ptrace_detach(child, data); write_unlock_irq(&tasklist_lock); diff --git a/kernel/signal.c b/kernel/signal.c index 1b3c921737e2..52adf53929f6 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -1531,6 +1531,35 @@ static void do_notify_parent_cldstop(struct task_struct *tsk, int why) spin_unlock_irqrestore(&sighand->siglock, flags); } +static inline int may_ptrace_stop(void) +{ + if (!likely(current->ptrace & PT_PTRACED)) + return 0; + + if (unlikely(current->parent == current->real_parent && + (current->ptrace & PT_ATTACHED))) + return 0; + + if (unlikely(current->signal == current->parent->signal) && + unlikely(current->signal->flags & SIGNAL_GROUP_EXIT)) + return 0; + + /* + * Are we in the middle of do_coredump? + * If so and our tracer is also part of the coredump stopping + * is a deadlock situation, and pointless because our tracer + * is dead so don't allow us to stop. + * If SIGKILL was already sent before the caller unlocked + * ->siglock we must see ->core_waiters != 0. Otherwise it + * is safe to enter schedule(). + */ + if (unlikely(current->mm->core_waiters) && + unlikely(current->mm == current->parent->mm)) + return 0; + + return 1; +} + /* * This must be called with current->sighand->siglock held. * @@ -1559,11 +1588,7 @@ static void ptrace_stop(int exit_code, int nostop_code, siginfo_t *info) spin_unlock_irq(¤t->sighand->siglock); try_to_freeze(); read_lock(&tasklist_lock); - if (likely(current->ptrace & PT_PTRACED) && - likely(current->parent != current->real_parent || - !(current->ptrace & PT_ATTACHED)) && - (likely(current->parent->signal != current->signal) || - !unlikely(current->signal->flags & SIGNAL_GROUP_EXIT))) { + if (may_ptrace_stop()) { do_notify_parent_cldstop(current, CLD_TRAPPED); read_unlock(&tasklist_lock); schedule(); -- cgit v1.2.3