Message ID | alpine.DEB.2.20.1709191345040.2962@digraph.polyomino.org.uk |
---|---|
State | New, archived |
Headers |
Received: (qmail 6068 invoked by alias); 19 Sep 2017 13:47:01 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 5822 invoked by uid 89); 19 Sep 2017 13:47:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.5 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS, URIBL_RED autolearn=ham version=3.3.2 spammy= X-HELO: relay1.mentorg.com Date: Tue, 19 Sep 2017 13:46:49 +0000 From: Joseph Myers <joseph@codesourcery.com> To: <libc-alpha@sourceware.org> CC: <vapier@gentoo.org>, <carlos@redhat.com>, <dave.anglin@bell.net> Subject: Fix hppa/ia64/microblaze executable stack default (bug 22156) Message-ID: <alpine.DEB.2.20.1709191345040.2962@digraph.polyomino.org.uk> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-ClientProxiedBy: svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) To svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) |
Commit Message
Joseph Myers
Sept. 19, 2017, 1:46 p.m. UTC
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.
Comments
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
diff --git a/sysdeps/hppa/stackinfo.h b/sysdeps/hppa/stackinfo.h index 83b1da1..a3af38f 100644 --- a/sysdeps/hppa/stackinfo.h +++ b/sysdeps/hppa/stackinfo.h @@ -23,9 +23,8 @@ #include <elf.h> -/* Default to an executable stack. PF_X can be overridden if PT_GNU_STACK is - * present, but it is presumed absent. */ -#define DEFAULT_STACK_PERMS (PF_R|PF_W|PF_X) +/* Default to a non-executable stack. */ +#define DEFAULT_STACK_PERMS (PF_R|PF_W) /* On PA the stack grows up. */ #define _STACK_GROWS_UP 1 diff --git a/sysdeps/ia64/stackinfo.h b/sysdeps/ia64/stackinfo.h index 87e1448..a50e727 100644 --- a/sysdeps/ia64/stackinfo.h +++ b/sysdeps/ia64/stackinfo.h @@ -27,8 +27,7 @@ here. */ #define _STACK_GROWS_DOWN 1 -/* Default to an executable stack. PF_X can be overridden if PT_GNU_STACK is - * present, but it is presumed absent. */ -#define DEFAULT_STACK_PERMS (PF_R|PF_W|PF_X) +/* Default to a non-executable stack. */ +#define DEFAULT_STACK_PERMS (PF_R|PF_W) #endif /* stackinfo.h */ diff --git a/sysdeps/microblaze/stackinfo.h b/sysdeps/microblaze/stackinfo.h index 3062b1e..05755c7 100644 --- a/sysdeps/microblaze/stackinfo.h +++ b/sysdeps/microblaze/stackinfo.h @@ -27,8 +27,7 @@ /* On MicroBlaze the stack grows down. */ # define _STACK_GROWS_DOWN 1 -/* Default to an executable stack. PF_X can be overridden if PT_GNU_STACK is - * present, but it is presumed absent. */ -# define DEFAULT_STACK_PERMS (PF_R|PF_W|PF_X) +/* Default to a non-executable stack. */ +# define DEFAULT_STACK_PERMS (PF_R|PF_W) #endif /* stackinfo.h. */