* [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11
@ 2005-02-06 11:36 Andi Kleen
2005-02-06 11:47 ` Arjan van de Ven
` (2 more replies)
0 siblings, 3 replies; 36+ messages in thread
From: Andi Kleen @ 2005-02-06 11:36 UTC (permalink / raw)
To: akpm, torvalds; +Cc: mingo, linux-kernel
Hallo,
I'm getting more and more reports that the PT_GNU_STACK change breaks
a lot of programs. In particular it breaks grub which implies that nobody
could have really tested it. Mono seems to break too, there are undoubtedly
others. There are some reports of Wine breaking too, I suspect
they are related to this too. The testing for this was obviously not very
effective.
PT_GNU_STACK assumes that any program with a PT_GNU_STACK header
always pass correct PROT_EXEC flags to mmap/mprotect etc. before
allocating mappings.
The problem is that people are recompiling these programs with
new compilers, the new compilers set PT_GNU_STACK and then they
break. Sometimes obviously (like grub), I bet in some other cases
they break only in obscure cases that are hard to catch.
Worse is that even when the program has trampolines and has PT_GNU_STACK
header with an E bit on the stack it still won't get an executable
heap by default (this is what broke grub)
This is a major source of source level and binary incompatibility.
People reporting this to me all the time for x86-64 because few people
run PAE/NX enabled kernels on i386, and without PAE this breakage
is not visible. Also you need a NX capable CPU of course,
and basically all NX capable CPUs are able to run x86-64, so a
lot of them run 64bit kernels. That is why the full impact of these
changes was not felt on i386.
I'm not really willing to handle all these problems on the x86-64 side,
for what was ultimatively a half baked change comming from i386.
My proposal is to turn this all off at least for 2.6.11.
If it should be reenabled later probably needs a lot of discussion.
IMHO it's not a very good idea in 2.6 because it's an extremly invasive change
in the ABI and the security advantages of using NX are not that great anyways.
I understand that some people want to have it as part of a marketing
campaign as "answer to Microsoft SP2", but IMHO source & binary compatibility
is too important to be sacrified for such things. If they really
want it they can continue to maintain it as private patches.
I also don't remember a thread on linux-kernel discussion these
trade offs and what significant impact it has on user space.
Before reenabling it I think we would need a lot of discussion
if it's really worth it, and a lot more education of user land
people on what they need to do to cope with this.
One way would be to have special command line flags or sysctls like
x86-64 used to have for this (but they were removed as "too confusing"),
and then only people who know what they are doing or can tolerate
some of their programs breaking can enable it.
But forcing it on all users doesn't seem to work very well.
Here's a patch to disable PT_GNU_STACK parsing for now. I would
prefer it to be merged before 2.6.11.
Thanks,
-Andi
Remove PT_GNU_STACK support because user land is not ready for it yet.
Signed-off-by: Andi Kleen <ak@suse.de>
diff -u linux-2.6.11rc3/fs/binfmt_elf.c-o linux-2.6.11rc3/fs/binfmt_elf.c
--- linux-2.6.11rc3/fs/binfmt_elf.c-o 2005-02-04 09:13:18.000000000 +0100
+++ linux-2.6.11rc3/fs/binfmt_elf.c 2005-02-06 12:30:59.000000000 +0100
@@ -675,6 +675,13 @@
elf_ppnt++;
}
+#if 1
+ /*
+ * The user land is not ready for PT_GNU_STACK yet. Disable
+ * it for now.
+ */
+ have_pt_gnu_stack = 0;
+#else
elf_ppnt = elf_phdata;
for (i = 0; i < loc->elf_ex.e_phnum; i++, elf_ppnt++)
if (elf_ppnt->p_type == PT_GNU_STACK) {
@@ -685,6 +692,7 @@
break;
}
have_pt_gnu_stack = (i < loc->elf_ex.e_phnum);
+#endif
/* Some simple consistency checks for the interpreter */
if (elf_interpreter) {
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 11:36 [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 Andi Kleen @ 2005-02-06 11:47 ` Arjan van de Ven 2005-02-06 12:02 ` Ingo Molnar 2005-02-06 12:33 ` Andi Kleen 2005-02-06 12:11 ` Paweł Sikora [not found] ` <200502061303.12377.pluto@pld-linux.org> 2 siblings, 2 replies; 36+ messages in thread From: Arjan van de Ven @ 2005-02-06 11:47 UTC (permalink / raw) To: Andi Kleen; +Cc: akpm, torvalds, mingo, linux-kernel, drepper On Sun, Feb 06, 2005 at 12:36:35PM +0100, Andi Kleen wrote: > PT_GNU_STACK assumes that any program with a PT_GNU_STACK header > always pass correct PROT_EXEC flags to mmap/mprotect etc. before > allocating mappings. that is incorrect and was introduced later > Worse is that even when the program has trampolines and has PT_GNU_STACK > header with an E bit on the stack it still won't get an executable > heap by default (this is what broke grub) this I can fix easy, see the patch below the problem is in the read_implies_exec() design, it passed in "does it have a PT_GNU_STACK flag" not the value. Easy fix. Your main objection is that *incorrect* programs that assume they can execute malloc() code without PROT_EXEC protection. For legacy binaries keeping this behavior makes sense, no objection from me. For newly compiled programs this is just wrong and incorrect. You mention grub (which has RWE and the patch below thus makes that work) and mono. mono has patches for this on their mailinglist and bugzilla since 2003 according to google, I'm surprised the novell mono guys haven't fixed this bug in their code. FWIW all jvm's don't suffer from this. They are either legacy binaries or mprotect properly (only i386 traditionally had this behavior, all others already required PROT_EXEC anyway so any half portable app already did this, as well as any app portable to BSD since they enforce this on x86 as well) So I rather see the patch below merged instead; it fixes the worst problems (RWE not marking the heap executable) while keeping this useful feature enabled. Signed-off-by: Arjan van de Ven <arjan@infradead.org> diff -purN linux-2.6.11-rc2-bk4/fs/binfmt_elf.c linux-foo/fs/binfmt_elf.c --- linux-2.6.11-rc2-bk4/fs/binfmt_elf.c 2005-01-26 18:24:50.000000000 +0100 +++ linux-foo/fs/binfmt_elf.c 2005-02-06 12:29:02.000000000 +0100 @@ -757,7 +757,7 @@ static int load_elf_binary(struct linux_ /* Do this immediately, since STACK_TOP as used in setup_arg_pages may depend on the personality. */ SET_PERSONALITY(loc->elf_ex, ibcs2_interpreter); - if (elf_read_implies_exec(loc->elf_ex, have_pt_gnu_stack)) + if (elf_read_implies_exec(loc->elf_ex, executable_stack)) current->personality |= READ_IMPLIES_EXEC; arch_pick_mmap_layout(current->mm); diff -purN linux-2.6.11-rc2-bk4/include/asm-i386/elf.h linux-foo/include/asm-i386/elf.h --- linux-2.6.11-rc2-bk4/include/asm-i386/elf.h 2004-12-24 22:35:15.000000000 +0100 +++ linux-foo/include/asm-i386/elf.h 2005-02-06 12:29:55.000000000 +0100 @@ -123,7 +123,7 @@ typedef struct user_fxsr_struct elf_fpxr * An executable for which elf_read_implies_exec() returns TRUE will * have the READ_IMPLIES_EXEC personality flag set automatically. */ -#define elf_read_implies_exec(ex, have_pt_gnu_stack) (!(have_pt_gnu_stack)) +#define elf_read_implies_exec(ex, executable_stack) (executable_stack != EXSTACK_DISABLE_X) extern int dump_task_regs (struct task_struct *, elf_gregset_t *); extern int dump_task_fpu (struct task_struct *, elf_fpregset_t *); diff -purN linux-2.6.11-rc2-bk4/include/asm-ia64/elf.h linux-foo/include/asm-ia64/elf.h --- linux-2.6.11-rc2-bk4/include/asm-ia64/elf.h 2004-12-24 22:35:18.000000000 +0100 +++ linux-foo/include/asm-ia64/elf.h 2005-02-06 12:32:47.000000000 +0100 @@ -186,8 +186,8 @@ extern void ia64_elf_core_copy_regs (str #ifdef __KERNEL__ #define SET_PERSONALITY(ex, ibcs2) set_personality(PER_LINUX) -#define elf_read_implies_exec(ex, have_pt_gnu_stack) \ - (!(have_pt_gnu_stack) && ((ex).e_flags & EF_IA_64_LINUX_EXECUTABLE_STACK) != 0) +#define elf_read_implies_exec(ex, executable_stack) \ + ((executable_stack!=EXSTACK_DISABLE_X) && ((ex).e_flags & EF_IA_64_LINUX_EXECUTABLE_STACK) != 0) struct task_struct; diff -purN linux-2.6.11-rc2-bk4/include/asm-x86_64/elf.h linux-foo/include/asm-x86_64/elf.h --- linux-2.6.11-rc2-bk4/include/asm-x86_64/elf.h 2004-12-24 22:35:24.000000000 +0100 +++ linux-foo/include/asm-x86_64/elf.h 2005-02-06 12:31:39.000000000 +0100 @@ -147,14 +147,7 @@ extern void set_personality_64bit(void); * An executable for which elf_read_implies_exec() returns TRUE will * have the READ_IMPLIES_EXEC personality flag set automatically. */ -#define elf_read_implies_exec(ex, have_pt_gnu_stack) (!(have_pt_gnu_stack)) - -/* - * An executable for which elf_read_implies_exec() returns TRUE will - * have the READ_IMPLIES_EXEC personality flag set automatically. - */ -#define elf_read_implies_exec_binary(ex, have_pt_gnu_stack) \ - (!(have_pt_gnu_stack)) +#define elf_read_implies_exec(ex, executable_stack) (executable_stack != EXSTACK_DISABLE_X) extern int dump_task_regs (struct task_struct *, elf_gregset_t *); extern int dump_task_fpu (struct task_struct *, elf_fpregset_t *); ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 11:47 ` Arjan van de Ven @ 2005-02-06 12:02 ` Ingo Molnar 2005-02-06 12:25 ` Ingo Molnar ` (2 more replies) 2005-02-06 12:33 ` Andi Kleen 1 sibling, 3 replies; 36+ messages in thread From: Ingo Molnar @ 2005-02-06 12:02 UTC (permalink / raw) To: Arjan van de Ven; +Cc: Andi Kleen, akpm, torvalds, linux-kernel, drepper * Arjan van de Ven <arjan@infradead.org> wrote: > > [...] when the program has trampolines and has PT_GNU_STACK > > header with an E bit on the stack it still won't get an executable > > heap by default (this is what broke grub) > > this I can fix easy, see the patch below > > the problem is in the read_implies_exec() design, it passed in "does > it have a PT_GNU_STACK flag" not the value. Easy fix. > So I rather see the patch below merged instead; it fixes the worst > problems (RWE not marking the heap executable) while keeping this > useful feature enabled. > > Signed-off-by: Arjan van de Ven <arjan@infradead.org> looks good. Signed-off-by: Ingo Molnar <mingo@elte.hu> (I'd like to stress that this problem only affects packages _recompiled_ with new gcc, running on NX capable CPUs - legacy apps or CPUs are in no way affected. Also, even with a recompile, apps/kernels/distros have a number of other options as well even without this kernel fix, of varying granularity: to use the setarch utility, to set the READ_IMPLIES_EXEC personality bit within the code, or to pass in the noexec=off kernel commandline option, or to add a oneliner patch to their heap of 1500+ kernel patches, or to fix the application. Also, with Arjan's patch applied, the execstack utility can be used to remark the binary permanently, if needed.) Ingo ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 12:02 ` Ingo Molnar @ 2005-02-06 12:25 ` Ingo Molnar 2005-02-06 12:36 ` Andi Kleen 2005-02-06 12:45 ` Ingo Molnar 2 siblings, 0 replies; 36+ messages in thread From: Ingo Molnar @ 2005-02-06 12:25 UTC (permalink / raw) To: Arjan van de Ven; +Cc: Andi Kleen, akpm, torvalds, linux-kernel, drepper * Ingo Molnar <mingo@elte.hu> wrote: > > > [...] when the program has trampolines and has PT_GNU_STACK > > > header with an E bit on the stack it still won't get an executable > > > heap by default (this is what broke grub) > > So I rather see the patch below merged instead; it fixes the worst > > problems (RWE not marking the heap executable) while keeping this > > useful feature enabled. > > > > Signed-off-by: Arjan van de Ven <arjan@infradead.org> > > looks good. > > Signed-off-by: Ingo Molnar <mingo@elte.hu> > > (I'd like to stress that this problem only affects packages > _recompiled_ with new gcc, running on NX capable CPUs - legacy apps or > CPUs are in no way affected. Also, even with a recompile, > apps/kernels/distros have a number of other options as well even > without this kernel fix, of varying granularity: to use the setarch > utility, to set the READ_IMPLIES_EXEC personality bit within the code, > or to pass in the noexec=off kernel commandline option, or to add a > oneliner patch to their heap of 1500+ kernel patches, or to fix the > application. Also, with Arjan's patch applied, the execstack utility > can be used to remark the binary permanently, if needed.) another, purely userspace solution is to add an execstack.c flag that clears the PT_GNU_STACK ELF program header and changes it to e.g. PT_NULL. That makes it a 'legacy' binary for the purposes of the kernel. Ingo ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 12:02 ` Ingo Molnar 2005-02-06 12:25 ` Ingo Molnar @ 2005-02-06 12:36 ` Andi Kleen 2005-02-06 12:45 ` Ingo Molnar 2 siblings, 0 replies; 36+ messages in thread From: Andi Kleen @ 2005-02-06 12:36 UTC (permalink / raw) To: Ingo Molnar Cc: Arjan van de Ven, Andi Kleen, akpm, torvalds, linux-kernel, drepper > Signed-off-by: Ingo Molnar <mingo@elte.hu> > > (I'd like to stress that this problem only affects packages _recompiled_ > with new gcc, running on NX capable CPUs - legacy apps or CPUs are in no Yeah, but who did the auditing of all user land packages if they don't need changes? And who told user land developers that they need to do this all if they only want to recompile software. That hasn't been done as far as I can figure out, and I'm not really willing to serve as the frontend/coordinator for this process on the x86-64 side. I would request that this stuff is only turned on by default until there is a reasonable process found for this. > way affected. Also, even with a recompile, apps/kernels/distros have a > number of other options as well even without this kernel fix, of varying > granularity: to use the setarch utility, to set the READ_IMPLIES_EXEC > personality bit within the code, or to pass in the noexec=off kernel > commandline option, or to add a oneliner patch to their heap of 1500+ > kernel patches, or to fix the application. Also, with Arjan's patch > applied, the execstack utility can be used to remark the binary > permanently, if needed.) That would still leave the mainline users who just update their kernels from a kernel.org tarball out in the rain. And guess who they will complain to if their favourite program doesn't work anymore in x86-64 32bit emulation? -Andi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 12:02 ` Ingo Molnar 2005-02-06 12:25 ` Ingo Molnar 2005-02-06 12:36 ` Andi Kleen @ 2005-02-06 12:45 ` Ingo Molnar 2005-02-06 12:50 ` Andi Kleen 2 siblings, 1 reply; 36+ messages in thread From: Ingo Molnar @ 2005-02-06 12:45 UTC (permalink / raw) To: Arjan van de Ven; +Cc: Andi Kleen, akpm, torvalds, linux-kernel, drepper * Ingo Molnar <mingo@elte.hu> wrote: > * Arjan van de Ven <arjan@infradead.org> wrote: > > > So I rather see the patch below merged instead; it fixes the worst > > problems (RWE not marking the heap executable) while keeping this > > useful feature enabled. > > > > Signed-off-by: Arjan van de Ven <arjan@infradead.org> > > looks good. > > Signed-off-by: Ingo Molnar <mingo@elte.hu> tested it against BK-curr, on an NX-enabled x86 CPU, and it builds/boots fine and works as expected. A PT_GNU_STACK RWE binary gets this layout: saturn:~/noexec> cat /proc/2983/maps 00888000-0089d000 r-xp 00000000 03:41 3433999 /lib/ld-2.3.3.so 0089d000-0089f000 rwxp 00014000 03:41 3433999 /lib/ld-2.3.3.so 008a1000-009bf000 r-xp 00000000 03:41 3434007 /lib/tls/libc-2.3.3.so 009bf000-009c1000 r-xp 0011d000 03:41 3434007 /lib/tls/libc-2.3.3.so 009c1000-009c3000 rwxp 0011f000 03:41 3434007 /lib/tls/libc-2.3.3.so 009c3000-009c5000 rwxp 009c3000 00:00 0 08048000-08049000 r-xp 00000000 03:41 1046974 /home/mingo/noexec/test-stack 08049000-0804a000 rwxp 00000000 03:41 1046974 /home/mingo/noexec/test-stack b7fe7000-b7fe8000 rwxp b7fe7000 00:00 0 bffeb000-c0000000 rwxp bffeb000 00:00 0 ffffe000-fffff000 ---p 00000000 00:00 0 i.e. all mappings are executable (i.e. READ_IMPLIES_EXEC effect) - the intended change. (although i dont fully agree with PT_GNU_STACK being about something else than the stack, from a security POV if the stack is executable then all bets are off anyway. The heap and all mmaps being executable too in that case makes little difference.) Ingo ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 12:45 ` Ingo Molnar @ 2005-02-06 12:50 ` Andi Kleen 2005-02-06 12:59 ` Arjan van de Ven 0 siblings, 1 reply; 36+ messages in thread From: Andi Kleen @ 2005-02-06 12:50 UTC (permalink / raw) To: Ingo Molnar Cc: Arjan van de Ven, Andi Kleen, akpm, torvalds, linux-kernel, drepper > i.e. all mappings are executable (i.e. READ_IMPLIES_EXEC effect) - the > intended change. (although i dont fully agree with PT_GNU_STACK being > about something else than the stack, from a security POV if the stack is > executable then all bets are off anyway. The heap and all mmaps being > executable too in that case makes little difference.) Well, that won't fix mono (and i suspect wine) and the others who don't use trampolines that the compiler can detect. And breaking programs silently definitely doesn't make them secure! -Andi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 12:50 ` Andi Kleen @ 2005-02-06 12:59 ` Arjan van de Ven 2005-02-06 13:01 ` Andi Kleen 0 siblings, 1 reply; 36+ messages in thread From: Arjan van de Ven @ 2005-02-06 12:59 UTC (permalink / raw) To: Andi Kleen; +Cc: Ingo Molnar, akpm, torvalds, linux-kernel, drepper On Sun, 2005-02-06 at 13:50 +0100, Andi Kleen wrote: > > i.e. all mappings are executable (i.e. READ_IMPLIES_EXEC effect) - the > > intended change. (although i dont fully agree with PT_GNU_STACK being > > about something else than the stack, from a security POV if the stack is > > executable then all bets are off anyway. The heap and all mmaps being > > executable too in that case makes little difference.) > > Well, that won't fix mono correct, http://lists.ximian.com/archives/public/mono-list/2004-June/021592.html that fixes mono instead > (and i suspect wine) and the others wine is ok, it uses PROT_EXEC correctly for things it's not sure about. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 12:59 ` Arjan van de Ven @ 2005-02-06 13:01 ` Andi Kleen 2005-02-06 13:04 ` Arjan van de Ven 2005-02-06 13:06 ` Christoph Hellwig 0 siblings, 2 replies; 36+ messages in thread From: Andi Kleen @ 2005-02-06 13:01 UTC (permalink / raw) To: Arjan van de Ven Cc: Andi Kleen, Ingo Molnar, akpm, torvalds, linux-kernel, drepper > correct, > http://lists.ximian.com/archives/public/mono-list/2004-June/021592.html > > that fixes mono instead Silent breakage => bad. > > > (and i suspect wine) and the others > > wine is ok, it uses PROT_EXEC correctly for things it's not sure about. Hmm, I got a report that it doesn't work anymore with 2.6.11rcs on x86-64. I haven't looked closely yet, but it wouldn't surprise me if this change isn't also involved. -Andi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 13:01 ` Andi Kleen @ 2005-02-06 13:04 ` Arjan van de Ven 2005-02-06 13:09 ` Andi Kleen 2005-02-06 13:06 ` Christoph Hellwig 1 sibling, 1 reply; 36+ messages in thread From: Arjan van de Ven @ 2005-02-06 13:04 UTC (permalink / raw) To: Andi Kleen; +Cc: Ingo Molnar, akpm, torvalds, linux-kernel, drepper > Hmm, I got a report that it doesn't work anymore with > 2.6.11rcs on x86-64. I haven't looked closely yet, > but it wouldn't surprise me if this change isn't also involved. PT_GNU_STACK change is there since like 2.6.6 (and was put in by a suse person) To me that is a strong indication that you are wrong on your suspicion... ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 13:04 ` Arjan van de Ven @ 2005-02-06 13:09 ` Andi Kleen 2005-02-06 13:31 ` Ingo Molnar 0 siblings, 1 reply; 36+ messages in thread From: Andi Kleen @ 2005-02-06 13:09 UTC (permalink / raw) To: Arjan van de Ven Cc: Andi Kleen, Ingo Molnar, akpm, torvalds, linux-kernel, drepper On Sun, Feb 06, 2005 at 02:04:57PM +0100, Arjan van de Ven wrote: > > > Hmm, I got a report that it doesn't work anymore with > > 2.6.11rcs on x86-64. I haven't looked closely yet, > > but it wouldn't surprise me if this change isn't also involved. > > PT_GNU_STACK change is there since like 2.6.6 (and was put in by a suse person) > To me that is a strong indication that you are wrong on your > suspicion... Nah, the change to break 32bit userland mmap/mprotect like this was only put in after 2.6.10. 64bit userland on the other hand always honored PROT_EXEC, but before PT_GNU_STACK it would run by default with executable stack. Hmm, I admit that my patch will break this case too, but fixing that (setting READ_IMPLIES_EXEC only for 32bit) would require more complicated changes that may not be appropiate for 2.6.11. If it's deemed appropiate I could do such a patch too though. -Andi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 13:09 ` Andi Kleen @ 2005-02-06 13:31 ` Ingo Molnar 2005-02-06 13:43 ` Andi Kleen 0 siblings, 1 reply; 36+ messages in thread From: Ingo Molnar @ 2005-02-06 13:31 UTC (permalink / raw) To: Andi Kleen; +Cc: Arjan van de Ven, akpm, torvalds, linux-kernel, drepper * Andi Kleen <ak@suse.de> wrote: > On Sun, Feb 06, 2005 at 02:04:57PM +0100, Arjan van de Ven wrote: > > > > > Hmm, I got a report that it doesn't work anymore with > > > 2.6.11rcs on x86-64. I haven't looked closely yet, > > > but it wouldn't surprise me if this change isn't also involved. > > > > PT_GNU_STACK change is there since like 2.6.6 (and was put in by a suse person) > > To me that is a strong indication that you are wrong on your > > suspicion... > > Nah, the change to break 32bit userland mmap/mprotect like this was only > put in after 2.6.10. i suspect there may be some fundamental misunderstanding here. The change to honor PT_GNU_STACK (on 64-bit) was added in April 2004 and appeared in 2.6.6. The change to enforce the protection bits on x86 NX CPUs (32-bit) was added in June 2004 and appeared in 2.6.8. I.e. it's been part of the upstream kernel for more than half a year. Try it and boot 2.6.8, you'll get NX protection. (I'm not sure about when it was enabled for 32-bit emulation on the x64 kernel, you should be the one to know that - but IIRC it was enabled prior 2.6.10.) so i think you are on to the wrong victim :-) Ingo ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 13:31 ` Ingo Molnar @ 2005-02-06 13:43 ` Andi Kleen 0 siblings, 0 replies; 36+ messages in thread From: Andi Kleen @ 2005-02-06 13:43 UTC (permalink / raw) To: Ingo Molnar Cc: Andi Kleen, Arjan van de Ven, akpm, torvalds, linux-kernel, drepper On Sun, Feb 06, 2005 at 02:31:33PM +0100, Ingo Molnar wrote: > > * Andi Kleen <ak@suse.de> wrote: > > > On Sun, Feb 06, 2005 at 02:04:57PM +0100, Arjan van de Ven wrote: > > > > > > > Hmm, I got a report that it doesn't work anymore with > > > > 2.6.11rcs on x86-64. I haven't looked closely yet, > > > > but it wouldn't surprise me if this change isn't also involved. > > > > > > PT_GNU_STACK change is there since like 2.6.6 (and was put in by a suse person) > > > To me that is a strong indication that you are wrong on your > > > suspicion... > > > > Nah, the change to break 32bit userland mmap/mprotect like this was only > > put in after 2.6.10. > > i suspect there may be some fundamental misunderstanding here. The > change to honor PT_GNU_STACK (on 64-bit) was added in April 2004 and > appeared in 2.6.6. The change to enforce the protection bits on x86 NX > CPUs (32-bit) was added in June 2004 and appeared in 2.6.8. I.e. it's > been part of the upstream kernel for more than half a year. Try it and > boot 2.6.8, you'll get NX protection. (I'm not sure about when it was > enabled for 32-bit emulation on the x64 kernel, you should be the one to > know that - but IIRC it was enabled prior 2.6.10.) I suspect it was afterwards. All the reports about grub/mono/etc. not working anymore etc. only appear now. It's possible that there wasn't enough testing before or the testing happened with completely different semantics (like Fedora on grub) Anyways, I don't care about the 32bit change much because nobody seems to test that one anyways. Basically all the NX capable machines are 64bit capable and often run 64bit kernels, and even if people run 32bit kernels they tend to run non PAE kernels where the problem is hidden. The main thing what annoys me is that the x86-64 32bit emulation seems to effectively serve as a testing ground for 32bit changes now, because the stuff is not tested on 32bit. And it's already enough work to keep the 32bit emulation working, without having to care about additional issues like this. > so i think you are on to the wrong victim :-) I don't think so, no. Linus, can you please tell me what you prefer? If you think it's better to only disable this on x86-64 I can change that too. But it would be probably the wrong thing to do, because then the same issues will all reappear later. -Andi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 13:01 ` Andi Kleen 2005-02-06 13:04 ` Arjan van de Ven @ 2005-02-06 13:06 ` Christoph Hellwig 2005-02-06 13:11 ` Andi Kleen 1 sibling, 1 reply; 36+ messages in thread From: Christoph Hellwig @ 2005-02-06 13:06 UTC (permalink / raw) To: Andi Kleen Cc: Arjan van de Ven, Ingo Molnar, akpm, torvalds, linux-kernel, drepper On Sun, Feb 06, 2005 at 02:01:52PM +0100, Andi Kleen wrote: > > correct, > > http://lists.ximian.com/archives/public/mono-list/2004-June/021592.html > > > > that fixes mono instead > > Silent breakage => bad. silent breakage for newly compiled buggty and non-portable code. Still not nice but certainly tolerable. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 13:06 ` Christoph Hellwig @ 2005-02-06 13:11 ` Andi Kleen 2005-02-06 13:32 ` Ingo Molnar 0 siblings, 1 reply; 36+ messages in thread From: Andi Kleen @ 2005-02-06 13:11 UTC (permalink / raw) To: Christoph Hellwig, Andi Kleen, Arjan van de Ven, Ingo Molnar, akpm, torvalds, linux-kernel, drepper On Sun, Feb 06, 2005 at 01:06:50PM +0000, Christoph Hellwig wrote: > On Sun, Feb 06, 2005 at 02:01:52PM +0100, Andi Kleen wrote: > > > correct, > > > http://lists.ximian.com/archives/public/mono-list/2004-June/021592.html > > > > > > that fixes mono instead > > > > Silent breakage => bad. > > silent breakage for newly compiled buggty and non-portable code. Executing custom code in mmap is by definition non portable, so this argument doesn't make very much sense. > > Still not nice but certainly tolerable. I strongly disagree that breaking source level compatibility silently like this is tolerable. Especially since it won't even affect most users, so most developers won't notice it, only the x86-64 users. This makes it extremly silent for most people. -Andi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 13:11 ` Andi Kleen @ 2005-02-06 13:32 ` Ingo Molnar 2005-02-06 13:46 ` Andi Kleen 0 siblings, 1 reply; 36+ messages in thread From: Ingo Molnar @ 2005-02-06 13:32 UTC (permalink / raw) To: Andi Kleen Cc: Christoph Hellwig, Arjan van de Ven, akpm, torvalds, linux-kernel, drepper * Andi Kleen <ak@suse.de> wrote: > On Sun, Feb 06, 2005 at 01:06:50PM +0000, Christoph Hellwig wrote: > > On Sun, Feb 06, 2005 at 02:01:52PM +0100, Andi Kleen wrote: > > > > correct, > > > > http://lists.ximian.com/archives/public/mono-list/2004-June/021592.html > > > > > > > > that fixes mono instead > > > > > > Silent breakage => bad. > > > > silent breakage for newly compiled buggty and non-portable code. > > Executing custom code in mmap is by definition non portable, > so this argument doesn't make very much sense. > > > > > Still not nice but certainly tolerable. > > I strongly disagree that breaking source level compatibility silently > like this is tolerable. > > Especially since it won't even affect most users, so most developers > won't notice it, only the x86-64 users. This makes it extremly silent > for most people. fortunately there's this 'NX-emulation' thing called exec-shield which is part of Fedora (and has been part of it for almost 2 years) and did all the testing for you on all x86 hardware, on thousands of packages and on over a ten thousand binaries, well in advance of this going upstream. It wasnt a bump-free ride, but it was worth it. (the Mono bug was found this way, the Grub one wasnt, due to it being RWE. But if it triggers it shows up immediately so it's not like you have no sign that something wrong is going on. Only grub-install triggers it and no boot/install kernel i know of defaults to PAE-enabled, that's what caused grub-install being used in an NX scenario so infrequently.) anyway, this particular flamewar might have made more sense last Summer. Ingo ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 13:32 ` Ingo Molnar @ 2005-02-06 13:46 ` Andi Kleen 2005-02-06 14:08 ` Ingo Molnar 0 siblings, 1 reply; 36+ messages in thread From: Andi Kleen @ 2005-02-06 13:46 UTC (permalink / raw) To: Ingo Molnar Cc: Andi Kleen, Christoph Hellwig, Arjan van de Ven, akpm, torvalds, linux-kernel, drepper > fortunately there's this 'NX-emulation' thing called exec-shield which > is part of Fedora (and has been part of it for almost 2 years) and did > all the testing for you on all x86 hardware, on thousands of packages > and on over a ten thousand binaries, well in advance of this going > upstream. It wasnt a bump-free ride, but it was worth it. But apparently a completely different patch was tested with quite different semantics. > > (the Mono bug was found this way, the Grub one wasnt, due to it being But not fixed in mainline mono. And also as far as I can figure out there was no effort to warn user land people about this intrusive change, and tell them what they need to do when they recompile. > RWE. But if it triggers it shows up immediately so it's not like you > have no sign that something wrong is going on. Only grub-install > triggers it and no boot/install kernel i know of defaults to > PAE-enabled, that's what caused grub-install being used in an NX > scenario so infrequently.) > > anyway, this particular flamewar might have made more sense last Summer. Last summer nobody did change the 32bit ABI on x86-64. I only started it because the bug reports are appearing now and it's clear now that we have a problem. -Andi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 13:46 ` Andi Kleen @ 2005-02-06 14:08 ` Ingo Molnar 2005-02-06 14:22 ` Ingo Molnar 2005-02-06 14:29 ` Andi Kleen 0 siblings, 2 replies; 36+ messages in thread From: Ingo Molnar @ 2005-02-06 14:08 UTC (permalink / raw) To: Andi Kleen Cc: Christoph Hellwig, Arjan van de Ven, akpm, torvalds, linux-kernel, drepper * Andi Kleen <ak@suse.de> wrote: > > RWE. But if it triggers it shows up immediately so it's not like you > > have no sign that something wrong is going on. Only grub-install > > triggers it and no boot/install kernel i know of defaults to > > PAE-enabled, that's what caused grub-install being used in an NX > > scenario so infrequently.) > > > > anyway, this particular flamewar might have made more sense last Summer. > > Last summer nobody did change the 32bit ABI on x86-64. > > I only started it because the bug reports are appearing now and it's > clear now that we have a problem. the vanilla 2.6.10 x64 kernel, using 32-bit fedora userland boots fine here, and gives a noexec stack: $ ./test-stack Segmentation fault $ $ uname -a Linux saturn 2.6.10 #1 Sun Feb 6 15:52:44 CET 2005 x86_64 x86_64 x86_64 GNU/Linux $ $ readelf -l test-stack | grep STA GNU_STACK 0x000000 0x00000000 0x00000000 0x00000 0x00000 RW 0x4 Ingo ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 14:08 ` Ingo Molnar @ 2005-02-06 14:22 ` Ingo Molnar 2005-02-06 14:29 ` Andi Kleen 1 sibling, 0 replies; 36+ messages in thread From: Ingo Molnar @ 2005-02-06 14:22 UTC (permalink / raw) To: Andi Kleen Cc: Christoph Hellwig, Arjan van de Ven, akpm, torvalds, linux-kernel, drepper * Ingo Molnar <mingo@elte.hu> wrote: > > Last summer nobody did change the 32bit ABI on x86-64. > > > > I only started it because the bug reports are appearing now and it's > > clear now that we have a problem. > > the vanilla 2.6.10 x64 kernel, using 32-bit fedora userland boots fine > here, and gives a noexec stack: same with SuSE 9.1 32-bit user-space running a vanilla 2.6.10 x64 kernel - PT_GNU_STACK is honored and the stack is noexec. Ingo ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 14:08 ` Ingo Molnar 2005-02-06 14:22 ` Ingo Molnar @ 2005-02-06 14:29 ` Andi Kleen 2005-02-06 17:08 ` Linus Torvalds 1 sibling, 1 reply; 36+ messages in thread From: Andi Kleen @ 2005-02-06 14:29 UTC (permalink / raw) To: Ingo Molnar Cc: Andi Kleen, Christoph Hellwig, Arjan van de Ven, akpm, torvalds, linux-kernel, drepper On Sun, Feb 06, 2005 at 03:08:02PM +0100, Ingo Molnar wrote: > * Andi Kleen <ak@suse.de> wrote: > > > > RWE. But if it triggers it shows up immediately so it's not like you > > > have no sign that something wrong is going on. Only grub-install > > > triggers it and no boot/install kernel i know of defaults to > > > PAE-enabled, that's what caused grub-install being used in an NX > > > scenario so infrequently.) > > > > > > anyway, this particular flamewar might have made more sense last Summer. > > > > Last summer nobody did change the 32bit ABI on x86-64. > > > > I only started it because the bug reports are appearing now and it's > > clear now that we have a problem. > > the vanilla 2.6.10 x64 kernel, using 32-bit fedora userland boots fine > here, and gives a noexec stack: I'm not too concerned about the noexec stack (PT_GNU_STACK seems to handle that well enough), more about the no exec heap which seems to cause all the problems. The stack change is not very nice, but at least doesn't seem to cause wide spread breakage. Anyways, you're right I double checked and the problem was already in 2.6.10. I can just say that I didn't get any reports, maybe nobody tried grub with 2.6.10 or they didn't report it the problem properly. Probably my original patch was a bit too radical too, just setting READ_IMPLIES_EXEC on all 32bit process unconditionally should be good enough. I still think it's something that needs to be addressed for 2.6.11. Here's a new patch that just sets READ_IMPLIES_EXEC unconditionally. It still has a bug that you cannot change it with personality() without getting overwritten later (that is a bug that was in the original changes, you would really need two independent bits for this). But I'm not going to change that now so late in the game. It implies that you cannot use personality to force READ_IMPLIES_EXEC for a 64bit process. -Andi Force READ_IMPLIES_EXEC for all 32bit processes to fix the 32bit source compatibility. Signed-off-by: Andi Kleen <ak@suse.de> diff -u linux-2.6.11rc3/include/asm-i386/elf.h-o linux-2.6.11rc3/include/asm-i386/elf.h --- linux-2.6.11rc3/include/asm-i386/elf.h-o 2004-10-19 01:55:33.000000000 +0200 +++ linux-2.6.11rc3/include/asm-i386/elf.h 2005-02-06 15:27:13.000000000 +0100 @@ -117,7 +117,8 @@ #define AT_SYSINFO_EHDR 33 #ifdef __KERNEL__ -#define SET_PERSONALITY(ex, ibcs2) do { } while (0) +#define SET_PERSONALITY(ex, ibcs2) \ + do { current->personality |= READ_IMPLIES_EXEC; } while (0) /* * An executable for which elf_read_implies_exec() returns TRUE will diff -u linux-2.6.11rc3/arch/x86_64/kernel/process.c-o linux-2.6.11rc3/arch/x86_64/kernel/process.c --- linux-2.6.11rc3/arch/x86_64/kernel/process.c-o 2005-02-04 09:12:52.000000000 +0100 +++ linux-2.6.11rc3/arch/x86_64/kernel/process.c 2005-02-06 15:26:45.000000000 +0100 @@ -577,6 +577,11 @@ /* Make sure to be in 64bit mode */ clear_thread_flag(TIF_IA32); + + /* Clear in case it was set from a 32bit parent. + Bug: this overwrites the user choice. Would need + a second bit too. */ + current->personality &= ~READ_IMPLIES_EXEC; } asmlinkage long sys_fork(struct pt_regs *regs) diff -u linux-2.6.11rc3/arch/x86_64/ia32/ia32_binfmt.c-o linux-2.6.11rc3/arch/x86_64/ia32/ia32_binfmt.c --- linux-2.6.11rc3/arch/x86_64/ia32/ia32_binfmt.c-o 2005-02-04 09:12:52.000000000 +0100 +++ linux-2.6.11rc3/arch/x86_64/ia32/ia32_binfmt.c 2005-02-06 15:23:33.000000000 +0100 @@ -262,6 +262,7 @@ set_thread_flag(TIF_ABI_PENDING); \ else \ clear_thread_flag(TIF_ABI_PENDING); \ + current->personality |= READ_IMPLIES_EXEC; \ } while (0) /* Override some function names */ ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 14:29 ` Andi Kleen @ 2005-02-06 17:08 ` Linus Torvalds 2005-02-06 17:13 ` Arjan van de Ven 2005-02-06 17:56 ` Andi Kleen 0 siblings, 2 replies; 36+ messages in thread From: Linus Torvalds @ 2005-02-06 17:08 UTC (permalink / raw) To: Andi Kleen Cc: Ingo Molnar, Christoph Hellwig, Arjan van de Ven, akpm, linux-kernel, drepper On Sun, 6 Feb 2005, Andi Kleen wrote: > > Force READ_IMPLIES_EXEC for all 32bit processes to fix > the 32bit source compatibility. Andi, stop this. We're _not_ going to say "32-bit executables don't need PROT_EXEC. The executables would need to be marked broken per-executable, not some kind of "we don't do this globally" setting. Linus ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 17:08 ` Linus Torvalds @ 2005-02-06 17:13 ` Arjan van de Ven 2005-02-06 17:31 ` Linus Torvalds 2005-02-06 17:56 ` Andi Kleen 1 sibling, 1 reply; 36+ messages in thread From: Arjan van de Ven @ 2005-02-06 17:13 UTC (permalink / raw) To: Linus Torvalds Cc: Andi Kleen, Ingo Molnar, Christoph Hellwig, akpm, linux-kernel, drepper On Sun, 2005-02-06 at 09:08 -0800, Linus Torvalds wrote: > > On Sun, 6 Feb 2005, Andi Kleen wrote: > > > > Force READ_IMPLIES_EXEC for all 32bit processes to fix > > the 32bit source compatibility. > > Andi, stop this. We're _not_ going to say "32-bit executables don't need > PROT_EXEC. The executables would need to be marked broken per-executable, > not some kind of "we don't do this globally" setting. marking PT_GNU_STACK as RWE would be an acceptable marking imo; one can insert such a marking post build (via the execstack tool), and during the build with either an addition to the cflags or via a one line code addition. In addition there are runtime markings; you can use setarch to start an application with READ-IMPLIES-EXEC set (although you can't do that for setuid binaries for obvious reasons) Note that these techniques all exist today. The only issue is that the current code doesn't do the RWE->READIMPLIESEXEC binding, which my patch fixed. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 17:13 ` Arjan van de Ven @ 2005-02-06 17:31 ` Linus Torvalds 2005-02-06 17:39 ` Arjan van de Ven 0 siblings, 1 reply; 36+ messages in thread From: Linus Torvalds @ 2005-02-06 17:31 UTC (permalink / raw) To: Arjan van de Ven Cc: Andi Kleen, Ingo Molnar, Christoph Hellwig, akpm, linux-kernel, drepper On Sun, 6 Feb 2005, Arjan van de Ven wrote: > > Note that these techniques all exist today. The only issue is that the > current code doesn't do the RWE->READIMPLIESEXEC binding, which my patch > fixed. My main objection to your patch is the naming. If 'executable_stack' affects the heap, then why is it called "executable_STACK"? Wouldn't it be much nicer to - get rid of "EXSTACK_DEFAULT" as a special case, and instead just have the architecture _initialize_ the damn variable to what it wants? In other words, make it a nice understandable binary value (or maybe a bitmask, if you want to have separate flags for stack/heap/mmap), rather than a ternary value where one of the values means something arch-dependent. - just rename the dang thing to "read_implies_exec" and be done with it? Hmm? Wouldn't that make a lot more sense? And if you want to split things up, there's at least three flags there: "stack" vs "file mapping" vs "anonymous mapping". For example, it might well make sense to enforce PROT_EXEC on real file mappings, but not on the stack or heap.. So "read_implies_exec" might well be a collection of bits to enable these one by one (and make the "legacy app" thing make it enable read-implies-exec for all the cases). Linus ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 17:31 ` Linus Torvalds @ 2005-02-06 17:39 ` Arjan van de Ven 2005-02-06 18:04 ` Linus Torvalds 0 siblings, 1 reply; 36+ messages in thread From: Arjan van de Ven @ 2005-02-06 17:39 UTC (permalink / raw) To: Linus Torvalds Cc: Andi Kleen, Ingo Molnar, Christoph Hellwig, akpm, linux-kernel, drepper On Sun, 2005-02-06 at 09:31 -0800, Linus Torvalds wrote: > > On Sun, 6 Feb 2005, Arjan van de Ven wrote: > > > > Note that these techniques all exist today. The only issue is that the > > current code doesn't do the RWE->READIMPLIESEXEC binding, which my patch > > fixed. > > My main objection to your patch is the naming. If 'executable_stack' > affects the heap, then why is it called "executable_STACK"? fair point. > > Wouldn't it be much nicer to > > - get rid of "EXSTACK_DEFAULT" as a special case, and instead just have > the architecture _initialize_ the damn variable to what it wants? In > other words, make it a nice understandable binary value (or maybe a > bitmask, if you want to have separate flags for stack/heap/mmap), > rather than a ternary value where one of the values means something > arch-dependent. > > - just rename the dang thing to "read_implies_exec" and be done with it? > > Hmm? Wouldn't that make a lot more sense? it would. It'll be a bit bigger than I'd be comfortable with this late in the 2.6.11 cycle though. > > And if you want to split things up, there's at least three flags there: > "stack" vs "file mapping" vs "anonymous mapping". For example, it might lets add "brk" as 4th I guess. Ok so what to do for 2.6.11... the setarch workaround is there; that works. My patch patches the worst issues and is quite minimal. What you propose will be more invasive and more suitable for 2.6.11-bk1... I can do such a patch no problem (although the next two days I won't have time). ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 17:39 ` Arjan van de Ven @ 2005-02-06 18:04 ` Linus Torvalds 2005-02-06 18:08 ` Arjan van de Ven 0 siblings, 1 reply; 36+ messages in thread From: Linus Torvalds @ 2005-02-06 18:04 UTC (permalink / raw) To: Arjan van de Ven Cc: Andi Kleen, Ingo Molnar, Christoph Hellwig, akpm, linux-kernel, drepper On Sun, 6 Feb 2005, Arjan van de Ven wrote: > > > > And if you want to split things up, there's at least three flags there: > > "stack" vs "file mapping" vs "anonymous mapping". For example, it might > > lets add "brk" as 4th I guess. I thought about that, but no normal user program uses brk() natively. They all just use "malloc()" and friends, and pretty much every implementation of those in turn just mixes brk/anon-mmap freely. > Ok so what to do for 2.6.11... the setarch workaround is there; that > works. My patch patches the worst issues and is quite minimal. What you > propose will be more invasive and more suitable for 2.6.11-bk1... > I can do such a patch no problem (although the next two days I won't > have time). Hmm.. I can take your initial patch now. Can somebody explain why this hassn't come up before, though? Linus ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 18:04 ` Linus Torvalds @ 2005-02-06 18:08 ` Arjan van de Ven 0 siblings, 0 replies; 36+ messages in thread From: Arjan van de Ven @ 2005-02-06 18:08 UTC (permalink / raw) To: Linus Torvalds Cc: Andi Kleen, Ingo Molnar, Christoph Hellwig, akpm, linux-kernel, drepper On Sun, 2005-02-06 at 10:04 -0800, Linus Torvalds wrote: > > Ok so what to do for 2.6.11... the setarch workaround is there; that > > works. My patch patches the worst issues and is quite minimal. What you > > propose will be more invasive and more suitable for 2.6.11-bk1... > > I can do such a patch no problem (although the next two days I won't > > have time). > > Hmm.. I can take your initial patch now. Can somebody explain why this > hassn't come up before, though? somehow things got "cleaned up" around 2.6.10-ish to change the default... it's quite rare so few people noticed. It seems Andi got a few reports in the last week or so and started investigating. ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 17:08 ` Linus Torvalds 2005-02-06 17:13 ` Arjan van de Ven @ 2005-02-06 17:56 ` Andi Kleen 1 sibling, 0 replies; 36+ messages in thread From: Andi Kleen @ 2005-02-06 17:56 UTC (permalink / raw) To: Linus Torvalds Cc: Andi Kleen, Ingo Molnar, Christoph Hellwig, Arjan van de Ven, akpm, linux-kernel, drepper On Sun, Feb 06, 2005 at 09:08:47AM -0800, Linus Torvalds wrote: > > > On Sun, 6 Feb 2005, Andi Kleen wrote: > > > > Force READ_IMPLIES_EXEC for all 32bit processes to fix > > the 32bit source compatibility. > > Andi, stop this. We're _not_ going to say "32-bit executables don't need > PROT_EXEC. The executables would need to be marked broken per-executable, > not some kind of "we don't do this globally" setting. The thing I'm annoyed about is that all the testing for this change seems to go towards the x86-64 32bit emulation (because effectively near nobody uses 32bit PAE+NX right now) And the main job of the 32bit emulation is not to prove as a testing ground for experimental stuff, but to be compatible. And changes like this break it and cause me a lot of additional work. Here's a slightly different patch to only turn it off for 32bit x86-64. If the 32bit experimental security people can get their stuff tested properly and 32bit NX CPUs are actually used widely and all the third party sources fixed I can probably follow in a few months. But I really don't have the capacity to track third party software fixes for stuff that really has nothing to do with compatible 32bit emulation. Please consider applying this patch. It only touches x86-64. Thanks: -Andi Always enable PROT_READ implies PROT_EXEC for 32bit programs running on x86-64. This reverts behaviour back to what 2.6.9 did. Signed-off-by: Andi Kleen <ak@suse.de> diff -u linux-2.6.11rc3/arch/x86_64/kernel/process.c-o linux-2.6.11rc3/arch/x86_64/kernel/process.c --- linux-2.6.11rc3/arch/x86_64/kernel/process.c-o 2005-02-04 09:12:52.000000000 +0100 +++ linux-2.6.11rc3/arch/x86_64/kernel/process.c 2005-02-06 15:26:45.000000000 +0100 @@ -577,6 +577,11 @@ /* Make sure to be in 64bit mode */ clear_thread_flag(TIF_IA32); + + /* Clear in case it was set from a 32bit parent. + Bug: this overwrites the user choice. Would need + a second bit too. */ + current->personality &= ~READ_IMPLIES_EXEC; } asmlinkage long sys_fork(struct pt_regs *regs) diff -u linux-2.6.11rc3/arch/x86_64/ia32/ia32_binfmt.c-o linux-2.6.11rc3/arch/x86_64/ia32/ia32_binfmt.c --- linux-2.6.11rc3/arch/x86_64/ia32/ia32_binfmt.c-o 2005-02-04 09:12:52.000000000 +0100 +++ linux-2.6.11rc3/arch/x86_64/ia32/ia32_binfmt.c 2005-02-06 15:23:33.000000000 +0100 @@ -262,6 +262,7 @@ set_thread_flag(TIF_ABI_PENDING); \ else \ clear_thread_flag(TIF_ABI_PENDING); \ + current->personality |= READ_IMPLIES_EXEC; \ } while (0) /* Override some function names */ ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 11:47 ` Arjan van de Ven 2005-02-06 12:02 ` Ingo Molnar @ 2005-02-06 12:33 ` Andi Kleen 2005-02-06 12:40 ` Arjan van de Ven 2005-02-06 17:05 ` Linus Torvalds 1 sibling, 2 replies; 36+ messages in thread From: Andi Kleen @ 2005-02-06 12:33 UTC (permalink / raw) To: Arjan van de Ven; +Cc: Andi Kleen, akpm, torvalds, mingo, linux-kernel, drepper > Your main objection is that *incorrect* programs that assume they can > execute malloc() code without PROT_EXEC protection. For legacy binaries > keeping this behavior makes sense, no objection from me. > > For newly compiled programs this is just wrong and incorrect. That's not true as the grub/mono/... experience shows. > You mention grub (which has RWE and the patch below thus makes that work) > and mono. mono has patches for this on their mailinglist and bugzilla since > 2003 according to google, I'm surprised the novell mono guys haven't fixed > this bug in their code. There are probably more. > FWIW all jvm's don't suffer from this. They are either legacy binaries or > mprotect properly (only i386 traditionally had this behavior, all others > already required PROT_EXEC anyway so any half portable app already did this, > as well as any app portable to BSD since they enforce this on x86 as well) > > So I rather see the patch below merged instead; it fixes the worst problems > (RWE not marking the heap executable) while keeping this useful feature > enabled. It still keeps x86-64 32bit emulation as a proving point for experimental i386 features which is really not its purpose. -Andi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 12:33 ` Andi Kleen @ 2005-02-06 12:40 ` Arjan van de Ven 2005-02-06 12:48 ` Andi Kleen 2005-02-06 17:05 ` Linus Torvalds 1 sibling, 1 reply; 36+ messages in thread From: Arjan van de Ven @ 2005-02-06 12:40 UTC (permalink / raw) To: Andi Kleen; +Cc: akpm, torvalds, mingo, linux-kernel, drepper On Sun, 2005-02-06 at 13:33 +0100, Andi Kleen wrote: > > Your main objection is that *incorrect* programs that assume they can > > execute malloc() code without PROT_EXEC protection. For legacy binaries > > keeping this behavior makes sense, no objection from me. > > > > For newly compiled programs this is just wrong and incorrect. > > That's not true as the grub/mono/... experience shows. both those apps are buggy and incorrect though and should be fixed. > > You mention grub (which has RWE and the patch below thus makes that work) > > and mono. mono has patches for this on their mailinglist and bugzilla since > > 2003 according to google, I'm surprised the novell mono guys haven't fixed > > this bug in their code. > > There are probably more. I consider that unlikely. Anything remotely portable (either to other architectures which required since in linux since day one, or to OpenBSD or similar which require this even for x86) already is correct. And this is there since 2.6.6.. it's not like this is new (and for the non-RWE case, fedora has had this behavior all along with execshield, and that even lead to a patch to fix mono fwiw) ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 12:40 ` Arjan van de Ven @ 2005-02-06 12:48 ` Andi Kleen 2005-02-06 15:54 ` Andreas Schwab 0 siblings, 1 reply; 36+ messages in thread From: Andi Kleen @ 2005-02-06 12:48 UTC (permalink / raw) To: Arjan van de Ven; +Cc: Andi Kleen, akpm, torvalds, mingo, linux-kernel, drepper On Sun, Feb 06, 2005 at 01:40:22PM +0100, Arjan van de Ven wrote: > On Sun, 2005-02-06 at 13:33 +0100, Andi Kleen wrote: > > > Your main objection is that *incorrect* programs that assume they can > > > execute malloc() code without PROT_EXEC protection. For legacy binaries > > > keeping this behavior makes sense, no objection from me. > > > > > > For newly compiled programs this is just wrong and incorrect. > > > > That's not true as the grub/mono/... experience shows. > > both those apps are buggy and incorrect though and should be fixed. They worked fine forever - and suddenly you define them as buggy. This might be, but it's still quite bad to change the definition of what is buggy and what is not so suddenly in a "mostly stable" release series. And who tells the users what is considered buggy this week? -Andi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 12:48 ` Andi Kleen @ 2005-02-06 15:54 ` Andreas Schwab 0 siblings, 0 replies; 36+ messages in thread From: Andreas Schwab @ 2005-02-06 15:54 UTC (permalink / raw) To: Andi Kleen; +Cc: Arjan van de Ven, akpm, torvalds, mingo, linux-kernel, drepper Andi Kleen <ak@suse.de> writes: > They worked fine forever - and suddenly you define them as buggy. Working fine does not imply non-buggy, never has, never will. Andreas. -- Andreas Schwab, SuSE Labs, schwab@suse.de SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 12:33 ` Andi Kleen 2005-02-06 12:40 ` Arjan van de Ven @ 2005-02-06 17:05 ` Linus Torvalds 2005-02-06 17:58 ` Andi Kleen 1 sibling, 1 reply; 36+ messages in thread From: Linus Torvalds @ 2005-02-06 17:05 UTC (permalink / raw) To: Andi Kleen; +Cc: Arjan van de Ven, akpm, mingo, linux-kernel, drepper On Sun, 6 Feb 2005, Andi Kleen wrote: > > There are probably more. So? Do you expect to never fix them, or what? The fact is, a program that tries to execute without using PROT_EXEC is a buggy program. Are there buggy programs out there? Yes. We should fix them. Linus ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 17:05 ` Linus Torvalds @ 2005-02-06 17:58 ` Andi Kleen 0 siblings, 0 replies; 36+ messages in thread From: Andi Kleen @ 2005-02-06 17:58 UTC (permalink / raw) To: Linus Torvalds Cc: Andi Kleen, Arjan van de Ven, akpm, mingo, linux-kernel, drepper On Sun, Feb 06, 2005 at 09:05:05AM -0800, Linus Torvalds wrote: > > > On Sun, 6 Feb 2005, Andi Kleen wrote: > > > > There are probably more. > > So? Do you expect to never fix them, or what? Someone will fix them, but it's not the job of the 32bit emulation of x86-64 to break compatibility. It's whole point is to be compatible, not expose bugs. If someone else wants to break existing programs they can do that if they think they have the capability to deal with (rightfully) annoyed user space people. But not with x86-64 compat mode leading the front here. -Andi ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 11:36 [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 Andi Kleen 2005-02-06 11:47 ` Arjan van de Ven @ 2005-02-06 12:11 ` Paweł Sikora [not found] ` <200502061303.12377.pluto@pld-linux.org> 2 siblings, 0 replies; 36+ messages in thread From: Paweł Sikora @ 2005-02-06 12:11 UTC (permalink / raw) To: akpm, torvalds, mingo, linux-kernel My proposal is to recompile broken software with cflags += -Wa,--noexecstack. This works fine with e.g. 2.6.10+pax/i386. -- /* Copyright (C) 2003, SCO, Inc. This is valuable Intellectual Property. */ #define say(x) lie(x) ^ permalink raw reply [flat|nested] 36+ messages in thread
[parent not found: <200502061303.12377.pluto@pld-linux.org>]
[parent not found: <20050206124701.GD30109@wotan.suse.de>]
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 [not found] ` <20050206124701.GD30109@wotan.suse.de> @ 2005-02-06 18:07 ` Paweł Sikora 2005-02-06 18:38 ` Andi Kleen 0 siblings, 1 reply; 36+ messages in thread From: Paweł Sikora @ 2005-02-06 18:07 UTC (permalink / raw) To: Andi Kleen; +Cc: linux-kernel [-- Attachment #1: Type: text/plain, Size: 2384 bytes --] On Sunday 06 of February 2005 13:47, you wrote: > On Sun, Feb 06, 2005 at 01:03:11PM +0100, Pawel Sikora wrote: > > On Sunday 06 of February 2005 12:36, you wrote: > > > Worse is that even when the program has trampolines and has > > > PT_GNU_STACK header with an E bit on the stack it still won't get an > > > executable heap by default (this is what broke grub) > > > (...) > > > My proposal is to turn this all off at least for 2.6.11. > > > > My proposal is to recompile broken software with cflags += > > -Wa,--noexecstack > > By how do you detect broken software? There doesn't seem to be any > fool proof way other than a extensive test on a NX capable system > with correct kernel. [1] glibc-2.3.4 kill buggy bins at the load time. (please look into: elf/dl-load.c, elf/dl-support.c, elf/rtld.c) This works on i386/PaX systems too (hardware NX isn't required). [2] `readelf -Wl |grep GNU_STACK` shows RWE ;-) Please look at this quick example. # gcc tmp1.c tmp2-invalid.S -o tmp -s # readelf -Wl tmp (...) GNU_STACK 0x000000 0x00000000 0x00000000 0x00000 0x00000 RWE 0x4 ^ execstack? PAX_FLAGS 0x000000 0x00000000 0x00000000 0x00000 0x00000 0x4 (...) Now, let's add section note to the asm. file and rebuild. # gcc tmp1.c tmp2-valid.S -o tmp -s # readelf -Wl tmp (...) GNU_STACK 0x000000 0x00000000 0x00000000 0x00000 0x00000 RW 0x4 PAX_FLAGS 0x000000 0x00000000 0x00000000 0x00000 0x00000 0x4 (...) The execstack req. disappeard (~99% of broken sources). I get the same effect with fixed cflags and invalid source. # gcc tmp1.c tmp2-invalid.S -o tmp -s -Wa,--noexecstack # readelf -Wl tmp (...) GNU_STACK 0x000000 0x00000000 0x00000000 0x00000 0x00000 RW 0x4 PAX_FLAGS 0x000000 0x00000000 0x00000000 0x00000 0x00000 0x4 (...) I known several apps that really need executable data/stack (eg. jvm, xorg). The rest of RWE-marked binaries have IMHO buggy sources. > It would be fine if there was a compile time error or something, > but there isn't. IMHO the `as` should warn about missed (.note.GNU-stack) section. Regards, Paweł. -- /* Copyright (C) 2003, SCO, Inc. This is valuable Intellectual Property. */ #define say(x) lie(x) [-- Attachment #2: tmp1.c --] [-- Type: text/x-csrc, Size: 83 bytes --] extern void test(); int main(int argc, char** argv) { test(); return 0; } [-- Attachment #3: tmp2-invalid.S --] [-- Type: text/plain, Size: 50 bytes --] .text .global test test: ret .end [-- Attachment #4: tmp2-valid.S --] [-- Type: text/plain, Size: 103 bytes --] .section .note.GNU-stack,"",@progbits; .previous .text .global test test: ret .end ^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 2005-02-06 18:07 ` Paweł Sikora @ 2005-02-06 18:38 ` Andi Kleen 0 siblings, 0 replies; 36+ messages in thread From: Andi Kleen @ 2005-02-06 18:38 UTC (permalink / raw) To: Pawe?? Sikora; +Cc: Andi Kleen, linux-kernel > [1] glibc-2.3.4 kill buggy bins at the load time. > (please look into: elf/dl-load.c, elf/dl-support.c, elf/rtld.c) I don't see how that can work for arbitary code executed in some arbitary mmap. Please explain. > This works on i386/PaX systems too (hardware NX isn't required). But it's for the 99.99999% of other users who don't use such weird third party patches. > The execstack req. disappeard (~99% of broken sources). My problem is basically that the effort of fixing these sources seems to be shifted to x86-64 now in mainstream linux. And I don't like that. -Andi ^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2005-02-06 18:40 UTC | newest]
Thread overview: 36+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-02-06 11:36 [PROPOSAL/PATCH] Remove PT_GNU_STACK support before 2.6.11 Andi Kleen
2005-02-06 11:47 ` Arjan van de Ven
2005-02-06 12:02 ` Ingo Molnar
2005-02-06 12:25 ` Ingo Molnar
2005-02-06 12:36 ` Andi Kleen
2005-02-06 12:45 ` Ingo Molnar
2005-02-06 12:50 ` Andi Kleen
2005-02-06 12:59 ` Arjan van de Ven
2005-02-06 13:01 ` Andi Kleen
2005-02-06 13:04 ` Arjan van de Ven
2005-02-06 13:09 ` Andi Kleen
2005-02-06 13:31 ` Ingo Molnar
2005-02-06 13:43 ` Andi Kleen
2005-02-06 13:06 ` Christoph Hellwig
2005-02-06 13:11 ` Andi Kleen
2005-02-06 13:32 ` Ingo Molnar
2005-02-06 13:46 ` Andi Kleen
2005-02-06 14:08 ` Ingo Molnar
2005-02-06 14:22 ` Ingo Molnar
2005-02-06 14:29 ` Andi Kleen
2005-02-06 17:08 ` Linus Torvalds
2005-02-06 17:13 ` Arjan van de Ven
2005-02-06 17:31 ` Linus Torvalds
2005-02-06 17:39 ` Arjan van de Ven
2005-02-06 18:04 ` Linus Torvalds
2005-02-06 18:08 ` Arjan van de Ven
2005-02-06 17:56 ` Andi Kleen
2005-02-06 12:33 ` Andi Kleen
2005-02-06 12:40 ` Arjan van de Ven
2005-02-06 12:48 ` Andi Kleen
2005-02-06 15:54 ` Andreas Schwab
2005-02-06 17:05 ` Linus Torvalds
2005-02-06 17:58 ` Andi Kleen
2005-02-06 12:11 ` Paweł Sikora
[not found] ` <200502061303.12377.pluto@pld-linux.org>
[not found] ` <20050206124701.GD30109@wotan.suse.de>
2005-02-06 18:07 ` Paweł Sikora
2005-02-06 18:38 ` Andi Kleen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox