Right now, any offending object files in Fennec that don't include a specific empty section will cause our entire stack to be executable. This is terribad for security. Please see: http://www.airs.com/blog/archives/518 Luckily (as noted in the above blog post), adding -z noexecstack when linking our binaries will fix the issue for good. You can use "scanelf -e fennec" to verify the fix. Filing as a security bug, though this isn't any sort of explicit vulnerability. Make it public if this isn't necessary.

For reference, here is the current list of files (from find -name \*.o -print0 | xargs -0 scanelf -qe) that don't indicate whether or not the need an executable stack (meaning by default it's assumed they do): !WX --- --- ./gfx/ycbcr/yuv_row_arm.o !WX --- --- ./gfx/cairo/libpixman/src/pixman-arm-neon-asm-bilinear.o !WX --- --- ./gfx/cairo/libpixman/src/pixman-arm-neon-asm.o !WX --- --- ./gfx/cairo/libpixman/src/pixman-arm-simd-asm.o !WX --- --- ./js/src/ctypes/libffi/src/arm/sysv.o !WX --- --- ./media/libvpx/idct_v6.asm.o !WX --- --- ./media/libvpx/dequant_idct_v6.asm.o !WX --- --- ./media/libvpx/idct_dequant_dc_0_2x_neon.asm.o !WX --- --- ./media/libvpx/dc_only_idct_add_neon.asm.o !WX --- --- ./media/libvpx/loopfilter_v6.asm.o !WX --- --- ./media/libvpx/copymem8x4_neon.asm.o !WX --- --- ./media/libvpx/loopfiltersimplehorizontaledge_neon.asm.o !WX --- --- ./media/libvpx/copymem8x8_v6.asm.o !WX --- --- ./media/libvpx/recon4b_neon.asm.o !WX --- --- ./media/libvpx/save_neon_reg.asm.o !WX --- --- ./media/libvpx/mbloopfilter_neon.asm.o !WX --- --- ./media/libvpx/iwalsh_v6.asm.o !WX --- --- ./media/libvpx/loopfilter_neon.asm.o !WX --- --- ./media/libvpx/simpleloopfilter_v6.asm.o !WX --- --- ./media/libvpx/copymem16x16_v6.asm.o !WX --- --- ./media/libvpx/sixtappredict4x4_neon.asm.o !WX --- --- ./media/libvpx/bilinearfilter_v6.asm.o !WX --- --- ./media/libvpx/idct_dequant_0_2x_neon.asm.o !WX --- --- ./media/libvpx/sixtappredict8x4_v6.asm.o !WX --- --- ./media/libvpx/iwalsh_neon.asm.o !WX --- --- ./media/libvpx/copymem16x16_neon.asm.o !WX --- --- ./media/libvpx/sixtappredict16x16_neon.asm.o !WX --- --- ./media/libvpx/bilinearpredict4x4_neon.asm.o !WX --- --- ./media/libvpx/buildintrapredictorsmby_neon.asm.o !WX --- --- ./media/libvpx/bilinearpredict8x8_neon.asm.o !WX --- --- ./media/libvpx/shortidct4x4llm_neon.asm.o !WX --- --- ./media/libvpx/dequant_dc_idct_v6.asm.o !WX --- --- ./media/libvpx/recon2b_neon.asm.o !WX --- --- ./media/libvpx/dequant_idct_neon.asm.o !WX --- --- ./media/libvpx/bilinearpredict16x16_neon.asm.o !WX --- --- ./media/libvpx/sixtappredict8x4_neon.asm.o !WX --- --- ./media/libvpx/reconb_neon.asm.o !WX --- --- ./media/libvpx/dequantizeb_neon.asm.o !WX --- --- ./media/libvpx/recon_v6.asm.o !WX --- --- ./media/libvpx/bilinearpredict8x4_neon.asm.o !WX --- --- ./media/libvpx/dc_only_idct_add_v6.asm.o !WX --- --- ./media/libvpx/dequantize_v6.asm.o !WX --- --- ./media/libvpx/idct_dequant_full_2x_neon.asm.o !WX --- --- ./media/libvpx/filter_v6.asm.o !WX --- --- ./media/libvpx/copymem8x4_v6.asm.o !WX --- --- ./media/libvpx/copymem8x8_neon.asm.o !WX --- --- ./media/libvpx/idct_dequant_dc_full_2x_neon.asm.o !WX --- --- ./media/libvpx/recon16x16mb_neon.asm.o !WX --- --- ./media/libvpx/detokenize.asm.o !WX --- --- ./media/libvpx/shortidct4x4llm_1_neon.asm.o !WX --- --- ./media/libvpx/sixtappredict8x8_neon.asm.o !WX --- --- ./media/libvpx/loopfiltersimpleverticaledge_neon.asm.o !WX --- --- ./media/libtheora/lib/armfrag-gnu.o !WX --- --- ./media/libtheora/lib/armopts-gnu.o !WX --- --- ./media/libtheora/lib/armbits-gnu.o !WX --- --- ./media/libtheora/lib/armloop-gnu.o I've fixed the libtheora ones upstream (but not yet patched our version), and notified Google about the libvpx ones. Pixman should work once we upgrade to ndk5, as it will then define __linux__ and __ELF__ (which the pixman code checks before emitting the requisite .note.GNU-stack section). That leaves the ycbcr code (which is also mine) and libffi. Even if we fix all the individual files causing this problem, I agree the ld flag is the way to go, as it ensures this won't crop up again unexpectedly in the future (and if we ever do use code that needs an executable stack, presumably we'll notice from the segfaults when it tries to use it). That was also the approach taken in bug 156605 and bug 486537.

Is this a concern for desktop Firefox too?

IMO, it could not hurt to generally include this flag for linux platforms.

see also https://bugzilla.mozilla.org/show_bug.cgi?id=620058 which is a generic 'use gcc hardening/security flags' i would definitely like to see this flag be turned on if possible as well as the other flags mentioned in bug 620058. note that a non exec stack doesn't necessarily preclude using -fstack-protector since the stack can be smashed and return into non-stack code (eg a ret-to-libc attack)

(In reply to comment #2) > Is this a concern for desktop Firefox too? Only on ARM, I think (I checked x86-64, which was fine). media/libvpx/vpx_ports/x86_abi_support.asm takes care of marking that asm as not requiring an executable stack, and libtheora uses inline asm on all x86, so gcc takes care of it. Not sure what everyone else's story is. It may still be worth including the flag, as Ben says, to avoid surprises.

For lack of a better place to track these: The upstream libtheora commit mentioned in comment 1 was svn r18031: https://trac.xiph.org/changeset/18031/ I've also patched libvpx upstream: https://review.webmproject.org/2669

While checking the status of what libraries say can be useful, it doesn't say much about what really happens at runtime. As a matter of fact, since we're starting through the dalvik vm, we inherit its non executable stack, and the android linker doesn't flip it because of the flags in the libraries. The plugin-container executable, however, has the flags and the kernel will happily set an executable stack for it. That currently only matter for XUL UI.

Also note that I filed bug 706116 about the same issue recently, and that one is public.

Wondering if this is an issue on B2G too - also, any objections to unhiding this as there's a public variant of it ? Please speak now, or i'll open it up shortly.

(In reply to Ian Melven :imelven from comment #9) > Wondering if this is an issue on B2G too - also, any objections to unhiding > this as there's a public variant of it ? Please speak now, or i'll open it > up shortly. No objection from me. BTW, a more recent scan produces: !WX --- --- ./objdir-x86_64-unknown-linux-gnu/gfx/ycbcr/yuv_row_arm.o !WX --- --- ./objdir-x86_64-unknown-linux-gnu/gfx/skia/memset.arm.o !WX --- --- ./objdir-x86_64-unknown-linux-gnu/media/libtheora/lib/armfrag-gnu.o !WX --- --- ./objdir-x86_64-unknown-linux-gnu/media/libtheora/lib/armopts-gnu.o !WX --- --- ./objdir-x86_64-unknown-linux-gnu/media/libtheora/lib/armbits-gnu.o !WX --- --- ./objdir-x86_64-unknown-linux-gnu/media/libtheora/lib/armloop-gnu.o !WX --- --- ./objdir-x86_64-unknown-linux-gnu/media/libtheora/lib/armidct-gnu.o I'll bring in the libtheora commit we need to fix those, and take care of yuv_row_arm. glandium, can you take care of the Skia one? But I still think the linker flag is the right way to go to prevent this in the future.

Discussed with dveditz, unhiding - i marked the public variant of this bug a dupe of this one.

(In reply to Timothy B. Terriberry (:derf) from comment #10) > (In reply to Ian Melven :imelven from comment #9) > > Wondering if this is an issue on B2G too - also, any objections to unhiding > > this as there's a public variant of it ? Please speak now, or i'll open it > > up shortly. > > No objection from me. > > BTW, a more recent scan produces: > > !WX --- --- ./objdir-x86_64-unknown-linux-gnu/gfx/ycbcr/yuv_row_arm.o > !WX --- --- ./objdir-x86_64-unknown-linux-gnu/gfx/skia/memset.arm.o > !WX --- --- > ./objdir-x86_64-unknown-linux-gnu/media/libtheora/lib/armfrag-gnu.o > !WX --- --- > ./objdir-x86_64-unknown-linux-gnu/media/libtheora/lib/armopts-gnu.o > !WX --- --- > ./objdir-x86_64-unknown-linux-gnu/media/libtheora/lib/armbits-gnu.o > !WX --- --- > ./objdir-x86_64-unknown-linux-gnu/media/libtheora/lib/armloop-gnu.o > !WX --- --- > ./objdir-x86_64-unknown-linux-gnu/media/libtheora/lib/armidct-gnu.o > > I'll bring in the libtheora commit we need to fix those, and take care of > yuv_row_arm. glandium, can you take care of the Skia one? > > But I still think the linker flag is the right way to go to prevent this in > the future. I wonder. Does -Wa,--noexecstack prevent the problem from happening, without modifying the source?

(In reply to Mike Hommey [:glandium] from comment #13) > I wonder. Does -Wa,--noexecstack prevent the problem from happening, without > modifying the source? From quick testing, it works. So we could just add -Wa,--noexecstack in ASFLAGS when supported by the assembler, and -Wl,-z,noexecstack in LDFLAGS when supported by the linker.

With bug 752139 and bug 752293 landed on m-i, only one result remains: !WX --- --- ./objdir-x86_64-unknown-linux-gnu/gfx/skia/memset.arm.o

Comment on attachment 621536 [details] [diff] [review] Avoid marking binaries as requiring executable stack Review of attachment 621536 [details] [diff] [review]: ----------------------------------------------------------------- ::: configure.in @@ +1722,5 @@ > fi > WARNINGS_AS_ERRORS='-Werror -Wno-error=uninitialized' > DSO_CFLAGS='' > DSO_PIC_CFLAGS='-fPIC' > + ASFLAGS="$ASFLAGS -fPIC -Wa,--noexecstack" Can we always assume this assembler flag is supported?