Re:[PATCH] Set __libc_stack_end earlier on all arches,...

Re:[PATCH] Set __libc_stack_end earlier on all arches,...

[Steven Munroe]

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

Jakub Jelinek Wed, 24 Sep 2003 15:47:04 writes:

> The following patch does that (tested on a bunch of arches).

Actually it does not include PowerPC 32/64. Currently for PowerPC linux
__libc_stack_end is set in libc_start_main, but this is not soon enough to
support execstack and the execstack tests in make check fails (because
__libc_stack_end is not set). The attached patch uses DL_PLATFORM_INIT to
capture the argv pointer and store that into __libc_stack_end during
_dl_sysdep_start before the call to dl_main. This will work for both powerpc32
and powerpc64.

This is soon enough to pass the stack-end into _dl_make_stack_executable but it
still fails in the mprotect with a "Permission Denied". I'll pursue that issue
further.

I would like an explanation of why we are doing this. Generally arbitrarily
marking the stack executable is a bad idea. So I would like a explaination
(pointer to one) of the function we trying to provide and the conditions under
which it will be used.

2003-09-24  Steven Munroe  <sjmunroe@us.ibm.com>

	* sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c 
	[DL_PLATFORM_INIT]: Define
	(frob_stack_end): New function.



-- 
Steven Munroe
sjmunroe@us.ibm.com
Linux on PowerPC-64 Development
GLIBC for PowerPC-64 Development

[-- Attachment #2: ppc-dl-sysdep-20030923.patch --]
[-- Type: text/plain, Size: 832 bytes --]

diff -urN libc23-cvstip-20030923/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c libc23/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c
--- libc23-cvstip-20030923/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c	2003-03-15 18:40:44.000000000 -0600
+++ libc23/sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c	2003-09-24 13:19:34.000000000 -0500
@@ -25,6 +25,16 @@
 extern int __cache_line_size;
 weak_extern (__cache_line_size)
 
+#define DL_PLATFORM_INIT frob_stack_end (start_argptr)
+
+extern void *__libc_stack_end;
+
+static inline void
+frob_stack_end (void *arg)
+{
+  __libc_stack_end = arg;	/* Initialize the break.  */
+}
+
 /* Scan the Aux Vector for the "Data Cache Block Size" entry.  If found
    verify that the static extern __cache_line_size is defined by checking
    for not NULL.  If it is defined then assign the cache block size

Re: [PATCH] Set __libc_stack_end earlier on all arches,...

[Ulrich Drepper]

Steven Munroe wrote:

> I would like an explanation of why we are doing this. Generally arbitrarily
> marking the stack executable is a bad idea.

Nobody disagrees.  The execstack functionality is exactly there to avoid
having an executable stack unless really needed.  To to this binaries
need to be marked appropriately and the defaults (what happens if the
status isn't known for a file) have to be selected correctly for each
platform.  See Jakub patches to ld, see the extra section gcc creates to
mark stack usage (.note.GNU-stack).

-- 
--------------.                        ,-.            444 Castro Street
Ulrich Drepper \    ,-----------------'   \ Mountain View, CA 94041 USA
Red Hat         `--' drepper at redhat.com `---------------------------

Re: [PATCH] Set __libc_stack_end earlier on all arches,...

[Jakub Jelinek]

On Wed, Sep 24, 2003 at 01:41:01PM -0500, Steven Munroe wrote:
> Jakub Jelinek Wed, 24 Sep 2003 15:47:04 writes:
> 
> > The following patch does that (tested on a bunch of arches).
> 
> Actually it does not include PowerPC 32/64. Currently for PowerPC linux
> __libc_stack_end is set in libc_start_main, but this is not soon enough to
> support execstack and the execstack tests in make check fails (because
> __libc_stack_end is not set). The attached patch uses DL_PLATFORM_INIT to
> capture the argv pointer and store that into __libc_stack_end during
> _dl_sysdep_start before the call to dl_main. This will work for both powerpc32
> and powerpc64.
> 
> This is soon enough to pass the stack-end into _dl_make_stack_executable but it
> still fails in the mprotect with a "Permission Denied". I'll pursue that issue
> further.
> 
> I would like an explanation of why we are doing this. Generally arbitrarily
> marking the stack executable is a bad idea. So I would like a explaination
> (pointer to one) of the function we trying to provide and the conditions under
> which it will be used.

Normally on ppc32 all processes get executable stack, which is not exactly
secure. But you cannot disable it blindly for all processes, as you don't
know if they really need it or not.
ppc32 needs executable stack for trampolines (e.g. when you take address
of a nested function in C). Recent GCC+binutils mark objects with
.note.GNU-stack segment (either "" or "x" depending on whether it needs
executable stack or not) and ld merges this into PT_GNU_STACK segment.
If a binary has PT_GNU_STACK PF_R|PF_W|PF_X (or no PT_GNU_STACK segment),
kernel needs to assume the program needs (or might need) executable stack.
If a binary has PT_GNU_STACK PF_R|PF_W, kernel can assume the program
doesn't need executable stack and can change its VMA permissions.
This all can be done in gcc/binutils and kernel, no glibc help needed.
But now, if you have PT_GNU_STACK PF_R|PF_W binary which dlopens
some library which needs or might need executable stack (or depends on
such library through DT_NEEDED), kernel which only sees that binary
and the dynamic linker and might tighten stack permissions while
they actually need to be executable for the library. Here is when ld.so
needs to change the permissions.
In similar situation to ppc32 are most other linux arches (i386, amd64,
...).
The only special arches here are ia64 and ppc64, as there trampolines
don't actually need executable stack. Because of this I chose not to
add .note.GNU-stack/PT_GNU_STACK marking for these two arches.
Unfortunately, on both these arches kernel defaults to executable stack
as I found out today and thus there can be programs which rely on this
and so for binary compatibility need to be assumed as needing executable
stack too. Which means I'll probably have to mark all ia64/ppc64
gcc output with .section .note.GNU-stack, "" and let all binaries be
marked too (which one can always override using -Wa,--execstack or
-Wl,-z,execstack).

> 2003-09-24  Steven Munroe  <sjmunroe@us.ibm.com>
> 
> 	* sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c 
> 	[DL_PLATFORM_INIT]: Define
> 	(frob_stack_end): New function.

Why do you need this? Isn't the __libc_stack_end patch I've posted
sufficient for ppc32/ppc64? Certainly I haven't seen any tst-execstack*
failures on either ppc32 or ppc64.

ppc32:
sysdeps/powerpc/powerpc32/dl-start.S sets
mr      r3,r1
which becomes argument to _dl_start, which passes arg
directly do _dl_start_final and that one to
_dl_sysdep_start.
sysdeps/unix/sysv/linux/powerpc/dl-sysdep.c includes
sysdeps/generic/dl-sysdep.c for _dl_sysdep_start
and there my patch sets __libc_stack_end to that argument.

ppc64:
similarly, just the first step is in
sysdeps/powerpc/powerpc64/dl-machine.h (RTLD_START):
mr      3,1

	Jakub

Re: [PATCH] Set __libc_stack_end earlier on all arches,...

[David Mosberger]

>>>>> On Wed, 24 Sep 2003 21:36:22 +0200, Jakub Jelinek <jakub@redhat.com> said:

  Jakub> The only special arches here are ia64 and ppc64, as there
  Jakub> trampolines don't actually need executable stack. Because of
  Jakub> this I chose not to add .note.GNU-stack/PT_GNU_STACK marking
  Jakub> for these two arches.  Unfortunately, on both these arches
  Jakub> kernel defaults to executable stack as I found out today and
  Jakub> thus there can be programs which rely on this and so for
  Jakub> binary compatibility need to be assumed as needing executable
  Jakub> stack too.

Why do you say this?

	$ uname -a
	Linux wailua.hpl.hp.com 2.6.0-test5 #30 Tue Sep 16 11:28:09 PDT 2003 ia64 GNU/Linux
	$ cat /proc/self/maps |grep 60000fff7
	60000fff7fffc000-60000fff80000000 rw-p 00000000 00:00 0

We stopped turning on execute permission some time again (2.4.1x).
For ia64 Linux, there is an ELF OS flag which can be set to force
executable data/stack (the user-tool to do this is called "chatr").

	--david

Re: [PATCH] Set __libc_stack_end earlier on all arches,...

[Steve Munroe]

David Mosberger writes:

> > On Wed, 24 Sep 2003 21:36:22 +0200, Jakub Jelinek <jakub@redhat.com> 
said:
>
>  Jakub> The only special arches here are ia64 and ppc64, as there
>  Jakub> trampolines don't actually need executable stack. Because of
>  Jakub> this I chose not to add .note.GNU-stack/PT_GNU_STACK marking
>  Jakub> for these two arches.  Unfortunately, on both these arches
>  Jakub> kernel defaults to executable stack as I found out today and
>  Jakub> thus there can be programs which rely on this and so for
>  Jakub> binary compatibility need to be assumed as needing executable
>  Jakub> stack too.
>
>Why do you say this?

Also for PPC64 :

000001ff7fffd000-000001ff80000000 rw-p ffffffffffffe000 00:00 0