Discussion:
[Patch, MIPS] Add .note.GNU-stack section
Steve Ellcey
2014-09-10 16:24:23 UTC
Permalink
Someone noticed that the MIPS GCC compiler was not putting out the
.note.GNU-stack section. This simple patch fixes that problem by
calling the standard file_end_indicate_exec_stack function.

Tested on mips-mti-linux-gnu, OK to checkin?

Steve Ellcey
***@mips.com



2014-09-10 Steve Ellcey <***@mips.com>

* config/mips/mips.c (TARGET_ASM_FILE_END): Define.


diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 646bb4d..bcaa9cd 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -19146,6 +19146,9 @@ mips_lra_p (void)
#undef TARGET_LRA_P
#define TARGET_LRA_P mips_lra_p

+#undef TARGET_ASM_FILE_END
+#define TARGET_ASM_FILE_END file_end_indicate_exec_stack
+
struct gcc_target targetm = TARGET_INITIALIZER;

#include "gt-mips.h"
p***@gmail.com
2014-09-10 16:27:19 UTC
Permalink
Post by Steve Ellcey
Someone noticed that the MIPS GCC compiler was not putting out the
.note.GNU-stack section. This simple patch fixes that problem by
calling the standard file_end_indicate_exec_stack function.
Tested on mips-mti-linux-gnu, OK to checkin?
This works except you did not update the assembly files in libgcc or glibc. We (Cavium) have the same patch in our tree for a few released versions.

Thanks,
Andrew
Post by Steve Ellcey
Steve Ellcey
* config/mips/mips.c (TARGET_ASM_FILE_END): Define.
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index 646bb4d..bcaa9cd 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -19146,6 +19146,9 @@ mips_lra_p (void)
#undef TARGET_LRA_P
#define TARGET_LRA_P mips_lra_p
+#undef TARGET_ASM_FILE_END
+#define TARGET_ASM_FILE_END file_end_indicate_exec_stack
+
struct gcc_target targetm = TARGET_INITIALIZER;
#include "gt-mips.h"
Steve Ellcey
2014-09-26 19:59:57 UTC
Permalink
Post by p***@gmail.com
This works except you did not update the assembly files in
libgcc or glibc. We (Cavium) have the same patch in our tree
for a few released versions.
Mind just checking yours in then Andrew?
Thanks!
-eric
I talked to Andrew about what files he changed in GCC and created and
tested this new patch. Andrew also mentioned changing some assembly
files in glibc but I don't see any use of '.section .note.GNU-stack' in
any assembly files in glibc (for any platform) so I wasn't planning on
creating a glibc to add them to mips glibc assembly language files.

OK to check in this patch?

Steve Ellcey
***@mips.com



2014-09-26 Steve Ellcey <***@mips.com>

* config/mips/mips.c (TARGET_ASM_FILE_END): Define.
* libgcc/config/mips/mips16.S: Add .note.GNU-stack section.
* libgcc/config/mips/vr4120-div.S: Ditto.
* libgcc/config/mips/crti.S: Ditto.
* libgcc/config/mips/crtn.S: Ditto.


diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index f9713c1..39020d7 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -19146,6 +19146,9 @@ mips_lra_p (void)
#undef TARGET_LRA_P
#define TARGET_LRA_P mips_lra_p

+#undef TARGET_ASM_FILE_END
+#define TARGET_ASM_FILE_END file_end_indicate_exec_stack
+
struct gcc_target targetm = TARGET_INITIALIZER;

#include "gt-mips.h"
diff --git a/libgcc/config/mips/crti.S b/libgcc/config/mips/crti.S
index 6980594..93436c0 100644
--- a/libgcc/config/mips/crti.S
+++ b/libgcc/config/mips/crti.S
@@ -21,6 +21,10 @@ a copy of the GCC Runtime Library Exception along with this program;
see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
<http://www.gnu.org/licenses/>. */

+
+/* An executable stack is *not* required for these functions. */
+ .section .note.GNU-stack,"",%progbits
+
/* 4 slots for argument spill area. 1 for cpreturn, 1 for stack.
Return spill offset of 40 and 20. Aligned to 16 bytes for n32. */

diff --git a/libgcc/config/mips/crtn.S b/libgcc/config/mips/crtn.S
index 0de2d0c..6f2c301 100644
--- a/libgcc/config/mips/crtn.S
+++ b/libgcc/config/mips/crtn.S
@@ -21,6 +21,9 @@ a copy of the GCC Runtime Library Exception along with this program;
see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
<http://www.gnu.org/licenses/>. */

+/* An executable stack is *not* required for these functions. */
+ .section .note.GNU-stack,"",%progbits
+
/* 4 slots for argument spill area. 1 for cpreturn, 1 for stack.
Return spill offset of 40 and 20. Aligned to 16 bytes for n32. */

diff --git a/libgcc/config/mips/mips16.S b/libgcc/config/mips/mips16.S
index dde8939..58e4377 100644
--- a/libgcc/config/mips/mips16.S
+++ b/libgcc/config/mips/mips16.S
@@ -35,6 +35,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
values using the soft-float calling convention, but do the actual
operation using the hard floating point instructions. */

+/* An executable stack is *not* required for these functions. */
+ .section .note.GNU-stack,"",%progbits
+ .previous
+
#if defined _MIPS_SIM && (_MIPS_SIM == _ABIO32 || _MIPS_SIM == _ABIO64)

/* This file contains 32-bit assembly code. */
diff --git a/libgcc/config/mips/vr4120-div.S b/libgcc/config/mips/vr4120-div.S
index 76c4e7a..664d3c3 100644
--- a/libgcc/config/mips/vr4120-div.S
+++ b/libgcc/config/mips/vr4120-div.S
@@ -26,6 +26,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
-mfix-vr4120. div and ddiv do not give the correct result when one
of the operands is negative. */

+/* An executable stack is *not* required for these functions. */
+ .section .note.GNU-stack,"",%progbits
+ .previous
+
.set nomips16

#define DIV \
Matthew Fortune
2014-10-08 11:52:50 UTC
Permalink
Post by Steve Ellcey
I talked to Andrew about what files he changed in GCC and created and
tested this new patch. Andrew also mentioned changing some assembly
files in glibc but I don't see any use of '.section .note.GNU-stack' in
any assembly files in glibc (for any platform) so I wasn't planning on
creating a glibc to add them to mips glibc assembly language files.
OK to check in this patch?
I think we need to wait for some conclusion about how the kernel will
cope with a non-executable stack before committing this. The discussion
is progressing on the glibc and kernel lists.

As far as I can tell the kernel will respond to the PT_GNU_STACK
segment already for MIPS and then lead to a crash if the FPU emulator is
invoked. If this is the case then it would be ideal if we can somehow
prevent a stack becoming non-executable unless built against a kernel
which is known to be safe for no-execstack.

Existing versions of glibc will simply follow suit with the compiler
and apply --noexecstack when building assembly files if the compiler
emits a .note.GNU-stack section. Given that behaviour then as soon
as we enable no-execstack support in the compiler then we will get
PT_GNU_STACK on the binaries and risk a crash.

I'm struggling to come up with a good way to make this safe on existing
kernels. One possibility is that MIPS would have to implement the
compiler driven no-execstack support by passing the --no-execstack
option to the assembler instead of using the .note.GNU-stack section
explicitly. This would avoid triggering the configure check in glibc
and allow us to write a MIPS specific check in glibc which looks at
both the kernel version and whether the compiler enables no-execstack.

Slightly convoluted but safe. I'm not sure how uclibc and musl deal
with this issue but we should really check them too (but they are still
secondary to glibc I think). Any other ideas?

Matthew
Post by Steve Ellcey
Steve Ellcey
* config/mips/mips.c (TARGET_ASM_FILE_END): Define.
* libgcc/config/mips/mips16.S: Add .note.GNU-stack section.
* libgcc/config/mips/vr4120-div.S: Ditto.
* libgcc/config/mips/crti.S: Ditto.
* libgcc/config/mips/crtn.S: Ditto.
diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
index f9713c1..39020d7 100644
--- a/gcc/config/mips/mips.c
+++ b/gcc/config/mips/mips.c
@@ -19146,6 +19146,9 @@ mips_lra_p (void)
#undef TARGET_LRA_P
#define TARGET_LRA_P mips_lra_p
+#undef TARGET_ASM_FILE_END
+#define TARGET_ASM_FILE_END file_end_indicate_exec_stack
+
struct gcc_target targetm = TARGET_INITIALIZER;
#include "gt-mips.h"
diff --git a/libgcc/config/mips/crti.S b/libgcc/config/mips/crti.S
index 6980594..93436c0 100644
--- a/libgcc/config/mips/crti.S
+++ b/libgcc/config/mips/crti.S
@@ -21,6 +21,10 @@ a copy of the GCC Runtime Library Exception along with this program;
see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
<http://www.gnu.org/licenses/>. */
+
+/* An executable stack is *not* required for these functions. */
+ .section .note.GNU-stack,"",%progbits
+
/* 4 slots for argument spill area. 1 for cpreturn, 1 for stack.
Return spill offset of 40 and 20. Aligned to 16 bytes for n32. */
diff --git a/libgcc/config/mips/crtn.S b/libgcc/config/mips/crtn.S
index 0de2d0c..6f2c301 100644
--- a/libgcc/config/mips/crtn.S
+++ b/libgcc/config/mips/crtn.S
@@ -21,6 +21,9 @@ a copy of the GCC Runtime Library Exception along with this program;
see the files COPYING3 and COPYING.RUNTIME respectively. If not, see
<http://www.gnu.org/licenses/>. */
+/* An executable stack is *not* required for these functions. */
+ .section .note.GNU-stack,"",%progbits
+
/* 4 slots for argument spill area. 1 for cpreturn, 1 for stack.
Return spill offset of 40 and 20. Aligned to 16 bytes for n32. */
diff --git a/libgcc/config/mips/mips16.S b/libgcc/config/mips/mips16.S
index dde8939..58e4377 100644
--- a/libgcc/config/mips/mips16.S
+++ b/libgcc/config/mips/mips16.S
@@ -35,6 +35,10 @@ see the files COPYING3 and COPYING.RUNTIME
respectively. If not, see
values using the soft-float calling convention, but do the actual
operation using the hard floating point instructions. */
+/* An executable stack is *not* required for these functions. */
+ .section .note.GNU-stack,"",%progbits
+ .previous
+
#if defined _MIPS_SIM && (_MIPS_SIM == _ABIO32 || _MIPS_SIM == _ABIO64)
/* This file contains 32-bit assembly code. */
diff --git a/libgcc/config/mips/vr4120-div.S b/libgcc/config/mips/vr4120-
div.S
index 76c4e7a..664d3c3 100644
--- a/libgcc/config/mips/vr4120-div.S
+++ b/libgcc/config/mips/vr4120-div.S
@@ -26,6 +26,10 @@ see the files COPYING3 and COPYING.RUNTIME
respectively. If not, see
-mfix-vr4120. div and ddiv do not give the correct result when one
of the operands is negative. */
+/* An executable stack is *not* required for these functions. */
+ .section .note.GNU-stack,"",%progbits
+ .previous
+
.set nomips16
#define DIV \
Loading...