| Message ID | 1506025575-1559-1-git-send-email-jim.wilson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [GOLD,AArch64] default stack not executable | expand |
Testcase attached. On Thu, Sep 21, 2017 at 1:26 PM, Jim Wilson <jim.wilson@linaro.org> wrote: > For aarch64, ld.bfd makes the stack not executable when a GNU-stack note is > missing. Note that elf_backend_default_execstack is 0 in bfd/elfnn-aarch64.c. > > However, ld.gold makes the stack executable when a GNU-stack note is missing. > Note that is_default_stack_executable is true in gold/aarch64.cc. > > This appears to be a bug in gold. It also looks like 64-bit ppc gets this > wrong also, though I have not tested that. > > The following patch fixes this for aarch64 by changing gold aarch64 to make > is_default_stack_executable false. This was tested with a make check, and > there were no regressions. It was also verified against a testcase using a > .s file with a missing GNU-stack note. > > gold/ > * aarch64.cc (Target_aarch64::aarch64_info): Set > is_default_stack_executable to false. > --- > gold/aarch64.cc | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/gold/aarch64.cc b/gold/aarch64.cc > index a72e2c3..4c6e920 100644 > --- a/gold/aarch64.cc > +++ b/gold/aarch64.cc > @@ -3523,7 +3523,7 @@ const Target::Target_info Target_aarch64<64, false>::aarch64_info = > false, // has_make_symbol > false, // has_resolve > false, // has_code_fill > - true, // is_default_stack_executable > + false, // is_default_stack_executable > true, // can_icf_inline_merge_sections > '\0', // wrap_char > "/lib/ld.so.1", // program interpreter > @@ -3551,7 +3551,7 @@ const Target::Target_info Target_aarch64<32, false>::aarch64_info = > false, // has_make_symbol > false, // has_resolve > false, // has_code_fill > - true, // is_default_stack_executable > + false, // is_default_stack_executable > false, // can_icf_inline_merge_sections > '\0', // wrap_char > "/lib/ld.so.1", // program interpreter > @@ -3579,7 +3579,7 @@ const Target::Target_info Target_aarch64<64, true>::aarch64_info = > false, // has_make_symbol > false, // has_resolve > false, // has_code_fill > - true, // is_default_stack_executable > + false, // is_default_stack_executable > true, // can_icf_inline_merge_sections > '\0', // wrap_char > "/lib/ld.so.1", // program interpreter > @@ -3607,7 +3607,7 @@ const Target::Target_info Target_aarch64<32, true>::aarch64_info = > false, // has_make_symbol > false, // has_resolve > false, // has_code_fill > - true, // is_default_stack_executable > + false, // is_default_stack_executable > false, // can_icf_inline_merge_sections > '\0', // wrap_char > "/lib/ld.so.1", // program interpreter > -- > 2.7.4 > weathertop:2106$ cat tmp2.s .comm i,4,4 weathertop:2107$ cd bin weathertop:2108$ rm ld weathertop:2109$ ln -s ld.bfd ld weathertop:2110$ cd .. weathertop:2111$ gcc -Bbin/ tmp.c tmp2.s weathertop:2112$ readelf -l a.out Elf file type is EXEC (Executable file) Entry point 0x400410 There are 8 program headers, starting at offset 64 Program Headers: Type Offset VirtAddr PhysAddr FileSiz MemSiz Flags Align PHDR 0x0000000000000040 0x0000000000400040 0x0000000000400040 0x00000000000001c0 0x00000000000001c0 R E 8 INTERP 0x0000000000000200 0x0000000000400200 0x0000000000400200 0x000000000000001b 0x000000000000001b R 1 [Requesting program interpreter: /lib/ld-linux-aarch64.so.1] LOAD 0x0000000000000000 0x0000000000400000 0x0000000000400000 0x00000000000005fc 0x00000000000005fc R E 10000 LOAD 0x0000000000000df0 0x0000000000410df0 0x0000000000410df0 0x0000000000000238 0x0000000000000240 RW 10000 DYNAMIC 0x0000000000000e08 0x0000000000410e08 0x0000000000410e08 0x00000000000001d0 0x00000000000001d0 RW 8 NOTE 0x000000000000021c 0x000000000040021c 0x000000000040021c 0x0000000000000044 0x0000000000000044 R 4 GNU_STACK 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 RW 10 GNU_RELRO 0x0000000000000df0 0x0000000000410df0 0x0000000000410df0 0x0000000000000210 0x0000000000000210 R 1 Section to Segment mapping: Segment Sections... 00 01 .interp 02 .interp .note.ABI-tag .note.gnu.build-id .gnu.hash .dynsym .dynstr .gnu.version .gnu.version_r .rela.dyn .rela.plt .init .plt .text .fini .rodata .eh_frame 03 .init_array .fini_array .jcr .dynamic .got .got.plt .data .bss 04 .dynamic 05 .note.ABI-tag .note.gnu.build-id 06 07 .init_array .fini_array .jcr .dynamic .got weathertop:2113$ cd bin weathertop:2114$ rm ld weathertop:2115$ ln -s ld.gold ld weathertop:2116$ cd .. weathertop:2117$ gcc -Bbin/ tmp.c tmp2.s weathertop:2118$ readelf -l a.out Elf file type is EXEC (Executable file) Entry point 0x4004e0 There are 9 program headers, starting at offset 64 Program Headers: Type Offset VirtAddr PhysAddr FileSiz MemSiz Flags Align PHDR 0x0000000000000040 0x0000000000400040 0x0000000000400040 0x00000000000001f8 0x00000000000001f8 R 8 INTERP 0x0000000000000238 0x0000000000400238 0x0000000000400238 0x000000000000001b 0x000000000000001b R 1 [Requesting program interpreter: /lib/ld-linux-aarch64.so.1] LOAD 0x0000000000000000 0x0000000000400000 0x0000000000400000 0x00000000000006d4 0x00000000000006d4 R E 10000 LOAD 0x000000000000fdf0 0x000000000041fdf0 0x000000000041fdf0 0x0000000000000238 0x0000000000000240 RW 10000 DYNAMIC 0x000000000000fe08 0x000000000041fe08 0x000000000041fe08 0x00000000000001d0 0x00000000000001d0 RW 8 NOTE 0x0000000000000254 0x0000000000400254 0x0000000000400254 0x0000000000000044 0x0000000000000044 R 4 GNU_EH_FRAME 0x00000000000006cc 0x00000000004006cc 0x00000000004006cc 0x0000000000000008 0x0000000000000008 R 4 GNU_STACK 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 RWE 10 GNU_RELRO 0x000000000000fdf0 0x000000000041fdf0 0x000000000041fdf0 0x0000000000000210 0x0000000000000210 RW 8 Section to Segment mapping: Segment Sections... 00 01 .interp 02 .interp .note.ABI-tag .note.gnu.build-id .dynsym .dynstr .gnu.hash .gnu.version .gnu.version_r .rela.dyn .rela.plt .init .plt .text .fini .rodata .eh_frame .eh_frame_hdr 03 .jcr .fini_array .init_array .dynamic .got .got.plt .data .bss 04 .dynamic 05 .note.ABI-tag .note.gnu.build-id 06 .eh_frame_hdr 07 08 .jcr .fini_array .init_array .dynamic .got weathertop:2119$
> For aarch64, ld.bfd makes the stack not executable when a GNU-stack note is > missing. Note that elf_backend_default_execstack is 0 in bfd/elfnn-aarch64.c. > > However, ld.gold makes the stack executable when a GNU-stack note is missing. > Note that is_default_stack_executable is true in gold/aarch64.cc. > > This appears to be a bug in gold. It also looks like 64-bit ppc gets this > wrong also, though I have not tested that. > > The following patch fixes this for aarch64 by changing gold aarch64 to make > is_default_stack_executable false. This was tested with a make check, and > there were no regressions. It was also verified against a testcase using a > .s file with a missing GNU-stack note. > > gold/ > * aarch64.cc (Target_aarch64::aarch64_info): Set > is_default_stack_executable to false. This is currently set to true in *all* targets that gold supports, because objects without a stack note must be assumed to be old objects that may require an executable stack. All recent objects (where "recent" is quite generous now) should have stack notes in them. The only case that I know of where a fairly recent object would be missing the stack note is an assembler-generated object that was assembled without the --noexecstack option. I don't see why aarch64 should be treated differently from other targets here, unless you can claim that there is no such thing as an old object that requires an executable stack but does not contain a stack note to that effect. If that's the case for aarch64 (and Han agrees), I'll support this patch. Since you raise the possibility that ppc should also default to false, I'll ask Alan the same question: are there old objects that must be assumed to require an executable stack? Regardless, I would support changing the --warn-execstack option to be on by default, so that users are properly warned about the presence of objects that lack the stack note. I think it's in everyone's interest to have this warning turned on. I'd also argue that it's time for the assembler to turn on --noexecstack by default. -cary
On Thu, Sep 21, 2017 at 5:26 PM, Cary Coutant <ccoutant@gmail.com> wrote: > I don't see why aarch64 should be treated differently from other > targets here, unless you can claim that there is no such thing as an > old object that requires an executable stack but does not contain a > stack note to that effect. If that's the case for aarch64 (and Han > agrees), I'll support this patch. GNU-stack notes were added in 2004. The aarch64 port was added in 2012. So yes, there are no old object files that predate the GNU-stack notes, which is why aarch64 ld.bfd ignores a missing GNU-stack note. > Since you raise the possibility that ppc should also default to false, > I'll ask Alan the same question: are there old objects that must be > assumed to require an executable stack? That should be 64-bit ppc only. 32-bit ppc has old object files that we need to be compatible with. Though I'm not sure what the justification is for 64-bit ppc, since it did exist before GNU-stack notes. Maybe this is because of the new ABI that rolled out a few of years ago? Checking ChangeLogs, I see that execstack was turned off for 64-bit ppc in 2007. Jim
On Thu, Sep 21, 2017 at 05:56:51PM -0700, Jim Wilson wrote: > On Thu, Sep 21, 2017 at 5:26 PM, Cary Coutant <ccoutant@gmail.com> wrote: > > I don't see why aarch64 should be treated differently from other > > targets here, unless you can claim that there is no such thing as an > > old object that requires an executable stack but does not contain a > > stack note to that effect. If that's the case for aarch64 (and Han > > agrees), I'll support this patch. > > GNU-stack notes were added in 2004. The aarch64 port was added in > 2012. So yes, there are no old object files that predate the > GNU-stack notes, which is why aarch64 ld.bfd ignores a missing > GNU-stack note. > > > Since you raise the possibility that ppc should also default to false, > > I'll ask Alan the same question: are there old objects that must be > > assumed to require an executable stack? > > That should be 64-bit ppc only. 32-bit ppc has old object files that > we need to be compatible with. Though I'm not sure what the > justification is for 64-bit ppc, since it did exist before GNU-stack > notes. Maybe this is because of the new ABI that rolled out a few of > years ago? Checking ChangeLogs, I see that execstack was turned off > for 64-bit ppc in 2007. ppc64 ELFv1 never needs to be exec stack. Since function descriptors are used there is no need for stack trampolines. Current gcc doesn't emit the stack notes for ELFv1. ppc64 ELFv2 on the other hand *does* need stack trampolines, and gcc has always emitted the stack notes for ELFv2. So I think is_default_stack_executable should indeed be false for ppc64. -- Alan Modra Australia Development Lab, IBM
On Thu, Sep 21, 2017 at 6:31 PM, Alan Modra <amodra@gmail.com> wrote: > On Thu, Sep 21, 2017 at 05:56:51PM -0700, Jim Wilson wrote: >> On Thu, Sep 21, 2017 at 5:26 PM, Cary Coutant <ccoutant@gmail.com> wrote: >> > I don't see why aarch64 should be treated differently from other >> > targets here, unless you can claim that there is no such thing as an >> > old object that requires an executable stack but does not contain a >> > stack note to that effect. If that's the case for aarch64 (and Han >> > agrees), I'll support this patch. >> >> GNU-stack notes were added in 2004. The aarch64 port was added in >> 2012. So yes, there are no old object files that predate the >> GNU-stack notes, which is why aarch64 ld.bfd ignores a missing >> GNU-stack note. >> >> > Since you raise the possibility that ppc should also default to false, >> > I'll ask Alan the same question: are there old objects that must be >> > assumed to require an executable stack? >> >> That should be 64-bit ppc only. 32-bit ppc has old object files that >> we need to be compatible with. Though I'm not sure what the >> justification is for 64-bit ppc, since it did exist before GNU-stack >> notes. Maybe this is because of the new ABI that rolled out a few of >> years ago? Checking ChangeLogs, I see that execstack was turned off >> for 64-bit ppc in 2007. > > ppc64 ELFv1 never needs to be exec stack. Since function descriptors > are used there is no need for stack trampolines. Current gcc doesn't > emit the stack notes for ELFv1. > > ppc64 ELFv2 on the other hand *does* need stack trampolines, and gcc > has always emitted the stack notes for ELFv2. > > So I think is_default_stack_executable should indeed be false for > ppc64. OK, I'm satisfied for both aarch64 and ppc64. Jim, your patch is OK. Alan, I'll leave the ppc64 patch to you. Any thoughts on turning on --warn-execstack by default? -cary
On Thu, Sep 21, 2017 at 10:56 PM, Cary Coutant <ccoutant@gmail.com> wrote:
> Any thoughts on turning on --warn-execstack by default?
--warn-execstack does two things.
1) Warn if a missing GNU-stack note causes the stack to be marked as executable.
2) Warn if a present GNU-stack note causes the stack to be marked as executable.
I don't think the second one should be on by default, as this would be
inconvenient for anyone using nested functions on targets that require
executable stack trampolines. Correctly written and compiled code
should not be producing warnings by default.
I think the first one could be on by default. It has been 13 years
since we added GNU-stack notes. If there are ones that are still
missing, we should warn people. This would not affect aarch64 and
64-bit ppc, as don't mark the stack as executable when a note is
missing now.
Jim
On Fri, Sep 22, 2017 at 8:16 AM, Jim Wilson <jim.wilson@linaro.org> wrote: > On Thu, Sep 21, 2017 at 10:56 PM, Cary Coutant <ccoutant@gmail.com> wrote: >> Any thoughts on turning on --warn-execstack by default? > > --warn-execstack does two things. > 1) Warn if a missing GNU-stack note causes the stack to be marked as executable. > 2) Warn if a present GNU-stack note causes the stack to be marked as executable. Actually, there is a third one in a different function. I didn't look far enough. 3) Warn if -z noexecstack and a GNU-stack note is present and causes the stack to be marked as executable. I think that one should be on by default. If a command line option conflicts with the input files, then it seems like you should give a warning always. Jim
diff --git a/gold/aarch64.cc b/gold/aarch64.cc index a72e2c3..4c6e920 100644 --- a/gold/aarch64.cc +++ b/gold/aarch64.cc @@ -3523,7 +3523,7 @@ const Target::Target_info Target_aarch64<64, false>::aarch64_info = false, // has_make_symbol false, // has_resolve false, // has_code_fill - true, // is_default_stack_executable + false, // is_default_stack_executable true, // can_icf_inline_merge_sections '\0', // wrap_char "/lib/ld.so.1", // program interpreter @@ -3551,7 +3551,7 @@ const Target::Target_info Target_aarch64<32, false>::aarch64_info = false, // has_make_symbol false, // has_resolve false, // has_code_fill - true, // is_default_stack_executable + false, // is_default_stack_executable false, // can_icf_inline_merge_sections '\0', // wrap_char "/lib/ld.so.1", // program interpreter @@ -3579,7 +3579,7 @@ const Target::Target_info Target_aarch64<64, true>::aarch64_info = false, // has_make_symbol false, // has_resolve false, // has_code_fill - true, // is_default_stack_executable + false, // is_default_stack_executable true, // can_icf_inline_merge_sections '\0', // wrap_char "/lib/ld.so.1", // program interpreter @@ -3607,7 +3607,7 @@ const Target::Target_info Target_aarch64<32, true>::aarch64_info = false, // has_make_symbol false, // has_resolve false, // has_code_fill - true, // is_default_stack_executable + false, // is_default_stack_executable false, // can_icf_inline_merge_sections '\0', // wrap_char "/lib/ld.so.1", // program interpreter