[uml-devel] mm->dumpable issue (was: Re: /proc/mm patches)

[Blaisorblade <blaisorblade_spam@yahoo.it>]

[-- Attachment #1: Type: text/plain, Size: 810 bytes --]

On Wednesday 03 November 2004 00:00, Michael Richardson wrote:
> Is there a final patch that you think might work, and deal with the
> mm->dumpable flag issue?

> I was all set to test the previous patch, but then you said not to...
On top of -V7 you can apply the attached patch. I'm almost sure it works. The 
problem is that I have not the certainty that it is not exploitable, i.e. it 
can be used to gain access to root privileges. I am almost sure it is not... 
but please *avoid* using a setuid UML binary with this patch (it is not a 
good idea, anyway, but it is even worse with this patch. I believe that to 
exploit it, if at all possible, having a setuid binary which is coded to use 
SKAS (either UML or a malicious program).

-- 
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729

[-- Attachment #2: fix-dumpable-handling.patch --]
[-- Type: text/x-diff, Size: 2018 bytes --]


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.9-paolo/arch/i386/kernel/ptrace.c |    8 ++++++++
 vanilla-linux-2.6.9-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.9/mm/proc_mm.c~fix-dumpable-handling	2004-10-26 00:54:35.478864480 +0200
+++ vanilla-linux-2.6.9-paolo/mm/proc_mm.c	2004-10-26 00:54:35.482863872 +0200
@@ -126,6 +126,8 @@ static int open_proc_mm(struct inode *in
 
 	init_new_empty_context(mm);
 	arch_pick_mmap_layout(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.9/arch/i386/kernel/ptrace.c~fix-dumpable-handling	2004-10-26 00:54:35.480864176 +0200
+++ vanilla-linux-2.6.9-paolo/arch/i386/kernel/ptrace.c	2004-10-26 00:54:35.483863720 +0200
@@ -568,6 +568,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;
_