SKAS3: fix mm->dumpable handling From: Paolo 'Blaisorblade' Giarrusso , Henrik Nordstrom , Michael Richardson When a child mm is created by opening /proc/mm, without this patch its mm->dumpable flag is left set to 0, even when there is no reason to do so. This way, for instance, if is the pid of a userspace thread, /proc/ is only readable by root (which was the original reason letting this be diagnosed by Michael Richardson). Paolo and Henrik discussed about this in detail, finally Paolo wrote the patch and sent it for comment. Signed-off-by: Paolo 'Blaisorblade' Giarrusso Index: linux-2.6.14/arch/i386/Kconfig =================================================================== --- linux-2.6.14.orig/arch/i386/Kconfig 2005-10-29 05:50:05.000000000 +0200 +++ linux-2.6.14/arch/i386/Kconfig 2005-10-29 05:50:33.000000000 +0200 @@ -760,6 +760,22 @@ bool "/proc/mm support" default y +config PROC_MM_DUMPABLE + bool "Make UML childs /proc/ completely browsable" + default n + help + If in doubt, say N. + + This fiddles with some settings to make sure /proc/ is completely + browsable by who started UML, at the expense of some additional + locking (maybe this could slow down the runned UMLs of a few percents, + I've not tested this). + + Also, if there is a bug in this feature, there is some little + possibility to do privilege escalation if you have UML installed + setuid (which you shouldn't have done) or if UML changes uid on + startup (which will be a good thing, when enabled) ... + # Common NUMA Features config NUMA bool "Numa Memory Allocation and Scheduler Support" Index: linux-2.6.14/arch/i386/kernel/ptrace.c =================================================================== --- linux-2.6.14.orig/arch/i386/kernel/ptrace.c 2005-10-29 05:50:24.000000000 +0200 +++ linux-2.6.14/arch/i386/kernel/ptrace.c 2005-10-29 05:50:33.000000000 +0200 @@ -702,10 +702,14 @@ } atomic_inc(&new->mm_users); - task_lock(child); + + lock_fix_dumpable_setting(child, new); + child->mm = new; child->active_mm = new; + task_unlock(child); + mmput(old); ret = 0; break; Index: linux-2.6.14/arch/x86_64/ia32/ptrace32.c =================================================================== --- linux-2.6.14.orig/arch/x86_64/ia32/ptrace32.c 2005-10-29 05:50:33.000000000 +0200 +++ linux-2.6.14/arch/x86_64/ia32/ptrace32.c 2005-10-29 05:50:33.000000000 +0200 @@ -411,10 +411,14 @@ } atomic_inc(&new->mm_users); - task_lock(child); + + lock_fix_dumpable_setting(child, new); + child->mm = new; child->active_mm = new; + task_unlock(child); + mmput(old); ret = 0; break; Index: linux-2.6.14/arch/x86_64/Kconfig =================================================================== --- linux-2.6.14.orig/arch/x86_64/Kconfig 2005-10-29 05:50:31.000000000 +0200 +++ linux-2.6.14/arch/x86_64/Kconfig 2005-10-29 05:50:33.000000000 +0200 @@ -361,6 +361,22 @@ bool "/proc/mm support" default y +config PROC_MM_DUMPABLE + bool "Make UML childs /proc/ completely browsable" + default n + help + If in doubt, say N. + + This fiddles with some settings to make sure /proc/ is completely + browsable by who started UML, at the expense of some additional + locking (maybe this could slow down the runned UMLs of a few percents, + I've not tested this). + + Also, if there is a bug in this feature, there is some little + possibility to do privilege escalation if you have UML installed + setuid (which you shouldn't have done) or if UML changes uid on + startup (which will be a good thing, when enabled) ... + config X86_MCE bool "Machine check support" if EMBEDDED default y Index: linux-2.6.14/arch/x86_64/kernel/ptrace.c =================================================================== --- linux-2.6.14.orig/arch/x86_64/kernel/ptrace.c 2005-10-29 05:50:33.000000000 +0200 +++ linux-2.6.14/arch/x86_64/kernel/ptrace.c 2005-10-29 05:50:33.000000000 +0200 @@ -650,10 +650,14 @@ } atomic_inc(&new->mm_users); - task_lock(child); + + lock_fix_dumpable_setting(child, new); + child->mm = new; child->active_mm = new; + task_unlock(child); + mmput(old); ret = 0; break; Index: linux-2.6.14/include/linux/proc_mm.h =================================================================== --- linux-2.6.14.orig/include/linux/proc_mm.h 2005-10-29 05:50:31.000000000 +0200 +++ linux-2.6.14/include/linux/proc_mm.h 2005-10-29 05:50:33.000000000 +0200 @@ -8,6 +8,7 @@ #include #include +#include /* The differences between this one and do_mmap are that: * - we must perform controls for userspace-supplied params (which are @@ -90,4 +91,24 @@ extern struct mm_struct *proc_mm_get_mm(int fd); +/* Cope with older kernels */ +#ifndef __acquires +#define __acquires(x) +#endif + +#ifdef CONFIG_PROC_MM_DUMPABLE +/* + * Since we take task_lock of child and it's needed also by the caller, we + * return with it locked. + */ +extern void lock_fix_dumpable_setting(struct task_struct * child, + struct mm_struct* new) __acquires(child->alloc_lock); +#else +static inline void lock_fix_dumpable_setting(struct task_struct * child, + struct mm_struct* new) __acquires(child->alloc_lock) +{ + task_lock(child); +} +#endif + #endif Index: linux-2.6.14/mm/proc_mm.c =================================================================== --- linux-2.6.14.orig/mm/proc_mm.c 2005-10-29 05:50:31.000000000 +0200 +++ linux-2.6.14/mm/proc_mm.c 2005-10-29 05:50:33.000000000 +0200 @@ -4,6 +4,7 @@ */ #include +#include #include #include #include @@ -13,6 +14,62 @@ #include #include +#ifdef CONFIG_PROC_MM_DUMPABLE +/* Checks if a task must be considered dumpable + * + * XXX: copied from fs/proc/base.c, removed task_lock, added rmb(): this must be + * called with task_lock(task) held. */ +static int task_dumpable(struct task_struct *task) +{ + int dumpable = 0; + struct mm_struct *mm; + + mm = task->mm; + if (mm) { + rmb(); + dumpable = mm->dumpable; + } + return dumpable; +} + +/* + * This is to be used in PTRACE_SWITCH_MM handling. We are going to set + * child->mm to new, and we must first correctly set new->dumpable. + * Since we take task_lock of child and it's needed also by the caller, we + * return with it locked. + */ +void lock_fix_dumpable_setting(struct task_struct* child, struct mm_struct* new) + __acquires(child->alloc_lock) +{ + int dumpable = 1; + + /* We must be safe. + * If the child is ptraced from a non-dumpable process, + * let's not be dumpable. If the child is non-dumpable itself, + * copy this property across mm's. + * + * Don't try to be smart for the opposite case and turn + * child->mm->dumpable to 1: I've not made sure it is safe. + */ + + task_lock(current); + if (unlikely(!task_dumpable(current))) { + dumpable = 0; + } + task_unlock(current); + + task_lock(child); + if (likely(dumpable) && unlikely(!task_dumpable(child))) { + dumpable = 0; + } + + if (!dumpable) { + new->dumpable = 0; + wmb(); + } +} +#endif + /* Naming conventions are a mess, so I note them down. * * Things ending in _mm can be for everything. It's only for @@ -41,6 +98,10 @@ init_new_empty_context(mm); arch_pick_mmap_layout(mm); +#ifdef CONFIG_PROC_MM_DUMPABLE + mm->dumpable = current->mm->dumpable; + wmb(); +#endif file->private_data = mm;