[patch 1/1] SKAS3: fix mm->dumpable handling

[patch 1/1] SKAS3: fix mm->dumpable handling

[blaisorblade_spam]

From: Paolo 'Blaisorblade' Giarrusso <blaisorblade_spam@yahoo.it>, Henrik Nordstrom <uml@hno.marasystems.com>, Michael Richardson <mcr@sandelman.ottawa.on.ca>

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 <blaisorblade_spam@yahoo.it>
---

 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, &current->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;
_

Re: [uml-devel] [patch 1/1] SKAS3: fix mm->dumpable handling

[BlaisorBlade]

On Thursday 14 October 2004 18:17, blaisorblade_spam@yahoo.it wrote:
> From: Paolo 'Blaisorblade' Giarrusso <blaisorblade_spam@yahoo.it>, Henrik
> Nordstrom <uml@hno.marasystems.com>, Michael Richardson
> <mcr@sandelman.ottawa.on.ca>

> 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