Non-Exec stack patches

Non-Exec stack patches

[Kurt Garloff]

[-- Attachment #1.1: Type: text/plain, Size: 867 bytes --]

Hi,

find attached a patch to parse the elf binaries for a PT_GNU_STACK
section to set the stack non-executable if possible.
Most parts have been shamelessly stolen from Ingo Molnar's more
ambitious stackshield
http://people.redhat.com/mingo/exec-shield/exec-shield-2.6.4-C9

The toolchain has meainwhile support for marking the binaries with a
PT_GNU_STACK section with ot without x bit as needed.

If no such section is found, we leave the stack to whatever the 
arch defaults to. If there is one, we explicitly disabled the VM_EXEC 
bit if no x bit is found, otherwise explicitly enable.

I believe this part should be merged into official mainstream kernels.
Ingo, what do you think?

Regards,
-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

[-- Attachment #1.2: noexec_stack4 --]
[-- Type: text/plain, Size: 13247 bytes --]

diff -uNrp linux-2.6.4.EXSTACK/arch/ia64/ia32/binfmt_elf32.c linux-2.6.4.noexec/arch/ia64/ia32/binfmt_elf32.c
--- linux-2.6.4.EXSTACK/arch/ia64/ia32/binfmt_elf32.c	2004-03-23 21:18:35.000000000 +0100
+++ linux-2.6.4.noexec/arch/ia64/ia32/binfmt_elf32.c	2004-03-23 22:48:31.000000000 +0100
@@ -35,7 +35,7 @@ extern void ia64_elf32_init (struct pt_r
 
 static void elf32_set_personality (void);
 
-#define setup_arg_pages(bprm)		ia32_setup_arg_pages(bprm)
+#define setup_arg_pages(bprm,exec)		ia32_setup_arg_pages(bprm,exec)
 #define elf_map				elf32_map
 
 #undef SET_PERSONALITY
@@ -152,7 +152,7 @@ ia64_elf32_init (struct pt_regs *regs)
 }
 
 int
-ia32_setup_arg_pages (struct linux_binprm *bprm)
+ia32_setup_arg_pages (struct linux_binprm *bprm, int executable_stack)
 {
 	unsigned long stack_base;
 	struct vm_area_struct *mpnt;
@@ -181,8 +181,14 @@ ia32_setup_arg_pages (struct linux_binpr
 		mpnt->vm_mm = current->mm;
 		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
 		mpnt->vm_end = IA32_STACK_TOP;
-		mpnt->vm_page_prot = PAGE_COPY;
-		mpnt->vm_flags = VM_STACK_FLAGS;
+		if (executable_stack > 0)
+			mpnt->vm_flags = VM_STACK_FLAGS |  VM_EXEC;
+		else if (executable_stack < 0)
+			mpnt->vm_flags = VM_STACK_FLAGS & ~VM_EXEC;
+		else
+			mpnt->vm_flags = VM_STACK_FLAGS;
+		mpnt->vm_page_prot = (mpnt->vm_flags & VM_EXEC)?
+					PAGE_COPY_EXEC: PAGE_COPY;
 		mpnt->vm_ops = NULL;
 		mpnt->vm_pgoff = mpnt->vm_start >> PAGE_SHIFT;
 		mpnt->vm_file = NULL;
@@ -197,7 +203,7 @@ ia32_setup_arg_pages (struct linux_binpr
 		struct page *page = bprm->page[i];
 		if (page) {
 			bprm->page[i] = NULL;
-			put_dirty_page(current, page, stack_base, PAGE_COPY, mpnt);
+			put_dirty_page(current, page, stack_base, mpnt->vm_page_prot, mpnt);
 		}
 		stack_base += PAGE_SIZE;
 	}
diff -uNrp linux-2.6.4.EXSTACK/arch/ia64/ia32/ia32priv.h linux-2.6.4.noexec/arch/ia64/ia32/ia32priv.h
--- linux-2.6.4.EXSTACK/arch/ia64/ia32/ia32priv.h	2004-03-11 03:55:51.000000000 +0100
+++ linux-2.6.4.noexec/arch/ia64/ia32/ia32priv.h	2004-03-23 22:56:20.000000000 +0100
@@ -494,7 +494,7 @@ struct ia32_user_desc {
 struct linux_binprm;
 
 extern void ia32_init_addr_space (struct pt_regs *regs);
-extern int ia32_setup_arg_pages (struct linux_binprm *bprm);
+extern int ia32_setup_arg_pages (struct linux_binprm *bprm, int exec_stack);
 extern unsigned long ia32_do_mmap (struct file *, unsigned long, unsigned long, int, int, loff_t);
 extern void ia32_load_segment_descriptors (struct task_struct *task);
 
diff -uNrp linux-2.6.4.EXSTACK/arch/mips/kernel/irixelf.c linux-2.6.4.noexec/arch/mips/kernel/irixelf.c
--- linux-2.6.4.EXSTACK/arch/mips/kernel/irixelf.c	2004-03-11 03:56:03.000000000 +0100
+++ linux-2.6.4.noexec/arch/mips/kernel/irixelf.c	2004-03-23 22:56:42.000000000 +0100
@@ -688,7 +688,7 @@ static int load_irix_binary(struct linux
 	 * change some of these later.
 	 */
 	current->mm->rss = 0;
-	setup_arg_pages(bprm);
+	setup_arg_pages(bprm, 0);
 	current->mm->start_stack = bprm->p;
 
 	/* At this point, we assume that the image should be loaded at
diff -uNrp linux-2.6.4.EXSTACK/arch/s390/kernel/binfmt_elf32.c linux-2.6.4.noexec/arch/s390/kernel/binfmt_elf32.c
--- linux-2.6.4.EXSTACK/arch/s390/kernel/binfmt_elf32.c	2004-03-11 03:55:44.000000000 +0100
+++ linux-2.6.4.noexec/arch/s390/kernel/binfmt_elf32.c	2004-03-23 22:51:25.000000000 +0100
@@ -114,7 +114,7 @@ typedef s390_regs32 elf_gregset_t;
 #include <linux/binfmts.h>
 #include <linux/compat.h>
 
-int setup_arg_pages32(struct linux_binprm *bprm);
+int setup_arg_pages32(struct linux_binprm *bprm, int executable_stack);
 
 #define elf_prstatus elf_prstatus32
 struct elf_prstatus32
@@ -165,7 +165,7 @@ struct elf_prpsinfo32
 
 #undef start_thread
 #define start_thread                    start_thread31 
-#define setup_arg_pages(bprm)           setup_arg_pages32(bprm)
+#define setup_arg_pages(bprm, exec)     setup_arg_pages32(bprm, exec)
 #define elf_map				elf_map32
 
 MODULE_DESCRIPTION("Binary format loader for compatibility with 32bit Linux for S390 binaries,"
diff -uNrp linux-2.6.4.EXSTACK/arch/s390/kernel/compat_exec.c linux-2.6.4.noexec/arch/s390/kernel/compat_exec.c
--- linux-2.6.4.EXSTACK/arch/s390/kernel/compat_exec.c	2004-03-23 21:18:35.000000000 +0100
+++ linux-2.6.4.noexec/arch/s390/kernel/compat_exec.c	2004-03-23 22:58:15.000000000 +0100
@@ -37,7 +37,7 @@
 #undef STACK_TOP
 #define STACK_TOP TASK31_SIZE
 
-int setup_arg_pages32(struct linux_binprm *bprm)
+int setup_arg_pages32(struct linux_binprm *bprm, int executable_stack)
 {
 	unsigned long stack_base;
 	struct vm_area_struct *mpnt;
@@ -66,6 +66,7 @@ int setup_arg_pages32(struct linux_binpr
 		mpnt->vm_mm = mm;
 		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
 		mpnt->vm_end = STACK_TOP;
+		/* executable stack setting would be applied here */
 		mpnt->vm_page_prot = PAGE_COPY;
 		mpnt->vm_flags = VM_STACK_FLAGS;
 		mpnt->vm_ops = NULL;
diff -uNrp linux-2.6.4.EXSTACK/arch/sparc64/kernel/binfmt_aout32.c linux-2.6.4.noexec/arch/sparc64/kernel/binfmt_aout32.c
--- linux-2.6.4.EXSTACK/arch/sparc64/kernel/binfmt_aout32.c	2004-03-11 03:55:27.000000000 +0100
+++ linux-2.6.4.noexec/arch/sparc64/kernel/binfmt_aout32.c	2004-03-23 22:49:50.000000000 +0100
@@ -310,7 +310,7 @@ beyond_if:
 	orig_thr_flags = current_thread_info()->flags;
 	current_thread_info()->flags |= _TIF_32BIT;
 
-	retval = setup_arg_pages(bprm);
+	retval = setup_arg_pages(bprm, 0);
 	if (retval < 0) { 
 		current_thread_info()->flags = orig_thr_flags;
 
diff -uNrp linux-2.6.4.EXSTACK/arch/x86_64/ia32/ia32_aout.c linux-2.6.4.noexec/arch/x86_64/ia32/ia32_aout.c
--- linux-2.6.4.EXSTACK/arch/x86_64/ia32/ia32_aout.c	2004-03-11 03:55:54.000000000 +0100
+++ linux-2.6.4.noexec/arch/x86_64/ia32/ia32_aout.c	2004-03-23 22:59:47.000000000 +0100
@@ -35,7 +35,7 @@
 #undef WARN_OLD
 #undef CORE_DUMP /* probably broken */
 
-extern int ia32_setup_arg_pages(struct linux_binprm *bprm);
+extern int ia32_setup_arg_pages(struct linux_binprm *bprm, int exec_stack);
 
 static int load_aout_binary(struct linux_binprm *, struct pt_regs * regs);
 static int load_aout_library(struct file*);
@@ -395,7 +395,7 @@ beyond_if:
 
 	set_brk(current->mm->start_brk, current->mm->brk);
 
-	retval = ia32_setup_arg_pages(bprm); 
+	retval = ia32_setup_arg_pages(bprm, 0);
 	if (retval < 0) { 
 		/* Someone check-me: is this error path enough? */ 
 		send_sig(SIGKILL, current, 0); 
diff -uNrp linux-2.6.4.EXSTACK/arch/x86_64/ia32/ia32_binfmt.c linux-2.6.4.noexec/arch/x86_64/ia32/ia32_binfmt.c
--- linux-2.6.4.EXSTACK/arch/x86_64/ia32/ia32_binfmt.c	2004-03-23 22:42:31.000000000 +0100
+++ linux-2.6.4.noexec/arch/x86_64/ia32/ia32_binfmt.c	2004-03-23 21:18:35.000000000 +0100
@@ -272,8 +272,8 @@ do {							\
 #define load_elf_binary load_elf32_binary
 
 #define ELF_PLAT_INIT(r, load_addr)	elf32_init(r)
-#define setup_arg_pages(bprm)		ia32_setup_arg_pages(bprm)
-int ia32_setup_arg_pages(struct linux_binprm *bprm);
+#define setup_arg_pages(bprm, exec_stack)	ia32_setup_arg_pages(bprm, exec_stack)
+int ia32_setup_arg_pages(struct linux_binprm *bprm, int executable_stack);
 
 #undef start_thread
 #define start_thread(regs,new_rip,new_rsp) do { \
@@ -325,7 +325,7 @@ static void elf32_init(struct pt_regs *r
 	me->thread.es = __USER_DS;
 }
 
-int setup_arg_pages(struct linux_binprm *bprm)
+int setup_arg_pages(struct linux_binprm *bprm, int executable_stack)
 {
 	unsigned long stack_base;
 	struct vm_area_struct *mpnt;
@@ -354,7 +354,12 @@ int setup_arg_pages(struct linux_binprm 
 		mpnt->vm_mm = mm;
 		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
 		mpnt->vm_end = IA32_STACK_TOP;
-		mpnt->vm_flags = vm_stack_flags32; 
+		if (executable_stack > 0)
+			mpnt->vm_flags = vm_stack_flags32 |  VM_EXEC;
+		else if (executable_stack < 0)
+			mpnt->vm_flags = vm_stack_flags32 & ~VM_EXEC;
+		else
+			mpnt->vm_flags = vm_stack_flags32;
  		mpnt->vm_page_prot = (mpnt->vm_flags & VM_EXEC) ? 
  			PAGE_COPY_EXEC : PAGE_COPY;
 		mpnt->vm_ops = NULL;
@@ -373,7 +378,7 @@ int setup_arg_pages(struct linux_binprm 
 		struct page *page = bprm->page[i];
 		if (page) {
 			bprm->page[i] = NULL;
-			put_dirty_page(current,page,stack_base,PAGE_COPY_EXEC, mpnt);
+			put_dirty_page(current,page,stack_base,mpnt->vm_page_prot,mpnt);
 		}
 		stack_base += PAGE_SIZE;
 	}
diff -uNrp linux-2.6.4.EXSTACK/fs/binfmt_aout.c linux-2.6.4.noexec/fs/binfmt_aout.c
--- linux-2.6.4.EXSTACK/fs/binfmt_aout.c	2004-03-23 22:42:31.000000000 +0100
+++ linux-2.6.4.noexec/fs/binfmt_aout.c	2004-03-23 21:18:35.000000000 +0100
@@ -413,7 +413,7 @@ beyond_if:
 
 	set_brk(current->mm->start_brk, current->mm->brk);
 
-	retval = setup_arg_pages(bprm); 
+	retval = setup_arg_pages(bprm, 0); 
 	if (retval < 0) { 
 		/* Someone check-me: is this error path enough? */ 
 		send_sig(SIGKILL, current, 0); 
diff -uNrp linux-2.6.4.EXSTACK/fs/binfmt_elf.c linux-2.6.4.noexec/fs/binfmt_elf.c
--- linux-2.6.4.EXSTACK/fs/binfmt_elf.c	2004-03-23 22:42:31.000000000 +0100
+++ linux-2.6.4.noexec/fs/binfmt_elf.c	2004-03-23 21:18:35.000000000 +0100
@@ -476,6 +476,7 @@ static int load_elf_binary(struct linux_
   	struct exec interp_ex;
 	char passed_fileno[6];
 	struct files_struct *files;
+	int executable_stack = 0; 	/* Don't touch */
 	
 	/* Get the exec-header */
 	elf_ex = *((struct elfhdr *) bprm->buf);
@@ -599,6 +600,15 @@ static int load_elf_binary(struct linux_
 		elf_ppnt++;
 	}
 
+	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 =  1;
+			else
+				executable_stack = -1;
+		}
+
 	/* Some simple consistency checks for the interpreter */
 	if (elf_interpreter) {
 		interpreter_type = INTERPRETER_ELF | INTERPRETER_AOUT;
@@ -674,7 +684,7 @@ static int load_elf_binary(struct linux_
 	   change some of these later */
 	current->mm->rss = 0;
 	current->mm->free_area_cache = TASK_UNMAPPED_BASE;
-	retval = setup_arg_pages(bprm);
+	retval = setup_arg_pages(bprm, executable_stack);
 	if (retval < 0) {
 		send_sig(SIGKILL, current, 0);
 		goto out_free_dentry;
diff -uNrp linux-2.6.4.EXSTACK/fs/binfmt_som.c linux-2.6.4.noexec/fs/binfmt_som.c
--- linux-2.6.4.EXSTACK/fs/binfmt_som.c	2004-03-23 22:42:31.000000000 +0100
+++ linux-2.6.4.noexec/fs/binfmt_som.c	2004-03-23 21:18:35.000000000 +0100
@@ -254,7 +254,7 @@ load_som_binary(struct linux_binprm * bp
 
 	set_binfmt(&som_format);
 	compute_creds(bprm);
-	setup_arg_pages(bprm);
+	setup_arg_pages(bprm, 0);
 
 	create_som_tables(bprm);
 
diff -uNrp linux-2.6.4.EXSTACK/fs/exec.c linux-2.6.4.noexec/fs/exec.c
--- linux-2.6.4.EXSTACK/fs/exec.c	2004-03-23 22:42:31.000000000 +0100
+++ linux-2.6.4.noexec/fs/exec.c	2004-03-23 21:18:35.000000000 +0100
@@ -342,7 +342,7 @@ out_sig:
 	return;
 }
 
-int setup_arg_pages(struct linux_binprm *bprm)
+int setup_arg_pages(struct linux_binprm *bprm, int executable_stack)
 {
 	unsigned long stack_base;
 	struct vm_area_struct *mpnt;
@@ -425,8 +425,13 @@ int setup_arg_pages(struct linux_binprm 
 		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
 		mpnt->vm_end = STACK_TOP;
 #endif
-		mpnt->vm_page_prot = protection_map[VM_STACK_FLAGS & 0x7];
-		mpnt->vm_flags = VM_STACK_FLAGS;
+		if (executable_stack > 0)
+			mpnt->vm_flags = VM_STACK_FLAGS |  VM_EXEC;
+		else if (executable_stack < 0)
+			mpnt->vm_flags = VM_STACK_FLAGS & ~VM_EXEC;
+		else
+			mpnt->vm_flags = VM_STACK_FLAGS;
+		mpnt->vm_page_prot = protection_map[mpnt->vm_flags & 0x7];
 		mpnt->vm_ops = NULL;
 		mpnt->vm_pgoff = mpnt->vm_start >> PAGE_SHIFT;
 		mpnt->vm_file = NULL;
diff -uNrp linux-2.6.4.EXSTACK/include/linux/binfmts.h linux-2.6.4.noexec/include/linux/binfmts.h
--- linux-2.6.4.EXSTACK/include/linux/binfmts.h	2004-03-23 22:42:31.000000000 +0100
+++ linux-2.6.4.noexec/include/linux/binfmts.h	2004-03-23 21:18:35.000000000 +0100
@@ -58,7 +58,7 @@ extern int prepare_binprm(struct linux_b
 extern void remove_arg_zero(struct linux_binprm *);
 extern int search_binary_handler(struct linux_binprm *,struct pt_regs *);
 extern int flush_old_exec(struct linux_binprm * bprm);
-extern int setup_arg_pages(struct linux_binprm * bprm);
+extern int setup_arg_pages(struct linux_binprm * bprm, int executable_stack);
 extern int copy_strings(int argc,char __user * __user * argv,struct linux_binprm *bprm); 
 extern int copy_strings_kernel(int argc,char ** argv,struct linux_binprm *bprm);
 extern void compute_creds(struct linux_binprm *binprm);
diff -uNrp linux-2.6.4.EXSTACK/include/linux/elf.h linux-2.6.4.noexec/include/linux/elf.h
--- linux-2.6.4.EXSTACK/include/linux/elf.h	2004-03-23 22:42:31.000000000 +0100
+++ linux-2.6.4.noexec/include/linux/elf.h	2004-03-23 21:18:35.000000000 +0100
@@ -35,6 +35,8 @@ typedef __s64	Elf64_Sxword;
 #define PT_HIPROC  0x7fffffff
 #define PT_GNU_EH_FRAME		0x6474e550
 
+#define PT_GNU_STACK	(PT_LOOS + 0x474e551)
+
 /* These constants define the different elf file types */
 #define ET_NONE   0
 #define ET_REL    1

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Non-Exec stack patches

[Ingo Molnar]

On Wed, 24 Mar 2004, Kurt Garloff wrote:

> find attached a patch to parse the elf binaries for a PT_GNU_STACK
> section to set the stack non-executable if possible.
> Most parts have been shamelessly stolen from Ingo Molnar's more
> ambitious stackshield
> http://people.redhat.com/mingo/exec-shield/exec-shield-2.6.4-C9
> 
> The toolchain has meainwhile support for marking the binaries with a
> PT_GNU_STACK section with ot without x bit as needed.
> 
> If no such section is found, we leave the stack to whatever the arch
> defaults to. If there is one, we explicitly disabled the VM_EXEC bit if
> no x bit is found, otherwise explicitly enable.
> 
> I believe this part should be merged into official mainstream kernels.
> Ingo, what do you think?

agreed, and the patch looks good to me.

	Ingo

Re: Non-Exec stack patches

[Andrew Morton]

Kurt Garloff <garloff@suse.de> wrote:
>
> find attached a patch to parse the elf binaries for a PT_GNU_STACK
> section to set the stack non-executable if possible.

This patch propagates the PT_GNU_STACK setting into the vma flags, allowing
the architecture to set the stack permissions non-executable if the
architecture chooses to do that, yes?

Which architectures are currently making their pre-page execute permissions
depend upon VM_EXEC?  Would additional arch patches be needed for this?

This may not get past Linus of course.  It doesn't even get past me with
that magical undocumented -1/0/+1 value of the executable_stack argument. 
Please replace that with a proper, commented, #defined-or-enumerated value,
thanks.

Re: Non-Exec stack patches

[Kurt Garloff]

[-- Attachment #1.1: Type: text/plain, Size: 1337 bytes --]

On Tue, Mar 23, 2004 at 03:49:37PM -0800, Andrew Morton wrote:
> This patch propagates the PT_GNU_STACK setting into the vma flags, allowing
> the architecture to set the stack permissions non-executable if the
> architecture chooses to do that, yes?

Right.

> Which architectures are currently making their pre-page execute permissions
> depend upon VM_EXEC?  Would additional arch patches be needed for this?

It works on AMD64 (not ia32e), both for 64bit and 32bit binaries.
I have not yet tested other archs.

If the values in the protection_map are different depending on bit 2,
the patch will be effecitve. (OK, the CPU/MMU needs to honour the
setting of course.) Most likely, the values for 
protection_map[7] is PAGE_COPY_EXEC and of protection_map[3] is
PAGE_COPY.

> This may not get past Linus of course.  It doesn't even get past me with
> that magical undocumented -1/0/+1 value of the executable_stack argument. 
> Please replace that with a proper, commented, #defined-or-enumerated value,

As you wish, master.
Slightly edited and untested patch attached.

Regards,
-- 
Kurt Garloff                   <kurt@garloff.de>             [Koeln, DE]
Physics:Plasma modeling <garloff@plasimo.phys.tue.nl> [TU Eindhoven, NL]
Linux: SUSE Labs (Head)        <garloff@suse.de>    [SUSE Nuernberg, DE]

[-- Attachment #1.2: noexec_stack5 --]
[-- Type: text/plain, Size: 13740 bytes --]

diff -uNrp linux-2.6.4.EXSTACK/arch/ia64/ia32/binfmt_elf32.c linux-2.6.4.noexec2/arch/ia64/ia32/binfmt_elf32.c
--- linux-2.6.4.EXSTACK/arch/ia64/ia32/binfmt_elf32.c	2004-03-23 21:18:35.000000000 +0100
+++ linux-2.6.4.noexec2/arch/ia64/ia32/binfmt_elf32.c	2004-03-24 01:07:35.000000000 +0100
@@ -35,7 +35,7 @@ extern void ia64_elf32_init (struct pt_r
 
 static void elf32_set_personality (void);
 
-#define setup_arg_pages(bprm)		ia32_setup_arg_pages(bprm)
+#define setup_arg_pages(bprm,exec)		ia32_setup_arg_pages(bprm,exec)
 #define elf_map				elf32_map
 
 #undef SET_PERSONALITY
@@ -152,7 +152,7 @@ ia64_elf32_init (struct pt_regs *regs)
 }
 
 int
-ia32_setup_arg_pages (struct linux_binprm *bprm)
+ia32_setup_arg_pages (struct linux_binprm *bprm, int executable_stack)
 {
 	unsigned long stack_base;
 	struct vm_area_struct *mpnt;
@@ -181,8 +181,14 @@ ia32_setup_arg_pages (struct linux_binpr
 		mpnt->vm_mm = current->mm;
 		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
 		mpnt->vm_end = IA32_STACK_TOP;
-		mpnt->vm_page_prot = PAGE_COPY;
-		mpnt->vm_flags = VM_STACK_FLAGS;
+		if (executable_stack == EXSTACK_ENABLE_X)
+			mpnt->vm_flags = VM_STACK_FLAGS |  VM_EXEC;
+		else if (executable_stack == EXSTACK_DISABLE_X)
+			mpnt->vm_flags = VM_STACK_FLAGS & ~VM_EXEC;
+		else
+			mpnt->vm_flags = VM_STACK_FLAGS;
+		mpnt->vm_page_prot = (mpnt->vm_flags & VM_EXEC)?
+					PAGE_COPY_EXEC: PAGE_COPY;
 		mpnt->vm_ops = NULL;
 		mpnt->vm_pgoff = mpnt->vm_start >> PAGE_SHIFT;
 		mpnt->vm_file = NULL;
@@ -197,7 +203,7 @@ ia32_setup_arg_pages (struct linux_binpr
 		struct page *page = bprm->page[i];
 		if (page) {
 			bprm->page[i] = NULL;
-			put_dirty_page(current, page, stack_base, PAGE_COPY, mpnt);
+			put_dirty_page(current, page, stack_base, mpnt->vm_page_prot, mpnt);
 		}
 		stack_base += PAGE_SIZE;
 	}
diff -uNrp linux-2.6.4.EXSTACK/arch/ia64/ia32/ia32priv.h linux-2.6.4.noexec2/arch/ia64/ia32/ia32priv.h
--- linux-2.6.4.EXSTACK/arch/ia64/ia32/ia32priv.h	2004-03-11 03:55:51.000000000 +0100
+++ linux-2.6.4.noexec2/arch/ia64/ia32/ia32priv.h	2004-03-23 22:56:20.000000000 +0100
@@ -494,7 +494,7 @@ struct ia32_user_desc {
 struct linux_binprm;
 
 extern void ia32_init_addr_space (struct pt_regs *regs);
-extern int ia32_setup_arg_pages (struct linux_binprm *bprm);
+extern int ia32_setup_arg_pages (struct linux_binprm *bprm, int exec_stack);
 extern unsigned long ia32_do_mmap (struct file *, unsigned long, unsigned long, int, int, loff_t);
 extern void ia32_load_segment_descriptors (struct task_struct *task);
 
diff -uNrp linux-2.6.4.EXSTACK/arch/mips/kernel/irixelf.c linux-2.6.4.noexec2/arch/mips/kernel/irixelf.c
--- linux-2.6.4.EXSTACK/arch/mips/kernel/irixelf.c	2004-03-11 03:56:03.000000000 +0100
+++ linux-2.6.4.noexec2/arch/mips/kernel/irixelf.c	2004-03-24 01:08:11.000000000 +0100
@@ -688,7 +688,7 @@ static int load_irix_binary(struct linux
 	 * change some of these later.
 	 */
 	current->mm->rss = 0;
-	setup_arg_pages(bprm);
+	setup_arg_pages(bprm, EXSTACK_DEFAULT);
 	current->mm->start_stack = bprm->p;
 
 	/* At this point, we assume that the image should be loaded at
diff -uNrp linux-2.6.4.EXSTACK/arch/s390/kernel/binfmt_elf32.c linux-2.6.4.noexec2/arch/s390/kernel/binfmt_elf32.c
--- linux-2.6.4.EXSTACK/arch/s390/kernel/binfmt_elf32.c	2004-03-11 03:55:44.000000000 +0100
+++ linux-2.6.4.noexec2/arch/s390/kernel/binfmt_elf32.c	2004-03-23 22:51:25.000000000 +0100
@@ -114,7 +114,7 @@ typedef s390_regs32 elf_gregset_t;
 #include <linux/binfmts.h>
 #include <linux/compat.h>
 
-int setup_arg_pages32(struct linux_binprm *bprm);
+int setup_arg_pages32(struct linux_binprm *bprm, int executable_stack);
 
 #define elf_prstatus elf_prstatus32
 struct elf_prstatus32
@@ -165,7 +165,7 @@ struct elf_prpsinfo32
 
 #undef start_thread
 #define start_thread                    start_thread31 
-#define setup_arg_pages(bprm)           setup_arg_pages32(bprm)
+#define setup_arg_pages(bprm, exec)     setup_arg_pages32(bprm, exec)
 #define elf_map				elf_map32
 
 MODULE_DESCRIPTION("Binary format loader for compatibility with 32bit Linux for S390 binaries,"
diff -uNrp linux-2.6.4.EXSTACK/arch/s390/kernel/compat_exec.c linux-2.6.4.noexec2/arch/s390/kernel/compat_exec.c
--- linux-2.6.4.EXSTACK/arch/s390/kernel/compat_exec.c	2004-03-23 21:18:35.000000000 +0100
+++ linux-2.6.4.noexec2/arch/s390/kernel/compat_exec.c	2004-03-23 22:58:15.000000000 +0100
@@ -37,7 +37,7 @@
 #undef STACK_TOP
 #define STACK_TOP TASK31_SIZE
 
-int setup_arg_pages32(struct linux_binprm *bprm)
+int setup_arg_pages32(struct linux_binprm *bprm, int executable_stack)
 {
 	unsigned long stack_base;
 	struct vm_area_struct *mpnt;
@@ -66,6 +66,7 @@ int setup_arg_pages32(struct linux_binpr
 		mpnt->vm_mm = mm;
 		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
 		mpnt->vm_end = STACK_TOP;
+		/* executable stack setting would be applied here */
 		mpnt->vm_page_prot = PAGE_COPY;
 		mpnt->vm_flags = VM_STACK_FLAGS;
 		mpnt->vm_ops = NULL;
diff -uNrp linux-2.6.4.EXSTACK/arch/sparc64/kernel/binfmt_aout32.c linux-2.6.4.noexec2/arch/sparc64/kernel/binfmt_aout32.c
--- linux-2.6.4.EXSTACK/arch/sparc64/kernel/binfmt_aout32.c	2004-03-11 03:55:27.000000000 +0100
+++ linux-2.6.4.noexec2/arch/sparc64/kernel/binfmt_aout32.c	2004-03-24 01:09:55.000000000 +0100
@@ -310,7 +310,7 @@ beyond_if:
 	orig_thr_flags = current_thread_info()->flags;
 	current_thread_info()->flags |= _TIF_32BIT;
 
-	retval = setup_arg_pages(bprm);
+	retval = setup_arg_pages(bprm, EXSTACK_DEFAULT);
 	if (retval < 0) { 
 		current_thread_info()->flags = orig_thr_flags;
 
diff -uNrp linux-2.6.4.EXSTACK/arch/x86_64/ia32/ia32_aout.c linux-2.6.4.noexec2/arch/x86_64/ia32/ia32_aout.c
--- linux-2.6.4.EXSTACK/arch/x86_64/ia32/ia32_aout.c	2004-03-11 03:55:54.000000000 +0100
+++ linux-2.6.4.noexec2/arch/x86_64/ia32/ia32_aout.c	2004-03-24 01:13:31.000000000 +0100
@@ -35,7 +35,7 @@
 #undef WARN_OLD
 #undef CORE_DUMP /* probably broken */
 
-extern int ia32_setup_arg_pages(struct linux_binprm *bprm);
+extern int ia32_setup_arg_pages(struct linux_binprm *bprm, int exec_stack);
 
 static int load_aout_binary(struct linux_binprm *, struct pt_regs * regs);
 static int load_aout_library(struct file*);
@@ -395,7 +395,7 @@ beyond_if:
 
 	set_brk(current->mm->start_brk, current->mm->brk);
 
-	retval = ia32_setup_arg_pages(bprm); 
+	retval = ia32_setup_arg_pages(bprm, EXSTACK_DEFAULT);
 	if (retval < 0) { 
 		/* Someone check-me: is this error path enough? */ 
 		send_sig(SIGKILL, current, 0); 
diff -uNrp linux-2.6.4.EXSTACK/arch/x86_64/ia32/ia32_binfmt.c linux-2.6.4.noexec2/arch/x86_64/ia32/ia32_binfmt.c
--- linux-2.6.4.EXSTACK/arch/x86_64/ia32/ia32_binfmt.c	2004-03-23 22:42:31.000000000 +0100
+++ linux-2.6.4.noexec2/arch/x86_64/ia32/ia32_binfmt.c	2004-03-24 01:11:21.000000000 +0100
@@ -272,8 +272,8 @@ do {							\
 #define load_elf_binary load_elf32_binary
 
 #define ELF_PLAT_INIT(r, load_addr)	elf32_init(r)
-#define setup_arg_pages(bprm)		ia32_setup_arg_pages(bprm)
-int ia32_setup_arg_pages(struct linux_binprm *bprm);
+#define setup_arg_pages(bprm, exec_stack)	ia32_setup_arg_pages(bprm, exec_stack)
+int ia32_setup_arg_pages(struct linux_binprm *bprm, int executable_stack);
 
 #undef start_thread
 #define start_thread(regs,new_rip,new_rsp) do { \
@@ -325,7 +325,7 @@ static void elf32_init(struct pt_regs *r
 	me->thread.es = __USER_DS;
 }
 
-int setup_arg_pages(struct linux_binprm *bprm)
+int setup_arg_pages(struct linux_binprm *bprm, int executable_stack)
 {
 	unsigned long stack_base;
 	struct vm_area_struct *mpnt;
@@ -354,7 +354,12 @@ int setup_arg_pages(struct linux_binprm 
 		mpnt->vm_mm = mm;
 		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
 		mpnt->vm_end = IA32_STACK_TOP;
-		mpnt->vm_flags = vm_stack_flags32; 
+		if (executable_stack == EXSTACK_ENABLE_X)
+			mpnt->vm_flags = vm_stack_flags32 |  VM_EXEC;
+		else if (executable_stack == EXSTACK_DISABLE_X)
+			mpnt->vm_flags = vm_stack_flags32 & ~VM_EXEC;
+		else
+			mpnt->vm_flags = vm_stack_flags32;
  		mpnt->vm_page_prot = (mpnt->vm_flags & VM_EXEC) ? 
  			PAGE_COPY_EXEC : PAGE_COPY;
 		mpnt->vm_ops = NULL;
@@ -373,7 +378,7 @@ int setup_arg_pages(struct linux_binprm 
 		struct page *page = bprm->page[i];
 		if (page) {
 			bprm->page[i] = NULL;
-			put_dirty_page(current,page,stack_base,PAGE_COPY_EXEC, mpnt);
+			put_dirty_page(current,page,stack_base,mpnt->vm_page_prot,mpnt);
 		}
 		stack_base += PAGE_SIZE;
 	}
diff -uNrp linux-2.6.4.EXSTACK/fs/binfmt_aout.c linux-2.6.4.noexec2/fs/binfmt_aout.c
--- linux-2.6.4.EXSTACK/fs/binfmt_aout.c	2004-03-23 22:42:31.000000000 +0100
+++ linux-2.6.4.noexec2/fs/binfmt_aout.c	2004-03-24 01:05:35.000000000 +0100
@@ -413,7 +413,7 @@ beyond_if:
 
 	set_brk(current->mm->start_brk, current->mm->brk);
 
-	retval = setup_arg_pages(bprm); 
+	retval = setup_arg_pages(bprm, EXSTACK_DEFAULT); 
 	if (retval < 0) { 
 		/* Someone check-me: is this error path enough? */ 
 		send_sig(SIGKILL, current, 0); 
diff -uNrp linux-2.6.4.EXSTACK/fs/binfmt_elf.c linux-2.6.4.noexec2/fs/binfmt_elf.c
--- linux-2.6.4.EXSTACK/fs/binfmt_elf.c	2004-03-23 22:42:31.000000000 +0100
+++ linux-2.6.4.noexec2/fs/binfmt_elf.c	2004-03-24 01:03:32.000000000 +0100
@@ -476,6 +476,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;
 	
 	/* Get the exec-header */
 	elf_ex = *((struct elfhdr *) bprm->buf);
@@ -599,6 +600,15 @@ static int load_elf_binary(struct linux_
 		elf_ppnt++;
 	}
 
+	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;
+		}
+
 	/* Some simple consistency checks for the interpreter */
 	if (elf_interpreter) {
 		interpreter_type = INTERPRETER_ELF | INTERPRETER_AOUT;
@@ -674,7 +684,7 @@ static int load_elf_binary(struct linux_
 	   change some of these later */
 	current->mm->rss = 0;
 	current->mm->free_area_cache = TASK_UNMAPPED_BASE;
-	retval = setup_arg_pages(bprm);
+	retval = setup_arg_pages(bprm, executable_stack);
 	if (retval < 0) {
 		send_sig(SIGKILL, current, 0);
 		goto out_free_dentry;
diff -uNrp linux-2.6.4.EXSTACK/fs/binfmt_som.c linux-2.6.4.noexec2/fs/binfmt_som.c
--- linux-2.6.4.EXSTACK/fs/binfmt_som.c	2004-03-23 22:42:31.000000000 +0100
+++ linux-2.6.4.noexec2/fs/binfmt_som.c	2004-03-24 01:05:55.000000000 +0100
@@ -254,7 +254,7 @@ load_som_binary(struct linux_binprm * bp
 
 	set_binfmt(&som_format);
 	compute_creds(bprm);
-	setup_arg_pages(bprm);
+	setup_arg_pages(bprm, EXSTACK_DEFAULT);
 
 	create_som_tables(bprm);
 
diff -uNrp linux-2.6.4.EXSTACK/fs/exec.c linux-2.6.4.noexec2/fs/exec.c
--- linux-2.6.4.EXSTACK/fs/exec.c	2004-03-23 22:42:31.000000000 +0100
+++ linux-2.6.4.noexec2/fs/exec.c	2004-03-24 01:18:14.000000000 +0100
@@ -342,7 +342,7 @@ out_sig:
 	return;
 }
 
-int setup_arg_pages(struct linux_binprm *bprm)
+int setup_arg_pages(struct linux_binprm *bprm, int executable_stack)
 {
 	unsigned long stack_base;
 	struct vm_area_struct *mpnt;
@@ -425,8 +425,16 @@ int setup_arg_pages(struct linux_binprm 
 		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
 		mpnt->vm_end = STACK_TOP;
 #endif
-		mpnt->vm_page_prot = protection_map[VM_STACK_FLAGS & 0x7];
-		mpnt->vm_flags = VM_STACK_FLAGS;
+		/* Adjust stack execute permissions; explicitly enable 
+		 * for EXSTACK_ENABLE_X, disable for EXSTACK_DISABLE_X
+		 * and leave alone (arch default) otherwise. */
+		if (unlikely(executable_stack == EXSTACK_ENABLE_X))
+			mpnt->vm_flags = VM_STACK_FLAGS |  VM_EXEC;
+		else if (executable_stack == EXSTACK_DISABLE_X)
+			mpnt->vm_flags = VM_STACK_FLAGS & ~VM_EXEC;
+		else
+			mpnt->vm_flags = VM_STACK_FLAGS;
+		mpnt->vm_page_prot = protection_map[mpnt->vm_flags & 0x7];
 		mpnt->vm_ops = NULL;
 		mpnt->vm_pgoff = mpnt->vm_start >> PAGE_SHIFT;
 		mpnt->vm_file = NULL;
diff -uNrp linux-2.6.4.EXSTACK/include/linux/binfmts.h linux-2.6.4.noexec2/include/linux/binfmts.h
--- linux-2.6.4.EXSTACK/include/linux/binfmts.h	2004-03-23 22:42:31.000000000 +0100
+++ linux-2.6.4.noexec2/include/linux/binfmts.h	2004-03-24 01:15:09.000000000 +0100
@@ -58,7 +58,10 @@ extern int prepare_binprm(struct linux_b
 extern void remove_arg_zero(struct linux_binprm *);
 extern int search_binary_handler(struct linux_binprm *,struct pt_regs *);
 extern int flush_old_exec(struct linux_binprm * bprm);
-extern int setup_arg_pages(struct linux_binprm * bprm);
+#define EXSTACK_DEFAULT   0
+#define EXSTACK_DISABLE_X 1
+#define EXSTACK_ENABLE_X  2
+extern int setup_arg_pages(struct linux_binprm * bprm, int executable_stack);
 extern int copy_strings(int argc,char __user * __user * argv,struct linux_binprm *bprm); 
 extern int copy_strings_kernel(int argc,char ** argv,struct linux_binprm *bprm);
 extern void compute_creds(struct linux_binprm *binprm);
diff -uNrp linux-2.6.4.EXSTACK/include/linux/elf.h linux-2.6.4.noexec2/include/linux/elf.h
--- linux-2.6.4.EXSTACK/include/linux/elf.h	2004-03-23 22:42:31.000000000 +0100
+++ linux-2.6.4.noexec2/include/linux/elf.h	2004-03-23 21:18:35.000000000 +0100
@@ -35,6 +35,8 @@ typedef __s64	Elf64_Sxword;
 #define PT_HIPROC  0x7fffffff
 #define PT_GNU_EH_FRAME		0x6474e550
 
+#define PT_GNU_STACK	(PT_LOOS + 0x474e551)
+
 /* These constants define the different elf file types */
 #define ET_NONE   0
 #define ET_REL    1

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Non-Exec stack patches

[David Mosberger]

>>>>> On Wed, 24 Mar 2004 01:21:49 +0100, Kurt Garloff <garloff@suse.de> said:

  >> Which architectures are currently making their pre-page execute
  >> permissions depend upon VM_EXEC?  Would additional arch patches
  >> be needed for this?

  Kurt> It works on AMD64 (not ia32e), both for 64bit and 32bit
  Kurt> binaries.  I have not yet tested other archs.

IA64 Linux had non-executable stack/data before PT_GNU_STACK was invented.
It's keyed off a ELF header flags bit:

/* Least-significant four bits of ELF header's e_flags are
   OS-specific.  The bits are interpreted as follows by Linux: */
#define EF_IA_64_LINUX_EXECUTABLE_STACK 0x1

I guess I never quiet understood why an entire program header is
needed for this, but that's just me.

	--david

Re: Non-Exec stack patches

[Ulrich Drepper]

David Mosberger wrote:

> I guess I never quiet understood why an entire program header is
> needed for this, but that's just me.

This just means you haven't looked at the problem.

First, the ELF bits are limited and very crowded on some archs.  There
is no central assignment so conflicts will happen.

And one single bit does not cut it.  If you'd take a look, the
PT_GNU_STACK entry's permissions field specifies what permissions the
stack must have, not the presence of the field.  So at least two bits
are needed which only adds to the problems of finding appropriate bits.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖

Re: Non-Exec stack patches

[David Mosberger]

>>>>> On Tue, 23 Mar 2004 17:20:12 -0800, Ulrich Drepper <drepper@redhat.com> said:

  Uli> David Mosberger wrote:
  >> I guess I never quiet understood why an entire program header is
  >> needed for this, but that's just me.

  Uli> First, the ELF bits are limited and very crowded on some archs.  There
  Uli> is no central assignment so conflicts will happen.

Fair enough, but I don't see why this should imply that platforms that
already do have support for no-exec data/stack should be forced into
using PT_GNU_STACK.  Just for uniformity's sake?  Or is there a real
benefit?

  Uli> And one single bit does not cut it.  If you'd take a look, the
  Uli> PT_GNU_STACK entry's permissions field specifies what permissions the
  Uli> stack must have, not the presence of the field.  So at least two bits
  Uli> are needed which only adds to the problems of finding appropriate bits.

What stack protections other than RW- and RWX are useful?

	--david

Re: Non-Exec stack patches

[Ulrich Drepper]

David Mosberger wrote:

> What stack protections other than RW- and RWX are useful?

It's not about "what protections".  The three currently recognized
states are PT_GNU_STACK not present, rwx, rw-.  Ingo's code documents
this.  For those who need more, I'll have a paper coming up for a
conference in Toronto in April.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖

Re: Non-Exec stack patches

[David Mosberger]

>>>>> On Tue, 23 Mar 2004 18:01:14 -0800, Ulrich Drepper <drepper@redhat.com> said:

  Uli> David Mosberger wrote:
  >> What stack protections other than RW- and RWX are useful?

  Uli> It's not about "what protections".  The three currently
  Uli> recognized states are PT_GNU_STACK not present, rwx, rw-.
  Uli> Ingo's code documents this.  For those who need more, I'll have
  Uli> a paper coming up for a conference in Toronto in April.

I'd find the PT_GNU_STACK approach much more appealing if it actually
was used to get rid of the stack-special case in the kernel
altogether.  Certainly it seems like it could be used to get rid of
STACK_TOP (just use the segment's p_vaddr).  That would still leave
the TASK_UNMAPPED_BASE special-case, but at least it would be a step
in the right direction.  Also, in the case of ia64, p_memsz could be
used to give user-level control over whether they want the
register/memory stacks to grow towards each other or apart from each
other.

	--david

Re: Non-Exec stack patches

[Jakub Jelinek]

On Tue, Mar 23, 2004 at 05:41:49PM -0800, David Mosberger wrote:
>   Uli> First, the ELF bits are limited and very crowded on some archs.  There
>   Uli> is no central assignment so conflicts will happen.
> 
> Fair enough, but I don't see why this should imply that platforms that
> already do have support for no-exec data/stack should be forced into
> using PT_GNU_STACK.  Just for uniformity's sake?  Or is there a real
> benefit?

Note that PT_GNU_STACK program header is not generated on EM_IA_64 and
EM_PPC64 ATM, because initially I thought it shouldn't be needed
(these 2 arches are the only ones which don't need executable stack
for GCC trampolines).
But I think we should change the toolchain and generate it on IA64 and PPC64
as well (only GCC would need changing, emitting
.section .note.GNU-stack,"",@progbits
at the end of every compile unit, ld takes care of the rest) exactly for
uniformity's sake and because you get ld.so handling free.
GLIBC dynamic linker will take care of making the stack executable if
say a binary which doesn't need executable stack depends on a shared library
which needs executable stack or even dlopens a library which needs
executable stack.

>   Uli> And one single bit does not cut it.  If you'd take a look, the
>   Uli> PT_GNU_STACK entry's permissions field specifies what permissions the
>   Uli> stack must have, not the presence of the field.  So at least two bits
>   Uli> are needed which only adds to the problems of finding appropriate bits.
> 
> What stack protections other than RW- and RWX are useful?

At least RW- (read-write but not executable stack), RWX (rw and executable
stack) and no PT_GNU_STACK segment (unknown, not marked binary), but it is
certainly extendable.

	Jakub

Re: Non-Exec stack patches

[David Mosberger]

>>>>> On Wed, 24 Mar 2004 02:00:20 -0500, Jakub Jelinek <jakub@redhat.com> said:

  Jakub> But I think we should change the toolchain and generate it on
  Jakub> IA64 and PPC64 as well (only GCC would need changing,
  Jakub> emitting .section .note.GNU-stack,"",@progbits at the end of
  Jakub> every compile unit, ld takes care of the rest) exactly for
  Jakub> uniformity's sake and because you get ld.so handling free.

I'm not following you on the "get ld.so handling free" part.  How is
that handling free?

  Jakub> GLIBC dynamic linker will take care of making the stack
  Jakub> executable if say a binary which doesn't need executable
  Jakub> stack depends on a shared library which needs executable
  Jakub> stack or even dlopens a library which needs executable stack.

Actually, that's something that worries me.  Somebody just needs to
succeed in loading any shared object with the right PT_GNU_STACK
header and then the entire program will be exposed to the risk of a
writable stack.  On ia64, I just don't see any need to ever implicitly
turn on execute-permission on the stack, so why allow this extra
backdoor?

	--david

Re: Non-Exec stack patches

[Jakub Jelinek]

On Tue, Mar 23, 2004 at 11:16:36PM -0800, David Mosberger wrote:
> I'm not following you on the "get ld.so handling free" part.  How is
> that handling free?

What I meant is that it is already written and tested.

> Actually, that's something that worries me.  Somebody just needs to
> succeed in loading any shared object with the right PT_GNU_STACK
> header and then the entire program will be exposed to the risk of a
> writable stack.  On ia64, I just don't see any need to ever implicitly
> turn on execute-permission on the stack, so why allow this extra
> backdoor?

What kind of backdoor is it?  If you dlopen untrusted shared libraries
into your program you have far bigger problem than executable
stack (you can execute any code it wants in its constructors).

If there is a shared library which needs executable stack for its use
(on !IA64 !PPC64 this is e.g. any library which takes address of
a nested function and passes it to some other function and/or stores
it into some variable which cannot be optimized out, on IA64 or PPC64
this is of course much rarer, but it is still possible some language
interpreter or something builds code on the fly on the stack).

	Jakub

Re: Non-Exec stack patches

[David Mosberger]

>>>>> On Wed, 24 Mar 2004 02:28:40 -0500, Jakub Jelinek <jakub@redhat.com> said:

  Jakub> On Tue, Mar 23, 2004 at 11:16:36PM -0800, David Mosberger
  Jakub> wrote:

  >> I'm not following you on the "get ld.so handling free" part.  How
  >> is that handling free?

  Jakub> What I meant is that it is already written and tested.

OK.  The ELF flags bit is even more free, then... ;-)

  >> Actually, that's something that worries me.  Somebody just needs
  >> to succeed in loading any shared object with the right
  >> PT_GNU_STACK header and then the entire program will be exposed
  >> to the risk of a writable stack.  On ia64, I just don't see any
  >> need to ever implicitly turn on execute-permission on the stack,
  >> so why allow this extra backdoor?

  Jakub> What kind of backdoor is it?  If you dlopen untrusted shared
  Jakub> libraries into your program you have far bigger problem than
  Jakub> executable stack (you can execute any code it wants in its
  Jakub> constructors).

Sure.  Theoretically, none of this matters at all (thanks to
mprotect()), so we're justs talking about raising the barrier.  With
PT_GNU_STACK, it's sufficient to tweak one bit in any dependent
library, whereas with the ELF flag bit, you need to tweak one bit in
the main executable.  Not a huge difference, I'll submit, but from an
admin perspective, I find it a lot easier to check the main program to
see if it has the "executable stack/data" bit set rather than all
dependent libraries.  Additionally, we could easily choose to drop
support for the ELF flag bit altogether.  The only program that I know
of that ever needed executable data was XFree86 and that was only due
to an oversight---and that has long been fixed.

  Jakub> If there is a shared library which needs executable stack for
  Jakub> its use (on !IA64 !PPC64 this is e.g. any library which takes
  Jakub> address of a nested function and passes it to some other
  Jakub> function and/or stores it into some variable which cannot be
  Jakub> optimized out, on IA64 or PPC64 this is of course much rarer,

For sufficiently small values of "much rarer", I agree. ;-)

  Jakub> but it is still possible some language interpreter or
  Jakub> something builds code on the fly on the stack).

That's why there is mprotect().

	--david

Re: Non-Exec stack patches

[John Reiser]

Jakub> but it is still possible some language interpreter or
Jakub> something builds code on the fly on the stack).

David> That's why there is mprotect().

But mprotect() costs enough (hundreds of cycles) to be a significant burden
in some cases.  Generating code to a stack region that is inherently
executable is inexpensive (even allowing for restrictive alignment and
avoiding I/D cache conflicts), is thread safe, is async-signal safe, and
takes less work than other alternatives.  Yes, the "black hats" do this;
so do the "white hats."  Please do not increase the minimum cost for
applications that want generate-and-execute on the stack at upredictable
high frequency.

-- 
John Reiser, jreiser@BitWagon.com

Re: Non-Exec stack patches

[David Mosberger]

>>>>> On Wed, 24 Mar 2004 08:29:24 -0800, John Reiser <jreiser@BitWagon.com> said:

  Jakub> but it is still possible some language interpreter or
  Jakub> something builds code on the fly on the stack).

  David> That's why there is mprotect().

  John> But mprotect() costs enough (hundreds of cycles) to be a
  John> significant burden in some cases.  Generating code to a stack
  John> region that is inherently executable is inexpensive (even
  John> allowing for restrictive alignment and avoiding I/D cache
  John> conflicts), is thread safe, is async-signal safe, and takes
  John> less work than other alternatives.  Yes, the "black hats" do
  John> this; so do the "white hats."  Please do not increase the
  John> minimum cost for applications that want generate-and-execute
  John> on the stack at upredictable high frequency.

Huh?  Only one mprotect() call is needed to make the entire stack
executable.

	--david

Re: Non-Exec stack patches

[Jakub Jelinek]

On Wed, Mar 24, 2004 at 09:12:30AM -0800, David Mosberger wrote:
>   David> That's why there is mprotect().
> 
>   John> But mprotect() costs enough (hundreds of cycles) to be a
>   John> significant burden in some cases.  Generating code to a stack
>   John> region that is inherently executable is inexpensive (even
>   John> allowing for restrictive alignment and avoiding I/D cache
>   John> conflicts), is thread safe, is async-signal safe, and takes
>   John> less work than other alternatives.  Yes, the "black hats" do
>   John> this; so do the "white hats."  Please do not increase the
>   John> minimum cost for applications that want generate-and-execute
>   John> on the stack at upredictable high frequency.
> 
> Huh?  Only one mprotect() call is needed to make the entire stack
> executable.

Nope.  Think about multithreaded apps.  Furthermore, getting the exact
extents of the particular stack is difficult to find for applications,
but e.g. the threading library has to know such things.

	Jakub

Re: Non-Exec stack patches

[David Mosberger]

>>>>> On Wed, 24 Mar 2004 12:24:54 -0500, Jakub Jelinek <jakub@redhat.com> said:

  >> Huh?  Only one mprotect() call is needed to make the entire stack
  >> executable.

  Jakub> Nope.  Think about multithreaded apps.

I said one mprotect() to make the entire stack executable.  Obviously,
if you have multiple stacks, you need one call per stack.  Big deal.

  Jakub> Furthermore, getting the exact extents of the particular
  Jakub> stack is difficult to find for applications, but e.g. the
  Jakub> threading library has to know such things.

And how is this a kernel problem?  There are other reasons why
applications need to know the stack extents (think garbage collector)
and it's entirely glibc's failing if it's difficult to determine the
stack extent.

	--david

Re: Non-Exec stack patches

[John Reiser]

> Only one mprotect() call is needed to make the entire stack
> executable.

mprotect() only works on the portion that is currently allocated.
If the stack grows, then another call is needed.  Tracking the high-
water mark is an expense.  Forcing the allocation of the maximum
extent for the stack of the current thread, even to the same copy-
on-write all-zero page, can cause memory overcommit problems.

-- 
John Reiser, jreiser@BitWagon.com

Re: Non-Exec stack patches

[David Mosberger]

>>>>> On Wed, 24 Mar 2004 11:02:45 -0800, John Reiser <jreiser@BitWagon.com> said:

  >> Only one mprotect() call is needed to make the entire stack
  >> executable.

  John> mprotect() only works on the portion that is currently allocated.
  John> If the stack grows, then another call is needed.

No, mprotect() on the entire stack will mark the vm_area with the
desired protection and VM_GROWSDOWN/VM_GROWSUP will expand
automatically with the new protection.  And if you want to expand the
stack in user-level, e.g., by intercepting SIGSEGV, you'll either do
an mmap() or mprotect() at any rate so there is zero extra cost there.

	--david

Re: Non-Exec stack patches

[Andrew Morton]

Kurt Garloff <garloff@suse.de> wrote:
>
> > Which architectures are currently making their pre-page execute permissions
> > depend upon VM_EXEC?  Would additional arch patches be needed for this?
> 
> It works on AMD64 (not ia32e), both for 64bit and 32bit binaries.
> I have not yet tested other archs.
> 
> If the values in the protection_map are different depending on bit 2,
> the patch will be effecitve. (OK, the CPU/MMU needs to honour the
> setting of course.) Most likely, the values for 
> protection_map[7] is PAGE_COPY_EXEC and of protection_map[3] is
> PAGE_COPY.

OK.

> > This may not get past Linus of course.  It doesn't even get past me with
> > that magical undocumented -1/0/+1 value of the executable_stack argument. 
> > Please replace that with a proper, commented, #defined-or-enumerated value,
> 
> As you wish, master.
> Slightly edited and untested patch attached.

It gets rejects in arch/x86_64/ia32/ia32_binfmt.c and
arch/ia64/ia32/binfmt_elf32.c - someone has been dinking with your
put_dirty_page() prototype.  I dropped those bits.

And I added the missing bit:

--- 25/include/linux/binfmts.h~noexec-stack-comments	Tue Mar 23 16:35:50 2004
+++ 25-akpm/include/linux/binfmts.h	Tue Mar 23 16:37:11 2004
@@ -62,9 +62,12 @@ extern int prepare_binprm(struct linux_b
 extern void remove_arg_zero(struct linux_binprm *);
 extern int search_binary_handler(struct linux_binprm *,struct pt_regs *);
 extern int flush_old_exec(struct linux_binprm * bprm);
-#define EXSTACK_DEFAULT   0
-#define EXSTACK_DISABLE_X 1
-#define EXSTACK_ENABLE_X  2
+
+/* Stack area protections */
+#define EXSTACK_DEFAULT   0	/* Whatever the arch defaults to */
+#define EXSTACK_DISABLE_X 1	/* Disable executable stacks */
+#define EXSTACK_ENABLE_X  2	/* Enable executable stacks */
+
 extern int setup_arg_pages(struct linux_binprm * bprm, int executable_stack);
 extern int copy_strings(int argc,char __user * __user * argv,struct linux_binprm *bprm); 
 extern int copy_strings_kernel(int argc,char ** argv,struct linux_binprm *bprm);



Now, what should the kernel do if the executable requests EXSTACK_DISABLE_X
but the kernel cannot do that?  Is it not a bit misleading/dangerous to
permit the executable to run anyway?

Re: Non-Exec stack patches

[Ingo Molnar]

On Tue, 23 Mar 2004, Andrew Morton wrote:

> Now, what should the kernel do if the executable requests
> EXSTACK_DISABLE_X but the kernel cannot do that?  Is it not a bit
> misleading/dangerous to permit the executable to run anyway?

it's not really a problem. We already ignore the !X bit on x86.

	Ingo

Re: Non-Exec stack patches

[Kurt Garloff]

[-- Attachment #1.1: Type: text/plain, Size: 1080 bytes --]

Hi Andrew, Ingo,

On Tue, Mar 23, 2004 at 04:41:04PM -0800, Andrew Morton wrote:
> It gets rejects in arch/x86_64/ia32/ia32_binfmt.c and
> arch/ia64/ia32/binfmt_elf32.c - someone has been dinking with your
> put_dirty_page() prototype. 

That's the anon_vma stuff from Andrea, which we have merged locally.
Sorry, I forgot that it touched these places.

>  I dropped those bits.

Not a good idea, as stack protection won't work in ia32 emulation of
x86_64 and ia64 then.

I readded and did a clean diff against 2.6.5-rc2 and also integrated 
your comments patch.

> And I added the missing bit:

Thx!

> Now, what should the kernel do if the executable requests EXSTACK_DISABLE_X
> but the kernel cannot do that?  Is it not a bit misleading/dangerous to
> permit the executable to run anyway?

Yes. It has to ... 
Otherwise, most binaries would not work in i386 any more :-/

Updated patch attached.
-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

[-- Attachment #1.2: noexec-stack.patch --]
[-- Type: text/plain, Size: 15351 bytes --]


From: Kurt Garloff <garloff@suse.de>

A patch to parse the elf binaries for a PT_GNU_STACK section to set the
stack non-executable if possible.  Most parts have been shamelessly stolen
from Ingo Molnar's more ambitious stackshield
http://people.redhat.com/mingo/exec-shield/exec-shield-2.6.4-C9

The toolchain has meanwhile support for marking the binaries with a
PT_GNU_STACK section wwithout x bit as needed.

If no such section is found, we leave the stack to whatever the arch
defaults to.  If there is one, we explicitly disabled the VM_EXEC bit if no
x bit is found, otherwise explicitly enable.

---

 arch/ia64/ia32/binfmt_elf32.c       |   16 +++++++++++-----
 arch/ia64/ia32/ia32priv.h           |    2 +-
 arch/mips/kernel/irixelf.c          |    2 +-
 arch/s390/kernel/binfmt_elf32.c     |    4 ++--
 arch/s390/kernel/compat_exec.c      |    3 ++-
 arch/sparc64/kernel/binfmt_aout32.c |    2 +-
 arch/x86_64/ia32/ia32_aout.c        |    4 ++--
 arch/x86_64/ia32/ia32_binfmt.c      |   15 ++++++++++-----
 fs/binfmt_aout.c                    |    2 +-
 fs/binfmt_elf.c                     |   12 +++++++++++-
 fs/binfmt_som.c                     |    2 +-
 fs/exec.c                           |   14 +++++++++++---
 include/linux/binfmts.h             |    8 +++++++-
 include/linux/elf.h                 |    2 ++
 14 files changed, 63 insertions(+), 25 deletions(-)

diff -uNrp linux-2.6.5-rc2/arch/ia64/ia32/binfmt_elf32.c linux-2.6.5-rc2.nonexecstack/arch/ia64/ia32/binfmt_elf32.c
--- linux-2.6.5-rc2/arch/ia64/ia32/binfmt_elf32.c	2004-03-11 03:55:44.000000000 +0100
+++ linux-2.6.5-rc2.nonexecstack/arch/ia64/ia32/binfmt_elf32.c	2004-03-24 11:43:50.000000000 +0100
@@ -35,7 +35,7 @@ extern void ia64_elf32_init (struct pt_r
 
 static void elf32_set_personality (void);
 
-#define setup_arg_pages(bprm)		ia32_setup_arg_pages(bprm)
+#define setup_arg_pages(bprm,exec)		ia32_setup_arg_pages(bprm,exec)
 #define elf_map				elf32_map
 
 #undef SET_PERSONALITY
@@ -149,7 +149,7 @@ ia64_elf32_init (struct pt_regs *regs)
 }
 
 int
-ia32_setup_arg_pages (struct linux_binprm *bprm)
+ia32_setup_arg_pages (struct linux_binprm *bprm, int executable_stack)
 {
 	unsigned long stack_base;
 	struct vm_area_struct *mpnt;
@@ -178,8 +178,14 @@ ia32_setup_arg_pages (struct linux_binpr
 		mpnt->vm_mm = current->mm;
 		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
 		mpnt->vm_end = IA32_STACK_TOP;
-		mpnt->vm_page_prot = PAGE_COPY;
-		mpnt->vm_flags = VM_STACK_FLAGS;
+		if (executable_stack == EXSTACK_ENABLE_X)
+			mpnt->vm_flags = VM_STACK_FLAGS |  VM_EXEC;
+		else if (executable_stack == EXSTACK_DISABLE_X)
+			mpnt->vm_flags = VM_STACK_FLAGS & ~VM_EXEC;
+		else
+			mpnt->vm_flags = VM_STACK_FLAGS;
+		mpnt->vm_page_prot = (mpnt->vm_flags & VM_EXEC)?
+					PAGE_COPY_EXEC: PAGE_COPY;
 		mpnt->vm_ops = NULL;
 		mpnt->vm_pgoff = 0;
 		mpnt->vm_file = NULL;
@@ -192,7 +198,7 @@ ia32_setup_arg_pages (struct linux_binpr
 		struct page *page = bprm->page[i];
 		if (page) {
 			bprm->page[i] = NULL;
-			put_dirty_page(current, page, stack_base, PAGE_COPY);
+			put_dirty_page(current, page, stack_base, mpnt->vm_page_prot);
 		}
 		stack_base += PAGE_SIZE;
 	}
diff -uNrp linux-2.6.5-rc2/arch/ia64/ia32/ia32priv.h linux-2.6.5-rc2.nonexecstack/arch/ia64/ia32/ia32priv.h
--- linux-2.6.5-rc2/arch/ia64/ia32/ia32priv.h	2004-03-11 03:55:51.000000000 +0100
+++ linux-2.6.5-rc2.nonexecstack/arch/ia64/ia32/ia32priv.h	2004-03-24 11:43:04.000000000 +0100
@@ -494,7 +494,7 @@ struct ia32_user_desc {
 struct linux_binprm;
 
 extern void ia32_init_addr_space (struct pt_regs *regs);
-extern int ia32_setup_arg_pages (struct linux_binprm *bprm);
+extern int ia32_setup_arg_pages (struct linux_binprm *bprm, int exec_stack);
 extern unsigned long ia32_do_mmap (struct file *, unsigned long, unsigned long, int, int, loff_t);
 extern void ia32_load_segment_descriptors (struct task_struct *task);
 
diff -uNrp linux-2.6.5-rc2/arch/mips/kernel/irixelf.c linux-2.6.5-rc2.nonexecstack/arch/mips/kernel/irixelf.c
--- linux-2.6.5-rc2/arch/mips/kernel/irixelf.c	2004-03-11 03:56:03.000000000 +0100
+++ linux-2.6.5-rc2.nonexecstack/arch/mips/kernel/irixelf.c	2004-03-24 11:43:04.000000000 +0100
@@ -688,7 +688,7 @@ static int load_irix_binary(struct linux
 	 * change some of these later.
 	 */
 	current->mm->rss = 0;
-	setup_arg_pages(bprm);
+	setup_arg_pages(bprm, EXSTACK_DEFAULT);
 	current->mm->start_stack = bprm->p;
 
 	/* At this point, we assume that the image should be loaded at
diff -uNrp linux-2.6.5-rc2/arch/s390/kernel/binfmt_elf32.c linux-2.6.5-rc2.nonexecstack/arch/s390/kernel/binfmt_elf32.c
--- linux-2.6.5-rc2/arch/s390/kernel/binfmt_elf32.c	2004-03-11 03:55:44.000000000 +0100
+++ linux-2.6.5-rc2.nonexecstack/arch/s390/kernel/binfmt_elf32.c	2004-03-24 11:43:04.000000000 +0100
@@ -114,7 +114,7 @@ typedef s390_regs32 elf_gregset_t;
 #include <linux/binfmts.h>
 #include <linux/compat.h>
 
-int setup_arg_pages32(struct linux_binprm *bprm);
+int setup_arg_pages32(struct linux_binprm *bprm, int executable_stack);
 
 #define elf_prstatus elf_prstatus32
 struct elf_prstatus32
@@ -165,7 +165,7 @@ struct elf_prpsinfo32
 
 #undef start_thread
 #define start_thread                    start_thread31 
-#define setup_arg_pages(bprm)           setup_arg_pages32(bprm)
+#define setup_arg_pages(bprm, exec)     setup_arg_pages32(bprm, exec)
 #define elf_map				elf_map32
 
 MODULE_DESCRIPTION("Binary format loader for compatibility with 32bit Linux for S390 binaries,"
diff -uNrp linux-2.6.5-rc2/arch/s390/kernel/compat_exec.c linux-2.6.5-rc2.nonexecstack/arch/s390/kernel/compat_exec.c
--- linux-2.6.5-rc2/arch/s390/kernel/compat_exec.c	2004-03-11 03:55:44.000000000 +0100
+++ linux-2.6.5-rc2.nonexecstack/arch/s390/kernel/compat_exec.c	2004-03-24 11:43:04.000000000 +0100
@@ -37,7 +37,7 @@
 #undef STACK_TOP
 #define STACK_TOP TASK31_SIZE
 
-int setup_arg_pages32(struct linux_binprm *bprm)
+int setup_arg_pages32(struct linux_binprm *bprm, int executable_stack)
 {
 	unsigned long stack_base;
 	struct vm_area_struct *mpnt;
@@ -66,6 +66,7 @@ int setup_arg_pages32(struct linux_binpr
 		mpnt->vm_mm = mm;
 		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
 		mpnt->vm_end = STACK_TOP;
+		/* executable stack setting would be applied here */
 		mpnt->vm_page_prot = PAGE_COPY;
 		mpnt->vm_flags = VM_STACK_FLAGS;
 		mpnt->vm_ops = NULL;
diff -uNrp linux-2.6.5-rc2/arch/sparc64/kernel/binfmt_aout32.c linux-2.6.5-rc2.nonexecstack/arch/sparc64/kernel/binfmt_aout32.c
--- linux-2.6.5-rc2/arch/sparc64/kernel/binfmt_aout32.c	2004-03-11 03:55:27.000000000 +0100
+++ linux-2.6.5-rc2.nonexecstack/arch/sparc64/kernel/binfmt_aout32.c	2004-03-24 11:43:04.000000000 +0100
@@ -310,7 +310,7 @@ beyond_if:
 	orig_thr_flags = current_thread_info()->flags;
 	current_thread_info()->flags |= _TIF_32BIT;
 
-	retval = setup_arg_pages(bprm);
+	retval = setup_arg_pages(bprm, EXSTACK_DEFAULT);
 	if (retval < 0) { 
 		current_thread_info()->flags = orig_thr_flags;
 
diff -uNrp linux-2.6.5-rc2/arch/x86_64/ia32/ia32_aout.c linux-2.6.5-rc2.nonexecstack/arch/x86_64/ia32/ia32_aout.c
--- linux-2.6.5-rc2/arch/x86_64/ia32/ia32_aout.c	2004-03-11 03:55:54.000000000 +0100
+++ linux-2.6.5-rc2.nonexecstack/arch/x86_64/ia32/ia32_aout.c	2004-03-24 11:43:04.000000000 +0100
@@ -35,7 +35,7 @@
 #undef WARN_OLD
 #undef CORE_DUMP /* probably broken */
 
-extern int ia32_setup_arg_pages(struct linux_binprm *bprm);
+extern int ia32_setup_arg_pages(struct linux_binprm *bprm, int exec_stack);
 
 static int load_aout_binary(struct linux_binprm *, struct pt_regs * regs);
 static int load_aout_library(struct file*);
@@ -395,7 +395,7 @@ beyond_if:
 
 	set_brk(current->mm->start_brk, current->mm->brk);
 
-	retval = ia32_setup_arg_pages(bprm); 
+	retval = ia32_setup_arg_pages(bprm, EXSTACK_DEFAULT);
 	if (retval < 0) { 
 		/* Someone check-me: is this error path enough? */ 
 		send_sig(SIGKILL, current, 0); 
diff -uNrp linux-2.6.5-rc2/arch/x86_64/ia32/ia32_binfmt.c linux-2.6.5-rc2.nonexecstack/arch/x86_64/ia32/ia32_binfmt.c
--- linux-2.6.5-rc2/arch/x86_64/ia32/ia32_binfmt.c	2004-03-24 11:42:03.000000000 +0100
+++ linux-2.6.5-rc2.nonexecstack/arch/x86_64/ia32/ia32_binfmt.c	2004-03-24 11:44:30.000000000 +0100
@@ -272,8 +272,8 @@ do {							\
 #define load_elf_binary load_elf32_binary
 
 #define ELF_PLAT_INIT(r, load_addr)	elf32_init(r)
-#define setup_arg_pages(bprm)		ia32_setup_arg_pages(bprm)
-int ia32_setup_arg_pages(struct linux_binprm *bprm);
+#define setup_arg_pages(bprm, exec_stack)	ia32_setup_arg_pages(bprm, exec_stack)
+int ia32_setup_arg_pages(struct linux_binprm *bprm, int executable_stack);
 
 #undef start_thread
 #define start_thread(regs,new_rip,new_rsp) do { \
@@ -325,7 +325,7 @@ static void elf32_init(struct pt_regs *r
 	me->thread.es = __USER_DS;
 }
 
-int setup_arg_pages(struct linux_binprm *bprm)
+int setup_arg_pages(struct linux_binprm *bprm, int executable_stack)
 {
 	unsigned long stack_base;
 	struct vm_area_struct *mpnt;
@@ -354,7 +354,12 @@ int setup_arg_pages(struct linux_binprm 
 		mpnt->vm_mm = mm;
 		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
 		mpnt->vm_end = IA32_STACK_TOP;
-		mpnt->vm_flags = vm_stack_flags32; 
+		if (executable_stack == EXSTACK_ENABLE_X)
+			mpnt->vm_flags = vm_stack_flags32 |  VM_EXEC;
+		else if (executable_stack == EXSTACK_DISABLE_X)
+			mpnt->vm_flags = vm_stack_flags32 & ~VM_EXEC;
+		else
+			mpnt->vm_flags = vm_stack_flags32;
  		mpnt->vm_page_prot = (mpnt->vm_flags & VM_EXEC) ? 
  			PAGE_COPY_EXEC : PAGE_COPY;
 		mpnt->vm_ops = NULL;
@@ -370,7 +375,7 @@ int setup_arg_pages(struct linux_binprm 
 		struct page *page = bprm->page[i];
 		if (page) {
 			bprm->page[i] = NULL;
-			put_dirty_page(current,page,stack_base,PAGE_COPY_EXEC);
+			put_dirty_page(current,page,stack_base,mpnt->vm_page_prot);
 		}
 		stack_base += PAGE_SIZE;
 	}
diff -uNrp linux-2.6.5-rc2/fs/binfmt_aout.c linux-2.6.5-rc2.nonexecstack/fs/binfmt_aout.c
--- linux-2.6.5-rc2/fs/binfmt_aout.c	2004-03-11 03:55:23.000000000 +0100
+++ linux-2.6.5-rc2.nonexecstack/fs/binfmt_aout.c	2004-03-24 11:43:04.000000000 +0100
@@ -413,7 +413,7 @@ beyond_if:
 
 	set_brk(current->mm->start_brk, current->mm->brk);
 
-	retval = setup_arg_pages(bprm); 
+	retval = setup_arg_pages(bprm, EXSTACK_DEFAULT);
 	if (retval < 0) { 
 		/* Someone check-me: is this error path enough? */ 
 		send_sig(SIGKILL, current, 0); 
diff -uNrp linux-2.6.5-rc2/fs/binfmt_elf.c linux-2.6.5-rc2.nonexecstack/fs/binfmt_elf.c
--- linux-2.6.5-rc2/fs/binfmt_elf.c	2004-03-24 11:42:03.000000000 +0100
+++ linux-2.6.5-rc2.nonexecstack/fs/binfmt_elf.c	2004-03-24 11:43:04.000000000 +0100
@@ -476,6 +476,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;
 	
 	/* Get the exec-header */
 	elf_ex = *((struct elfhdr *) bprm->buf);
@@ -599,6 +600,15 @@ static int load_elf_binary(struct linux_
 		elf_ppnt++;
 	}
 
+	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;
+		}
+
 	/* Some simple consistency checks for the interpreter */
 	if (elf_interpreter) {
 		interpreter_type = INTERPRETER_ELF | INTERPRETER_AOUT;
@@ -674,7 +684,7 @@ static int load_elf_binary(struct linux_
 	   change some of these later */
 	current->mm->rss = 0;
 	current->mm->free_area_cache = TASK_UNMAPPED_BASE;
-	retval = setup_arg_pages(bprm);
+	retval = setup_arg_pages(bprm, executable_stack);
 	if (retval < 0) {
 		send_sig(SIGKILL, current, 0);
 		goto out_free_dentry;
diff -uNrp linux-2.6.5-rc2/fs/binfmt_som.c linux-2.6.5-rc2.nonexecstack/fs/binfmt_som.c
--- linux-2.6.5-rc2/fs/binfmt_som.c	2004-03-11 03:55:25.000000000 +0100
+++ linux-2.6.5-rc2.nonexecstack/fs/binfmt_som.c	2004-03-24 11:43:04.000000000 +0100
@@ -254,7 +254,7 @@ load_som_binary(struct linux_binprm * bp
 
 	set_binfmt(&som_format);
 	compute_creds(bprm);
-	setup_arg_pages(bprm);
+	setup_arg_pages(bprm, EXSTACK_DEFAULT);
 
 	create_som_tables(bprm);
 
diff -uNrp linux-2.6.5-rc2/fs/exec.c linux-2.6.5-rc2.nonexecstack/fs/exec.c
--- linux-2.6.5-rc2/fs/exec.c	2004-03-11 03:55:25.000000000 +0100
+++ linux-2.6.5-rc2.nonexecstack/fs/exec.c	2004-03-24 11:43:04.000000000 +0100
@@ -342,7 +342,7 @@ out_sig:
 	return;
 }
 
-int setup_arg_pages(struct linux_binprm *bprm)
+int setup_arg_pages(struct linux_binprm *bprm, int executable_stack)
 {
 	unsigned long stack_base;
 	struct vm_area_struct *mpnt;
@@ -425,8 +425,16 @@ int setup_arg_pages(struct linux_binprm 
 		mpnt->vm_start = PAGE_MASK & (unsigned long) bprm->p;
 		mpnt->vm_end = STACK_TOP;
 #endif
-		mpnt->vm_page_prot = protection_map[VM_STACK_FLAGS & 0x7];
-		mpnt->vm_flags = VM_STACK_FLAGS;
+		/* Adjust stack execute permissions; explicitly enable
+		 * for EXSTACK_ENABLE_X, disable for EXSTACK_DISABLE_X
+		 * and leave alone (arch default) otherwise. */
+		if (unlikely(executable_stack == EXSTACK_ENABLE_X))
+			mpnt->vm_flags = VM_STACK_FLAGS |  VM_EXEC;
+		else if (executable_stack == EXSTACK_DISABLE_X)
+			mpnt->vm_flags = VM_STACK_FLAGS & ~VM_EXEC;
+		else
+			mpnt->vm_flags = VM_STACK_FLAGS;
+		mpnt->vm_page_prot = protection_map[mpnt->vm_flags & 0x7];
 		mpnt->vm_ops = NULL;
 		mpnt->vm_pgoff = 0;
 		mpnt->vm_file = NULL;
diff -uNrp linux-2.6.5-rc2/include/linux/binfmts.h linux-2.6.5-rc2.nonexecstack/include/linux/binfmts.h
--- linux-2.6.5-rc2/include/linux/binfmts.h	2004-03-11 03:55:27.000000000 +0100
+++ linux-2.6.5-rc2.nonexecstack/include/linux/binfmts.h	2004-03-24 11:43:10.000000000 +0100
@@ -58,7 +58,13 @@ extern int prepare_binprm(struct linux_b
 extern void remove_arg_zero(struct linux_binprm *);
 extern int search_binary_handler(struct linux_binprm *,struct pt_regs *);
 extern int flush_old_exec(struct linux_binprm * bprm);
-extern int setup_arg_pages(struct linux_binprm * bprm);
+
+/* Stack area protections */
+#define EXSTACK_DEFAULT   0	/* Whatever the arch defaults to */
+#define EXSTACK_DISABLE_X 1	/* Disable executable stacks */
+#define EXSTACK_ENABLE_X  2	/* Enable executable stacks */
+
+extern int setup_arg_pages(struct linux_binprm * bprm, int executable_stack);
 extern int copy_strings(int argc,char __user * __user * argv,struct linux_binprm *bprm); 
 extern int copy_strings_kernel(int argc,char ** argv,struct linux_binprm *bprm);
 extern void compute_creds(struct linux_binprm *binprm);
diff -uNrp linux-2.6.5-rc2/include/linux/elf.h linux-2.6.5-rc2.nonexecstack/include/linux/elf.h
--- linux-2.6.5-rc2/include/linux/elf.h	2004-03-11 03:55:23.000000000 +0100
+++ linux-2.6.5-rc2.nonexecstack/include/linux/elf.h	2004-03-24 11:43:04.000000000 +0100
@@ -35,6 +35,8 @@ typedef __s64	Elf64_Sxword;
 #define PT_HIPROC  0x7fffffff
 #define PT_GNU_EH_FRAME		0x6474e550
 
+#define PT_GNU_STACK	(PT_LOOS + 0x474e551)
+
 /* These constants define the different elf file types */
 #define ET_NONE   0
 #define ET_REL    1

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Non-Exec stack patches

[Andi Kleen]

Andrew Morton <akpm@osdl.org> writes:

> Which architectures are currently making their pre-page execute permissions
> depend upon VM_EXEC?  
> Would additional arch patches be needed for this?

Yes, they would need some straight forward minor patches e.g. in the
32bit emulation. IA64 would be a candidate I guess.

i386 could do it on NX capable CPUs with PAE kernels (but it would require 
backporting some fixes from x86-64). However currently it doesn't make
much sense because all x86 CPUs that support NX (AMD K8 currently only) 
support 64bit kernels and people can as well run 64bit kernels.
 
Doing it on i386 would only make sense if non 64bit capable CPUs ever get
NX. I heard VIA may be planning that, but so far there is nothing in their
shipping CPUs, so I guess we can skip that for now.

-Andi

Re: Non-Exec stack patches

[Stefan Smietanowski]

Hi Andi.

>>Which architectures are currently making their pre-page execute permissions
>>depend upon VM_EXEC?  
>>Would additional arch patches be needed for this?
> 
> 
> Yes, they would need some straight forward minor patches e.g. in the
> 32bit emulation. IA64 would be a candidate I guess.
> 
> i386 could do it on NX capable CPUs with PAE kernels (but it would require 
> backporting some fixes from x86-64). However currently it doesn't make
> much sense because all x86 CPUs that support NX (AMD K8 currently only) 
> support 64bit kernels and people can as well run 64bit kernels.
>  
> Doing it on i386 would only make sense if non 64bit capable CPUs ever get
> NX. I heard VIA may be planning that, but so far there is nothing in their
> shipping CPUs, so I guess we can skip that for now.

Well, there's also the case that (unknown if rumour or confirmed) there
will be AthlonXPs based on the K8 core that do NOT run 64bit code.

I would THINK they would include the NX bit but that's just a guess of
course.

// Stefan

Re: Non-Exec stack patches

[Andi Kleen]

> Well, there's also the case that (unknown if rumour or confirmed) there
> will be AthlonXPs based on the K8 core that do NOT run 64bit code.

Yes, you're right. Some HP laptops ship with such crippled chips. 

> I would THINK they would include the NX bit but that's just a guess of
> course.

Most likely yes. 

But who buys such crippled CPUs will have to live with that. Or do a patch.
NX support is mostly hype anyways, it doesn't give you much advantage.

-Andi

Re: Non-Exec stack patches

[Kurt Garloff]

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

Hi Andi,

On Wed, Mar 24, 2004 at 12:27:39PM +0100, Andi Kleen wrote:
Stefan Smietanowski <stesmi@stesmi.com> wrote:
> > I would THINK they would include the NX bit but that's just a guess of
> > course.
> 
> Most likely yes. 
> 
> But who buys such crippled CPUs will have to live with that. Or do a patch.

It should be straightforward. Put a different value in the
protection_map if such a CPU is detected. That should be all.

> NX support is mostly hype anyways, it doesn't give you much advantage.

It puts the hurdle somewhat higher and for some exploits does prevent
them. You can still overwrite the return address, but you need to have
put code into the data section prior to this to exploit. This may or may
not possible depending on how the daemon works. Marking the data section
non-executable would help further ...

It's always a tradeoff. Given the simplicity of the patch, I'm in favour
of having it.

Regards,
-- 
Kurt Garloff                   <kurt@garloff.de>             [Koeln, DE]
Physics:Plasma modeling <garloff@plasimo.phys.tue.nl> [TU Eindhoven, NL]
Linux: SUSE Labs (Head)        <garloff@suse.de>    [SUSE Nuernberg, DE]

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Non-Exec stack patches

[Andi Kleen]

Jakub Jelinek <jakub@redhat.com> writes:
> 
> Nope.  Think about multithreaded apps.  Furthermore, getting the exact
> extents of the particular stack is difficult to find for applications,
> but e.g. the threading library has to know such things.

It's actually not that difficult. You just have to read /proc/self/maps
and check for the mapping of your current stack pointer.
For the main stack GROWSDOWN will inherit the x or nx on growing
down.

And cache a flag about this in a TLS variable.

-Andi

Re: Non-Exec stack patches

[Jakub Jelinek]

On Wed, Mar 24, 2004 at 06:49:21PM +0100, Andi Kleen wrote:
> It's actually not that difficult. You just have to read /proc/self/maps
> and check for the mapping of your current stack pointer.
> For the main stack GROWSDOWN will inherit the x or nx on growing
> down.

That assumes there are always gaps around thread stacks, which doesn't
have to be true.  You could make executable some other mapping
as well easily.

	Jakub

RE: Non-Exec stack patches

[Siddha, Suresh B]

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

> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org
> [mailto:linux-kernel-owner@vger.kernel.org]On Behalf Of Andrew Morton
> Sent: Tuesday, March 23, 2004 4:41 PM
> To: Kurt Garloff
> Cc: linux-kernel@vger.kernel.org; mingo@redhat.com
> Subject: Re: Non-Exec stack patches
> 
> 
> Kurt Garloff <garloff@suse.de> wrote:
> >
> > > Which architectures are currently making their pre-page 
> execute permissions
> > > depend upon VM_EXEC?  Would additional arch patches be 
> needed for this?
> > 
> > It works on AMD64 (not ia32e), both for 64bit and 32bit binaries.
> > I have not yet tested other archs.
> > 
> > If the values in the protection_map are different depending 
> on bit 2,
> > the patch will be effecitve. (OK, the CPU/MMU needs to honour the
> > setting of course.) Most likely, the values for 
> > protection_map[7] is PAGE_COPY_EXEC and of protection_map[3] is
> > PAGE_COPY.
> 
> OK.
> 

Recent ia64 mm trees are broken because of this issue. Attached patch fixes protection_map[7] in IA64.

thanks,
suresh

[-- Attachment #2: noexec-ia64.fix --]
[-- Type: application/octet-stream, Size: 458 bytes --]

--- linux-265mm5/include/asm-ia64/pgtable.h~	2004-04-14 00:09:04.000000000 -0700
+++ linux-265mm5/include/asm-ia64/pgtable.h	2004-04-13 23:45:29.000000000 -0700
@@ -148,7 +148,7 @@
 #define __P100	__pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_X_RX)
 #define __P101	__pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_RX)
 #define __P110	PAGE_COPY
-#define __P111	PAGE_COPY
+#define __P111	PAGE_COPY_EXEC
 
 #define __S000	PAGE_NONE
 #define __S001	PAGE_READONLY

Re: Non-Exec stack patches

[Jamie Lokier]

Siddha, Suresh B wrote:
> Recent ia64 mm trees are broken because of this issue. Attached
> patch fixes protection_map[7] in IA64.

> --- linux-265mm5/include/asm-ia64/pgtable.h~    2004-04-14 00:09:04.000000000 -0700
> +++ linux-265mm5/include/asm-ia64/pgtable.h     2004-04-13 23:45:29.000000000 -0700
> @@ -148,7 +148,7 @@
>  #define __P100 __pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_X_RX)
>  #define __P101 __pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_RX)
>  #define __P110 PAGE_COPY
> -#define __P111 PAGE_COPY
> +#define __P111 PAGE_COPY_EXEC
>   
>  #define __S000 PAGE_NONE
>  #define __S001 PAGE_READONLY

I'm looking at 2.6.5, and it doesn't define PAGE_COPY_EXEC.  In 2.6.5,
asm-ia64/pgtable.h, PAGE_COPY is executable, and PAGE_READONLY is
non-executable.

Yes I know the naming is different from all the other asm-* files, but
that's what it says.  All of those definitions have confused names on
ia64, although they seem to have the rights bits set in the end.

Has the meaning of PAGE_COPY in asm-ia64/pgtable.h changed in 2.6.5-mm5?

If so, you want to change __P110 as well as __P111.

-- Jamie

Re: Non-Exec stack patches

[Jamie Lokier]

People looking at PROT_EXEC page table flags might want to be aware
that <asm-um/pgtable.h> mimics the behaviour of i386: read implies and
is implied by exec, write implies read.

That might mean user-mode linux doesn't provide no-exec-stack
protection even when the underlying kernel does offer it.  I'm not sure.

-- Jamie

Re: Non-Exec stack patches

[Kurt Garloff]

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

On Wed, Apr 14, 2004 at 10:47:02AM +0100, Jamie Lokier wrote:
> People looking at PROT_EXEC page table flags might want to be aware
> that <asm-um/pgtable.h> mimics the behaviour of i386: read implies and
> is implied by exec, write implies read.
> 
> That might mean user-mode linux doesn't provide no-exec-stack
> protection even when the underlying kernel does offer it.  I'm not sure.

I thought UML only runs on i386.
And on i386, you have no NX feature.
You can run i386 UML on AMD64 (with 64bit kernel) though.

Regards,
-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Non-Exec stack patches

[Jeff Dike]

garloff@suse.de said:
> I thought UML only runs on i386. And on i386, you have no NX feature.
> You can run i386 UML on AMD64 (with 64bit kernel) though. 

It also runs natively on and64 (i.e. a 64 bit UML).  The UML page table stuff
is going to need to be fixed for NX, but there's no reason that it won't work.

				Jeff

RE: Non-Exec stack patches

[Siddha, Suresh B]

Jamie Lokier wrote:
> Has the meaning of PAGE_COPY in asm-ia64/pgtable.h changed in 
> 2.6.5-mm5?

Yes. Kurt's recent patch for parsing PT_GNU_STACK section introduced
this change.

Here is the relevant hunk.
-#define PAGE_COPY      __pgprot(__ACCESS_BITS | _PAGE_PL_3 |
_PAGE_AR_RX)
+#define PAGE_COPY      __pgprot(__ACCESS_BITS | _PAGE_PL_3 |
_PAGE_AR_R)
+#define PAGE_COPY_EXEC __pgprot(__ACCESS_BITS | _PAGE_PL_3 |
_PAGE_AR_RX)

> 
> If so, you want to change __P110 as well as __P111.

No. Only EXEC bit is the difference.

thanks,
suresh

Re: Non-Exec stack patches

[Jamie Lokier]

Siddha, Suresh B wrote:
> > If so, you want to change __P110 as well as __P111.
> 
> No. Only EXEC bit is the difference.

Yes.  __P110 means WRITE+EXEC.  __P111 means READ+WRITE+EXEC.

-- Jamie

RE: Non-Exec stack patches

[Siddha, Suresh B]

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

Jamie Lokier wrote:
> Siddha, Suresh B wrote:
> > > If so, you want to change __P110 as well as __P111.
> > 
> > No. Only EXEC bit is the difference.
> 
> Yes.  __P110 means WRITE+EXEC.  __P111 means READ+WRITE+EXEC.
> 

Thanks Jamie. Attached the updated patch. Andrew please apply.

thanks,
suresh


[-- Attachment #2: noexec-ia64.fix --]
[-- Type: application/octet-stream, Size: 531 bytes --]

--- linux-265mm5/include/asm-ia64/pgtable.h~	2004-04-14 00:09:04.000000000 -0700
+++ linux-265mm5/include/asm-ia64/pgtable.h	2004-04-14 11:21:09.000000000 -0700
@@ -147,8 +147,8 @@
 #define __P011	PAGE_READONLY	/* ditto */
 #define __P100	__pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_X_RX)
 #define __P101	__pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_RX)
-#define __P110	PAGE_COPY
-#define __P111	PAGE_COPY
+#define __P110	PAGE_COPY_EXEC
+#define __P111	PAGE_COPY_EXEC
 
 #define __S000	PAGE_NONE
 #define __S001	PAGE_READONLY