From: Paolo 'Blaisorblade' Giarrusso <[email protected]>, Henrik Nordstrom <[email protected]>, Michael Richardson <[email protected]>
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 <pid> is the pid of a userspace thread,
/proc/<pid> 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 <[email protected]>
---
vanilla-linux-2.6.7-SKAS-paolo/arch/i386/kernel/ptrace.c | 8 ++++++++
vanilla-linux-2.6.7-SKAS-paolo/mm/proc_mm.c | 2 ++
2 files changed, 10 insertions(+)
diff -puN mm/proc_mm.c~fix-dumpable-handling mm/proc_mm.c
--- vanilla-linux-2.6.7-SKAS/mm/proc_mm.c~fix-dumpable-handling 2004-10-14 16:58:17.473473200 +0200
+++ vanilla-linux-2.6.7-SKAS-paolo/mm/proc_mm.c 2004-10-14 18:15:21.294545552 +0200
@@ -125,6 +125,8 @@ static int open_proc_mm(struct inode *in
goto out_mem;
init_new_empty_context(mm);
+ mm->dumpable = current->mm->dumpable;
+ wmb();
spin_lock(&mmlist_lock);
list_add(&mm->mmlist, ¤t->mm->mmlist);
diff -puN arch/i386/kernel/ptrace.c~fix-dumpable-handling arch/i386/kernel/ptrace.c
--- vanilla-linux-2.6.7-SKAS/arch/i386/kernel/ptrace.c~fix-dumpable-handling 2004-10-14 17:05:30.651620112 +0200
+++ vanilla-linux-2.6.7-SKAS-paolo/arch/i386/kernel/ptrace.c 2004-10-14 18:15:21.297545096 +0200
@@ -559,6 +559,14 @@ asmlinkage int sys_ptrace(long request,
break;
}
+ /* Let's be safe. If we are ptraced from a non-dumpable process,
+ * let's not be dumpable. Don't try to be smart and turn
+ * current->dumpable to 1: it may be unsafe.*/
+ if (!current->dumpable) {
+ new->dumpable = 0;
+ wmb();
+ }
+
atomic_inc(&new->mm_users);
child->mm = new;
child->active_mm = new;
_
On Thursday 14 October 2004 18:17, [email protected] wrote:
> From: Paolo 'Blaisorblade' Giarrusso <[email protected]>, Henrik
> Nordstrom <[email protected]>, Michael Richardson
> <[email protected]>
> 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 <pid> is the pid of a userspace thread,
> /proc/<pid> 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.
And Paolo (myself) fucked it up:
> + /* Let's be safe. If we are ptraced from a non-dumpable process,
> + * let's not be dumpable. Don't try to be smart and turn
> + * current->dumpable to 1: it may be unsafe.*/
> + if (!current->dumpable) {
This should be current->mm->dumpable, not current->dumpable.
> + new->dumpable = 0;
> + wmb();
> + }
> +
> atomic_inc(&new->mm_users);
> child->mm = new;
> child->active_mm = new;
> _
Also, we have not studied the case of another process trying to ptrace and
SWITCH_MM a process already having some mm's (it cannot give it any already
existing mm, because it has not the file descriptor). It seems safe to me,
but I'm going to study this and put the result in next patch version.
--
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729