|
From: Matthias S. <zz...@ge...> - 2015-09-27 19:19:41
Attachments:
valgrind-3.11.0-fix-executable-stack.patch
|
Hi there! This patch will fix all assembler files that do not declare their stack as non-executable. All assembler files that do not contain this code tells the linker to mark the stack executable: .section .note.GNU-stack,"",@progbits OR .section .note.GNU-stack,"",%progbits My patch creates a macro that contains either version or nothing if the platform does not require it. Then all asm-files are changed to unconditionally call this macro at the end of file. Additionally it removes an unneeded inclusion of "config.h" from "dispatch-ppc32-linux.S". The only thing I am not sure about is: On which platforms to use @progbits or %progbits. I tried to guess from the existing code, but that might be wrong. Regards Matthias |
|
From: Florian K. <fl...@ei...> - 2015-09-29 20:30:59
|
On 27.09.2015 21:19, Matthias Schwarzott wrote: > Hi there! > > This patch will fix all assembler files that do not declare their stack > as non-executable. > > All assembler files that do not contain this code tells the linker to > mark the stack executable: > > .section .note.GNU-stack,"",@progbits > OR > .section .note.GNU-stack,"",%progbits > > My patch creates a macro that contains either version or nothing if the > platform does not require it. > > Then all asm-files are changed to unconditionally call this macro at the > end of file. > It's nice to clean this up. Thanks! > Additionally it removes an unneeded inclusion of "config.h" from > "dispatch-ppc32-linux.S". No. dispatch-ppc32-linux.S has an #ifdef HAS_ALTIVEC and that symbol is being defined in config.h . > > The only thing I am not sure about is: On which platforms to use > @progbits or %progbits. m_trampoline.S has this: #if defined(VGO_linux) /* Let the linker know we don't need an executable stack */ # if defined(VGP_arm_linux) .section .note.GNU-stack,"",%progbits # else .section .note.GNU-stack,"",@progbits # endif #endif That code is shared for all platforms. A web search provides further evidence that on ARM the @ character introduces a comment in assembler code. On other platforms, the choice of @progbits or %progbits apparently does not matter. At least on mips a grep shows: ./coregrind/m_syswrap/syscall-mips64-linux.S:.section .note.GNU-stack,"",%progbits ./coregrind/m_dispatch/dispatch-mips64-linux.S:.section .note.GNU-stack,"",@progbits So let's run with the above ifdeffery. What I did not quite understand why (sometimes) you moved code outside the #ifdef that guards the complete contents of a file. For instance: --- a/coregrind/m_syswrap/syscall-x86-solaris.S +++ b/coregrind/m_syswrap/syscall-x86-solaris.S @@ -28,9 +28,10 @@ The GNU General Public License is contained in the file COPYING. */ +#include "pub_core_basics_asm.h" + #if defined(VGP_x86_solaris) -#include "pub_core_basics_asm.h" #include "pub_core_vkiscnums_asm.h" #include "libvex_guest_offsets.h" @@ -270,6 +271,9 @@ ML_(blksys_finished_DRET): .long 4b #endif // defined(VGP_x86_solaris) +/* Let the linker know we don't need an executable stack */ +MARK_STACK_NO_EXEC + I don't see a need for that or did you encounter some sort of problem? Florian |
|
From: Matthias S. <zz...@ge...> - 2015-09-30 17:59:19
Attachments:
valgrind-3.11.0-fix-executable-stack-V2.patch
|
Am 29.09.2015 um 22:30 schrieb Florian Krohm: > On 27.09.2015 21:19, Matthias Schwarzott wrote: >> Hi there! >> >> This patch will fix all assembler files that do not declare their stack >> as non-executable. >> >> All assembler files that do not contain this code tells the linker to >> mark the stack executable: >> >> .section .note.GNU-stack,"",@progbits >> OR >> .section .note.GNU-stack,"",%progbits >> >> My patch creates a macro that contains either version or nothing if the >> platform does not require it. >> >> Then all asm-files are changed to unconditionally call this macro at the >> end of file. >> > > It's nice to clean this up. Thanks! > >> Additionally it removes an unneeded inclusion of "config.h" from >> "dispatch-ppc32-linux.S". > > No. dispatch-ppc32-linux.S has an #ifdef HAS_ALTIVEC and that symbol is > being defined in config.h . > pub_core_basics_asm.h already includes config.h so I thought this second inclusion is not necessary. But as this is not the intention to clean up includes I dropped this part. > >> >> The only thing I am not sure about is: On which platforms to use >> @progbits or %progbits. > > m_trampoline.S has this: > > #if defined(VGO_linux) > /* Let the linker know we don't need an executable stack */ > # if defined(VGP_arm_linux) > .section .note.GNU-stack,"",%progbits > # else > .section .note.GNU-stack,"",@progbits > # endif > #endif > > That code is shared for all platforms. A web search provides further > evidence that on ARM the @ character introduces a comment in assembler > code. On other platforms, the choice of @progbits or %progbits > apparently does not matter. At least on mips a grep shows: > > ./coregrind/m_syswrap/syscall-mips64-linux.S:.section > .note.GNU-stack,"",%progbits > ./coregrind/m_dispatch/dispatch-mips64-linux.S:.section > .note.GNU-stack,"",@progbits > > So let's run with the above ifdeffery. > Yes, I also found this code, but was wondering about some mips and tilegx asm files using %progbits. As you say m_trampoline.S is compiled everywhere I use the logic from there. > What I did not quite understand why (sometimes) you moved code outside > the #ifdef that guards the complete contents of a file. For instance: > > --- a/coregrind/m_syswrap/syscall-x86-solaris.S > +++ b/coregrind/m_syswrap/syscall-x86-solaris.S > @@ -28,9 +28,10 @@ > The GNU General Public License is contained in the file COPYING. > */ > > +#include "pub_core_basics_asm.h" > + > #if defined(VGP_x86_solaris) > > -#include "pub_core_basics_asm.h" > #include "pub_core_vkiscnums_asm.h" > #include "libvex_guest_offsets.h" > > @@ -270,6 +271,9 @@ ML_(blksys_finished_DRET): .long 4b > > #endif // defined(VGP_x86_solaris) > > +/* Let the linker know we don't need an executable stack */ > +MARK_STACK_NO_EXEC > + > > I don't see a need for that or did you encounter some sort of problem? > The reason can be explained considering linker behaviour. The linker will request an executable stack as soon as at least one object file, that is linked in, wants an executable stack. And the absence of the above mentioned .note.GNU-stack section is enough to tell the linker that an executable stack is needed. So even an empty asm-file must at least contain this statement to not force executable stacks on the whole executable. To use the macro, also the "#include" must be unconditional. An updated patch is attached. Regards Matthias |
|
From: Florian K. <fl...@ei...> - 2015-09-30 20:32:36
|
On 30.09.2015 19:58, Matthias Schwarzott wrote: > >> What I did not quite understand why (sometimes) you moved code outside >> the #ifdef that guards the complete contents of a file. For instance: >> .... snip ..... >> >> I don't see a need for that or did you encounter some sort of problem? >> > The reason can be explained considering linker behaviour. > The linker will request an executable stack as soon as at least one > object file, that is linked in, wants an executable stack. > And the absence of the above mentioned .note.GNU-stack section is enough > to tell the linker that an executable stack is needed. > > So even an empty asm-file must at least contain this statement to not > force executable stacks on the whole executable. Thanks for explaining. I did not know that. Patch applied as r15692 Florian |
|
From: Ivo R. <ivo...@gm...> - 2015-10-01 21:02:50
|
>From r15692 what I can tell so far is that we care about non-executable stack only for the Valgrind launcher (valgrind binary) and the actual Valgrind tools. What about vgdb, for example? Or other binaries built? The reason I am asking is that on Solaris I have to include a special mapfile to explicitly mark binaries as not-having-executable-stack. The default varies across versions and system settings. Thanks! I. |