linux-kernel - Re: [PATCH] close mprotect noexec hole

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.64.0610151141280.3952@g5.osdl.org>
Date:	Sun, 15 Oct 2006 11:47:37 -0700 (PDT)
From:	Linus Torvalds <torvalds@...l.org>
To:	Ulrich Drepper <drepper@...hat.com>
cc:	akpm@...l.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] close mprotect noexec hole



On Sun, 15 Oct 2006, Ulrich Drepper wrote:
>
> The following patch closes the hole in mprotect discovered during
> the noexec mount discussions.  Without this the protection is
> incomplete and pretty much useless.  With it and additional techniques
> like SELinux all holes can be plugged in a fine-grained way.

This patch seems totally buggy.

mprotect() can cover _multiple_ mappings, and this one only checks the 
very first one, as far as I can tell.

The place to do this is where we do the "security_file_mprotect()", not 
where you did it. 

Ie something like this instead. Totally untested, but at least it compiles 
with current -git (unlike Uli's version - needs <linux/mount.h>)

		Linus
---
diff --git a/mm/mprotect.c b/mm/mprotect.c
index 3b8f3c0..09ed8de 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -21,6 +21,7 @@ #include <linux/personality.h>
 #include <linux/syscalls.h>
 #include <linux/swap.h>
 #include <linux/swapops.h>
+#include <linux/mount.h>
 #include <asm/uaccess.h>
 #include <asm/pgtable.h>
 #include <asm/cacheflush.h>
@@ -280,9 +281,14 @@ sys_mprotect(unsigned long start, size_t
 		newflags = vm_flags | (vma->vm_flags & ~(VM_READ | VM_WRITE | VM_EXEC));
 
 		/* newflags >> 4 shift VM_MAY% in place of VM_% */
-		if ((newflags & ~(newflags >> 4)) & (VM_READ | VM_WRITE | VM_EXEC)) {
-			error = -EACCES;
+		error = -EACCES;
+		if ((newflags & ~(newflags >> 4)) & (VM_READ | VM_WRITE | VM_EXEC))
 			goto out;
+
+		if (newflags & VM_EXEC) {
+			struct file *file = vma->vm_file;
+			if (file && (file->f_vfsmnt->mnt_flags & MNT_NOEXEC))
+				goto out;
 		}
 
 		error = security_file_mprotect(vma, reqprot, prot);
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/