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 <[email protected]>
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) {
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 <[email protected]>
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 *);
* Arjan van de Ven <[email protected]> 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 <[email protected]>
looks good.
Signed-off-by: Ingo Molnar <[email protected]>
(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
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)
* Ingo Molnar <[email protected]> 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 <[email protected]>
>
> looks good.
>
> Signed-off-by: Ingo Molnar <[email protected]>
>
> (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
> 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
> Signed-off-by: Ingo Molnar <[email protected]>
>
> (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
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)
* Ingo Molnar <[email protected]> wrote:
> * Arjan van de Ven <[email protected]> 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 <[email protected]>
>
> looks good.
>
> Signed-off-by: Ingo Molnar <[email protected]>
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
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
> 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
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.
> 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
> 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...
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.
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
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
* Andi Kleen <[email protected]> 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
* Andi Kleen <[email protected]> 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
On Sun, Feb 06, 2005 at 02:31:33PM +0100, Ingo Molnar wrote:
>
> * Andi Kleen <[email protected]> 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
> 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
* Andi Kleen <[email protected]> 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
* Ingo Molnar <[email protected]> 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
On Sun, Feb 06, 2005 at 03:08:02PM +0100, Ingo Molnar wrote:
> * Andi Kleen <[email protected]> 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 <[email protected]>
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 */
Andi Kleen <[email protected]> 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, [email protected]
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."
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
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
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.
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
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).
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 <[email protected]>
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 */
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
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
.section .note.GNU-stack,"",@progbits; .previous
.text
.global test
test:
ret
.end
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.
> [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