Re: [announce] [patch] NX (No eXecute) support for x86, 2.6.7-rc2-bk2

[Ingo Molnar <mingo@elte.hu>]

* Andi Kleen <ak@suse.de> wrote:

> > The whole point of NX, though, is that it prevents certain classes of 
> > exploits.  If a setuid binary is vulnerable to one of these, then Ingo's 
> > patch "fixes" it.  Your approach breaks that.
> 
> Good point.
> 
> But that only applies to the NX personality bit. For the uname
> emulation it is not an issue.
> 
> So maybe the dropping on exec should only zero a few selected
> personality bits, but not all.

ok, how about the attached patch then? There's a PERS_DROP_ON_SUID mask
that we drop upon setuid - all the other personality bits get inherited.

	Ingo

--- linux/include/linux/personality.h.orig	
+++ linux/include/linux/personality.h	
@@ -30,6 +30,7 @@ extern int abi_fake_utsname;
  */
 enum {
 	MMAP_PAGE_ZERO =	0x0100000,
+	ADDR_SPACE_EXECUTABLE =	0x0200000,
 	ADDR_LIMIT_32BIT =	0x0800000,
 	SHORT_INODE =		0x1000000,
 	WHOLE_SECONDS =		0x2000000,
@@ -37,6 +38,8 @@ enum {
 	ADDR_LIMIT_3GB = 	0x8000000,
 };
 
+#define PERS_DROP_ON_SUID (MMAP_PAGE_ZERO|ADDR_SPACE_EXECUTABLE)
+
 /*
  * Personality types.
  *
--- linux/include/asm-i386/elf.h.orig	
+++ linux/include/asm-i386/elf.h	
@@ -117,7 +117,8 @@ typedef struct user_fxsr_struct elf_fpxr
 #define AT_SYSINFO_EHDR		33
 
 #ifdef __KERNEL__
-#define SET_PERSONALITY(ex, ibcs2) set_personality((ibcs2)?PER_SVR4:PER_LINUX)
+/* child inherits the personality of the parent */
+#define SET_PERSONALITY(ex, ibcs2) do { } while (0)
 
 extern int dump_task_regs (struct task_struct *, elf_gregset_t *);
 extern int dump_task_fpu (struct task_struct *, elf_fpregset_t *);
--- linux/fs/exec.c.orig	
+++ linux/fs/exec.c	
@@ -886,8 +886,11 @@ int prepare_binprm(struct linux_binprm *
 
 	if(!(bprm->file->f_vfsmnt->mnt_flags & MNT_NOSUID)) {
 		/* Set-uid? */
-		if (mode & S_ISUID)
+		if (mode & S_ISUID) {
 			bprm->e_uid = inode->i_uid;
+			/* reset personality */
+			current->personality &= ~PERS_DROP_ON_SUID;
+		}
 
 		/* Set-gid? */
 		/*
@@ -895,8 +898,11 @@ int prepare_binprm(struct linux_binprm *
 		 * is a candidate for mandatory locking, not a setgid
 		 * executable.
 		 */
-		if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP))
+		if ((mode & (S_ISGID | S_IXGRP)) == (S_ISGID | S_IXGRP)) {
 			bprm->e_gid = inode->i_gid;
+			/* reset personality */
+			current->personality &= ~PERS_DROP_ON_SUID;
+		}
 	}
 
 	/* fill in binprm security blob */
--- linux/fs/binfmt_elf.c.orig	
+++ linux/fs/binfmt_elf.c	
@@ -490,7 +490,7 @@ static int load_elf_binary(struct linux_
   	struct exec interp_ex;
 	char passed_fileno[6];
 	struct files_struct *files;
-	int executable_stack = EXSTACK_DEFAULT;
+	int executable_stack;
 	
 	/* Get the exec-header */
 	elf_ex = *((struct elfhdr *) bprm->buf);
@@ -616,13 +616,19 @@ static int load_elf_binary(struct linux_
 	}
 
 	elf_ppnt = elf_phdata;
-	for (i = 0; i < elf_ex.e_phnum; i++, elf_ppnt++)
-		if (elf_ppnt->p_type == PT_GNU_STACK) {
-			if (elf_ppnt->p_flags & PF_X)
-				executable_stack = EXSTACK_ENABLE_X;
-			else
-				executable_stack = EXSTACK_DISABLE_X;
-		}
+	if (current->personality & ADDR_SPACE_EXECUTABLE)
+		executable_stack = EXSTACK_ENABLE_X;
+	else {
+		executable_stack = EXSTACK_DEFAULT;
+		for (i = 0; i < elf_ex.e_phnum; i++, elf_ppnt++)
+			if (elf_ppnt->p_type == PT_GNU_STACK) {
+				if (elf_ppnt->p_flags & PF_X)
+					executable_stack = EXSTACK_ENABLE_X;
+				else
+					executable_stack = EXSTACK_DISABLE_X;
+				break;
+			}
+	}
 
 	/* Some simple consistency checks for the interpreter */
 	if (elf_interpreter) {