Fix hppa/ia64/microblaze executable stack default (bug 22156)

  As per https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01220.html hppa,
ia64 and microblaze default to non-executable stacks in the Linux
kernel.  glibc however defines DEFAULT_STACK_PERMS to include PF_X for
those architectures, meaning (a) elf/check-execstack fails and (b)
(from code inspection, not tested, but this is why I think this is a
user-visible bug) thread stacks are unnecessarily mapped with execute
permission.  This patch fixes the DEFAULT_STACK_PERMS definitions in
question.

Tested (compilation only) with build-many-glibcs.py for those
configurations.  This fixes the check-execstack failure (hppa still
has a check-textrel failure as the only remaining issue stopping that
architecture having clean build-many-glibcs.py results; hopefully
architecture maintainers can help resolve that).

2017-09-19  Joseph Myers  <joseph@codesourcery.com>

	[BZ #22156]
	* sysdeps/hppa/stackinfo.h (DEFAULT_STACK_PERMS): Remove PF_X.
	* sysdeps/ia64/stackinfo.h (DEFAULT_STACK_PERMS): Likewise.
	* sysdeps/microblaze/stackinfo.h (DEFAULT_STACK_PERMS): Likewise.
  

On 09/19/2017 07:46 AM, Joseph Myers wrote:
> As per https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01220.html hppa,
> ia64 and microblaze default to non-executable stacks in the Linux
> kernel.  glibc however defines DEFAULT_STACK_PERMS to include PF_X for
> those architectures, meaning (a) elf/check-execstack fails and (b)
> (from code inspection, not tested, but this is why I think this is a
> user-visible bug) thread stacks are unnecessarily mapped with execute
> permission.  This patch fixes the DEFAULT_STACK_PERMS definitions in
> question.
> 
> Tested (compilation only) with build-many-glibcs.py for those
> configurations.  This fixes the check-execstack failure (hppa still
> has a check-textrel failure as the only remaining issue stopping that
> architecture having clean build-many-glibcs.py results; hopefully
> architecture maintainers can help resolve that).

I thought that PF_X was required for hppa because there was still code
generated by gcc for hppa that needed it? Otherwise *I* would have
removed it when I did the PT_GNU_STACK cleanups for all the arches.

Dave is the best positioned to answer this question.
  

On 09/19/2017 07:56 AM, Carlos O'Donell wrote:
> On 09/19/2017 07:46 AM, Joseph Myers wrote:
>> As per https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01220.html hppa,
>> ia64 and microblaze default to non-executable stacks in the Linux
>> kernel.  glibc however defines DEFAULT_STACK_PERMS to include PF_X for
>> those architectures, meaning (a) elf/check-execstack fails and (b)
>> (from code inspection, not tested, but this is why I think this is a
>> user-visible bug) thread stacks are unnecessarily mapped with execute
>> permission.  This patch fixes the DEFAULT_STACK_PERMS definitions in
>> question.
>>
>> Tested (compilation only) with build-many-glibcs.py for those
>> configurations.  This fixes the check-execstack failure (hppa still
>> has a check-textrel failure as the only remaining issue stopping that
>> architecture having clean build-many-glibcs.py results; hopefully
>> architecture maintainers can help resolve that).
> 
> I thought that PF_X was required for hppa because there was still code
> generated by gcc for hppa that needed it? Otherwise *I* would have
> removed it when I did the PT_GNU_STACK cleanups for all the arches.
> 
> Dave is the best positioned to answer this question.
I don't think the PA was ever converted to the new bits from the Adacore
guys -- meaning I think it still generates executable stack trampolines
for nested functions.

Dave would know for sure :-)

jeff
  

On Tue, 19 Sep 2017, Jeff Law wrote:

> > I thought that PF_X was required for hppa because there was still code
> > generated by gcc for hppa that needed it? Otherwise *I* would have
> > removed it when I did the PT_GNU_STACK cleanups for all the arches.
> > 
> > Dave is the best positioned to answer this question.
> I don't think the PA was ever converted to the new bits from the Adacore
> guys -- meaning I think it still generates executable stack trampolines
> for nested functions.
> 
> Dave would know for sure :-)

If Andreas's description of the kernel default is correct, I'd expect that 
to mean that any attempt to call a pointer to a nested function would 
already fail because the kernel had in fact mapped the stack 
non-executable (so the glibc patch would accurately reflect what the 
kernel does - a GCC patch would *also* be needed for pointers to nested 
functions to work, but it would only need to output stack markers in the 
case where trampolines had been generated, to request an executable stack 
in that case).
  

On Sep 19 2017, Jeff Law <law@redhat.com> wrote:

> I don't think the PA was ever converted to the new bits from the Adacore
> guys -- meaning I think it still generates executable stack trampolines
> for nested functions.

But that should trigger emitting a stack note, overriding the default.
It's the absence of the note that is relevant here.

Andreas.
  

On 09/19/2017 08:54 AM, Andreas Schwab wrote:
> It's the absence of the note that is relevant here.

It can't be safe to override this?

This breaks backwards compatibility with old binaries that don't have PT_GNU_STACK
but need executable stacks?

I thought perhaps we were arguing to make old binaries
safer, at the risk of breaking some of them.
  

On Tue, 19 Sep 2017, Carlos O'Donell wrote:

> On 09/19/2017 08:54 AM, Andreas Schwab wrote:
> > It's the absence of the note that is relevant here.
> 
> It can't be safe to override this?
> 
> This breaks backwards compatibility with old binaries that don't have PT_GNU_STACK
> but need executable stacks?
> 
> I thought perhaps we were arguing to make old binaries
> safer, at the risk of breaking some of them.

My understanding is:

(a) As per Andreas, the kernel defaults to non-executable stacks on these 
three architectures.

(b) That is, if ld.so has no PT_GNU_STACK, the stack is non-executable; if 
ld.so does have PT_GNU_STACK, the kernel sets the permissions on the stack 
accordingly (which can include making it executable for trampoline use 
when the default is non-executable).

(c) At present, these GCC ports will never create PT_GNU_STACK markings to 
indicate that a trampoline needs an executable stack (if it does).

(d) glibc presumes that the kernel set the stack permissions to 
DEFAULT_STACK_PERMS.  If the executable or a loaded shared library 
introduces an executable stack requirement, the dynamic linker can make 
the stack executable - but it will never do so if it thinks the kernel 
already did so.

(e) DEFAULT_STACK_PERMS is used when creating thread stacks.

Now, if trampolines need executable stacks (and in some cases, especially 
with function descriptors, they may not) but work at present on those 
architectures with the default stack (rather than a thread stack), that 
suggests there is something wrong with the above analysis.
  

On 2017-09-19, at 10:42 AM, Jeff Law wrote:

> I don't think the PA was ever converted to the new bits from the Adacore
> guys -- meaning I think it still generates executable stack trampolines
> for nested functions.

Yes.

Dave
--
John David Anglin	dave.anglin@bell.net
  

On Fri, 22 Sep 2017, John David Anglin wrote:

> On 2017-09-19, at 10:42 AM, Jeff Law wrote:
> 
> > I don't think the PA was ever converted to the new bits from the Adacore
> > guys -- meaning I think it still generates executable stack trampolines
> > for nested functions.
> 
> Yes.

We now have two different contradictory assertions about hppa, here and 
<https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01449.html>.

Can someone with hppa-linux hardware confirm what stack permissions the 
kernel in fact starts processes with in the absence of GNU-stack markers 
(Andreas's analysis indicates non-executable), what if any changes glibc 
makes to those permissions after process creation, and whether trampolines 
work in such a process?  Testing whether my glibc patch changes the 
answers to any of those questions would be a bonus.
  

On 2017-09-22, at 12:08 PM, Joseph Myers wrote:

> On Fri, 22 Sep 2017, John David Anglin wrote:
> 
>> On 2017-09-19, at 10:42 AM, Jeff Law wrote:
>> 
>>> I don't think the PA was ever converted to the new bits from the Adacore
>>> guys -- meaning I think it still generates executable stack trampolines
>>> for nested functions.
>> 
>> Yes.
> 
> We now have two different contradictory assertions about hppa, here and 
> <https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01449.html>.
> 
> Can someone with hppa-linux hardware confirm what stack permissions the 
> kernel in fact starts processes with in the absence of GNU-stack markers 
> (Andreas's analysis indicates non-executable), what if any changes glibc 
> makes to those permissions after process creation, and whether trampolines 
> work in such a process?  Testing whether my glibc patch changes the 
> answers to any of those questions would be a bonus.


This is what I see for /bin/bash when we reach main:
f8d01000-f8d23000 rwxp 00000000 00:00 0                                  [stack]

This what I see for ld.so at _start:
f8d01000-f8d23000 rwxp 00000000 00:00 0                                  [stack]

Don't see any GNU-stack markers in either.

I'll try to test patch this weekend.

Dave
--
John David Anglin	dave.anglin@bell.net
  

On Fri, 22 Sep 2017, John David Anglin wrote:

> > Can someone with hppa-linux hardware confirm what stack permissions the 
> > kernel in fact starts processes with in the absence of GNU-stack markers 
> > (Andreas's analysis indicates non-executable), what if any changes glibc 
> > makes to those permissions after process creation, and whether trampolines 
> > work in such a process?  Testing whether my glibc patch changes the 
> > answers to any of those questions would be a bonus.
> 
> 
> This is what I see for /bin/bash when we reach main:
> f8d01000-f8d23000 rwxp 00000000 00:00 0                                  [stack]
> 
> This what I see for ld.so at _start:
> f8d01000-f8d23000 rwxp 00000000 00:00 0                                  [stack]
> 
> Don't see any GNU-stack markers in either.
> 
> I'll try to test patch this weekend.

Thanks.

Given what Andreas said at 
<https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01220.html>, does this mean 
there are other causes of executable stacks in the kernel, such as 
VM_STACK_DEFAULT_FLAGS?  If so, maybe hppa and microblaze do in fact need 
the GCC patch rather than the glibc one?
  

On 2017-09-22, at 4:11 PM, Joseph Myers wrote:

> Given what Andreas said at 
> <https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01220.html>, does this mean 
> there are other causes of executable stacks in the kernel, such as 
> VM_STACK_DEFAULT_FLAGS?  If so, maybe hppa and microblaze do in fact need 
> the GCC patch rather than the glibc one?

VM_STACK_DEFAULT_FLAGS defaults to VM_DATA_DEFAULT_FLAGS when it is
not defined, and it is not defined on hppa.

On hppa,  VM_DATA_DEFAULT_FLAGS, and it is:

#define VM_DATA_DEFAULT_FLAGS   (VM_READ | VM_WRITE | VM_EXEC | \
                                 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)

Dave
--
John David Anglin	dave.anglin@bell.net
  

On Fri, 22 Sep 2017, John David Anglin wrote:

> On 2017-09-22, at 4:11 PM, Joseph Myers wrote:
> 
> > Given what Andreas said at 
> > <https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01220.html>, does this mean 
> > there are other causes of executable stacks in the kernel, such as 
> > VM_STACK_DEFAULT_FLAGS?  If so, maybe hppa and microblaze do in fact need 
> > the GCC patch rather than the glibc one?
> 
> VM_STACK_DEFAULT_FLAGS defaults to VM_DATA_DEFAULT_FLAGS when it is
> not defined, and it is not defined on hppa.
> 
> On hppa,  VM_DATA_DEFAULT_FLAGS, and it is:
> 
> #define VM_DATA_DEFAULT_FLAGS   (VM_READ | VM_WRITE | VM_EXEC | \
>                                  VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)

Given this, and your experiment showing the stack is indeed executable on 
hppa, are the hppa parts of the GCC patch OK?  (I think we've established 
that ia64 should use the glibc change.  Based on VM_DATA_DEFAULT_FLAGS, 
microblaze would use the GCC change but hopefully there will be more 
information from microblaze people soon.)
  

On 2017-09-22, at 4:35 PM, Joseph Myers wrote:

> Given this, and your experiment showing the stack is indeed executable on 
> hppa, are the hppa parts of the GCC patch OK?

The hppa parts are OK.

Thanks,
Dave
--
John David Anglin	dave.anglin@bell.net
  

> -----Original Message-----
> From: libc-alpha-owner@sourceware.org [mailto:libc-alpha-
> owner@sourceware.org] On Behalf Of Joseph Myers
> Sent: Saturday, September 23, 2017 2:05 AM
> To: John David Anglin <dave.anglin@bell.net>
> Cc: Jeff Law <law@redhat.com>; Carlos O'Donell <carlos@redhat.com>; GNU C
> Library <libc-alpha@sourceware.org>; Mike Frysinger <vapier@gentoo.org>;
> Helge Deller <deller@gmx.de>
> Subject: Re: Fix hppa/ia64/microblaze executable stack default (bug 22156)
> 
> On Fri, 22 Sep 2017, John David Anglin wrote:
> 
> > On 2017-09-22, at 4:11 PM, Joseph Myers wrote:
> >
> > > Given what Andreas said at
> > > <https://gcc.gnu.org/ml/gcc-patches/2017-09/msg01220.html>, does
> > > this mean there are other causes of executable stacks in the kernel,
> > > such as VM_STACK_DEFAULT_FLAGS?  If so, maybe hppa and microblaze
> do
> > > in fact need the GCC patch rather than the glibc one?
> >
> > VM_STACK_DEFAULT_FLAGS defaults to VM_DATA_DEFAULT_FLAGS when it
> is
> > not defined, and it is not defined on hppa.
> >
> > On hppa,  VM_DATA_DEFAULT_FLAGS, and it is:
> >
> > #define VM_DATA_DEFAULT_FLAGS   (VM_READ | VM_WRITE | VM_EXEC |
> \
> >                                  VM_MAYREAD | VM_MAYWRITE |
> > VM_MAYEXEC)
> 
> Given this, and your experiment showing the stack is indeed executable on
> hppa, are the hppa parts of the GCC patch OK?  (I think we've established that
> ia64 should use the glibc change.  Based on VM_DATA_DEFAULT_FLAGS,
> microblaze would use the GCC change but hopefully there will be more
> information from microblaze people soon.)
Yes, for Microblaze we need to apply GCC patch.
I have applied the patch and found no regressions with it.

Thanks,
Nagaraju
> --
> Joseph S. Myers
> joseph@codesourcery.com