[v3] powerpc64le: ROP Changes for strncpy/ppc-mount

Message ID 20241122183509.691384-1-smonga@linux.ibm.com
State Under Review
Delegated to: Peter Bergner
Headers
Series [v3] powerpc64le: ROP Changes for strncpy/ppc-mount |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Test passed
redhat-pt-bot/TryBot-32bit success Build for i686

Commit Message

Sachin Monga Nov. 22, 2024, 6:35 p.m. UTC
  Modified FRAME_MIN_SIZE to 48B for ELFv2 to reserve
additional 16 bytes for ROP save slot and padding.

Signed-off-by: Sachin Monga <smonga@linux.ibm.com>
---
>ppc-mcount.S
>Your changes look fine for this file.  However, we're missing the CFI directives
>cfi_adjust_cfa_offset(-FRAME_MIN_SIZE) and cfi_restore(lr).  That should be
>a separate fix.  Please post a small patch to add those.
As discussed, I'll work on separate fix for cfi_offsets post ROP patches.

 sysdeps/powerpc/powerpc64/le/power9/strncpy.S | 12 +++-
 sysdeps/powerpc/powerpc64/power8/strncpy.S    | 56 ++++++++++---------
 sysdeps/powerpc/powerpc64/ppc-mcount.S        |  6 ++
 sysdeps/powerpc/powerpc64/sysdep.h            |  2 +-
 4 files changed, 47 insertions(+), 29 deletions(-)
  

Comments

Peter Bergner Nov. 25, 2024, 3:53 p.m. UTC | #1
On 11/22/24 12:35 PM, Sachin Monga wrote:
> Modified FRAME_MIN_SIZE to 48B for ELFv2 to reserve
> additional 16 bytes for ROP save slot and padding.

LGTM, modulo some whitespace issues which I fixed for you.  Note we use tabs
instead of spaces before mnemonics and the first insn operand.

Reviewed-by: Peter Bergner <bergner@linux.ibm.com>

Pushed.

Peter
  
Sachin Monga Nov. 26, 2024, 2:38 a.m. UTC | #2
Thanks Peter. Noted.

On 25/11/24 9:23 pm, Peter Bergner wrote:
> On 11/22/24 12:35 PM, Sachin Monga wrote:
>> Modified FRAME_MIN_SIZE to 48B for ELFv2 to reserve
>> additional 16 bytes for ROP save slot and padding.
> LGTM, modulo some whitespace issues which I fixed for you.  Note we use tabs
> instead of spaces before mnemonics and the first insn operand.
>
> Reviewed-by: Peter Bergner <bergner@linux.ibm.com>
>
> Pushed.
>
> Peter
>
  

Patch

diff --git a/sysdeps/powerpc/powerpc64/le/power9/strncpy.S b/sysdeps/powerpc/powerpc64/le/power9/strncpy.S
index 58139ad9e8..ee92f60005 100644
--- a/sysdeps/powerpc/powerpc64/le/power9/strncpy.S
+++ b/sysdeps/powerpc/powerpc64/le/power9/strncpy.S
@@ -365,8 +365,8 @@  L(zero_padding_end):
 
 	.align	4
 L(zero_padding_memset):
-	std	r30,-8(r1)   /* Save r30 on the stack.  */
-	cfi_offset(r30, -8)
+	std	r30,-16(r1)   /* Save r30 on the stack after ROP slot */
+	cfi_offset(r30, -16)
 	mr	r30,r3       /* Save the return value of strncpy.  */
 	/* Prepare the call to memset.  */
 	mr	r3,r11       /* Pointer to the area to be zero-filled.  */
@@ -380,6 +380,9 @@  L(zero_padding_memset):
 	mflr	r0
 	std	r0,16(r1)
 
+#ifdef  __ROP_PROTECT__
+	hashst  0,FRAME_ROP_SAVE(r1)
+#endif
 	/* Create the stack frame.  */
 	stdu	r1,-FRAMESIZE(r1)
 	cfi_adjust_cfa_offset(FRAMESIZE)
@@ -395,13 +398,16 @@  L(zero_padding_memset):
 	mr	r3,r30       /* Restore the return value of strncpy, i.e.:
 				dest.  For stpncpy, the return value is the
 				same as return value of memset.  */
-	ld	r30,FRAMESIZE-8(r1) /* Restore r30.  */
+	ld	r30,FRAMESIZE-16(r1) /* Restore r30.  */
 	/* Restore the stack frame.  */
 	addi	r1,r1,FRAMESIZE
 	cfi_adjust_cfa_offset(-FRAMESIZE)
 	/* Restore the link register.  */
 	mtlr	r0
 	cfi_restore(lr)
+#ifdef __ROP_PROTECT__
+	hashchk 0,FRAME_ROP_SAVE(r1)
+#endif
 	blr
 
 END (FUNC_NAME)
diff --git a/sysdeps/powerpc/powerpc64/power8/strncpy.S b/sysdeps/powerpc/powerpc64/power8/strncpy.S
index e68453bcaa..eef3127e0d 100644
--- a/sysdeps/powerpc/powerpc64/power8/strncpy.S
+++ b/sysdeps/powerpc/powerpc64/power8/strncpy.S
@@ -80,27 +80,27 @@  ENTRY (FUNC_NAME, 4)
 	addi	r10,r4,16
 	rlwinm	r9,r4,0,19,19
 
-	/* Save some non-volatile registers on the stack.  */
-	std	r26,-48(r1)
-	std	r27,-40(r1)
+	/* Save some non-volatile registers on the stack after ROP slot */
+	std	r26,-56(r1)
+	std	r27,-48(r1)
 
 	rlwinm	r8,r10,0,19,19
 
-	std	r28,-32(r1)
-	std	r29,-24(r1)
+	std	r28,-40(r1)
+	std	r29,-32(r1)
 
 	cmpld	cr7,r9,r8
 
-	std	r30,-16(r1)
-	std	r31,-8(r1)
+	std	r30,-24(r1)
+	std	r31,-16(r1)
 
 	/* Update CFI.  */
-	cfi_offset(r26, -48)
-	cfi_offset(r27, -40)
-	cfi_offset(r28, -32)
-	cfi_offset(r29, -24)
-	cfi_offset(r30, -16)
-	cfi_offset(r31, -8)
+	cfi_offset(r26, -56)
+	cfi_offset(r27, -48)
+	cfi_offset(r28, -40)
+	cfi_offset(r29, -32)
+	cfi_offset(r30, -24)
+	cfi_offset(r31, -16)
 
 	beq	cr7,L(unaligned_lt_16)
 	rldicl	r9,r4,0,61
@@ -205,12 +205,12 @@  L(short_path_loop_end_1):
 #endif
 L(short_path_loop_end):
 	/* Restore non-volatile registers.  */
-	ld	r26,-48(r1)
-	ld	r27,-40(r1)
-	ld	r28,-32(r1)
-	ld	r29,-24(r1)
-	ld	r30,-16(r1)
-	ld	r31,-8(r1)
+	ld	r26,-56(r1)
+	ld	r27,-48(r1)
+	ld	r28,-40(r1)
+	ld	r29,-32(r1)
+	ld	r30,-24(r1)
+	ld	r31,-16(r1)
 	blr
 
 	/* This code pads the remainder of dest with NULL bytes.  The algorithm
@@ -242,6 +242,9 @@  L(zero_pad_start_1):
 	mflr	r0
 	std	r0,16(r1)
 
+#ifdef  __ROP_PROTECT__
+        hashst 0,FRAME_ROP_SAVE(r1)
+#endif
 	/* Create the stack frame.  */
 	stdu	r1,-FRAMESIZE(r1)
 	cfi_adjust_cfa_offset(FRAMESIZE)
@@ -261,18 +264,21 @@  L(zero_pad_start_1):
 #endif
 
 	/* Restore non-volatile registers and return.  */
-	ld	r26,FRAMESIZE-48(r1)
-	ld	r27,FRAMESIZE-40(r1)
-	ld	r28,FRAMESIZE-32(r1)
-	ld	r29,FRAMESIZE-24(r1)
-	ld	r30,FRAMESIZE-16(r1)
-	ld	r31,FRAMESIZE-8(r1)
+	ld	r26,FRAMESIZE-56(r1)
+	ld	r27,FRAMESIZE-48(r1)
+	ld	r28,FRAMESIZE-40(r1)
+	ld	r29,FRAMESIZE-32(r1)
+	ld	r30,FRAMESIZE-24(r1)
+	ld	r31,FRAMESIZE-16(r1)
 	/* Restore the stack frame.  */
 	addi	r1,r1,FRAMESIZE
 	cfi_adjust_cfa_offset(-FRAMESIZE)
 	/* Restore the link register.  */
 	mtlr	r0
 	cfi_restore(lr)
+#ifdef  __ROP_PROTECT__
+        hashchk 0,FRAME_ROP_SAVE(r1)
+#endif
 	blr
 
 	/* The common case where [src]+16 will not cross a 4K page boundary.
diff --git a/sysdeps/powerpc/powerpc64/ppc-mcount.S b/sysdeps/powerpc/powerpc64/ppc-mcount.S
index 7296d6a87d..c2b3cb2f8c 100644
--- a/sysdeps/powerpc/powerpc64/ppc-mcount.S
+++ b/sysdeps/powerpc/powerpc64/ppc-mcount.S
@@ -25,6 +25,9 @@  ENTRY(_mcount)
 	mflr		 r4
 	ld		 r11, 0(r1)
 	std		 r4, FRAME_LR_SAVE(r1)
+#ifdef  __ROP_PROTECT__
+        hashst r4, FRAME_ROP_SAVE(r1)
+#endif
 	stdu		 r1,-FRAME_MIN_SIZE(r1)
 	cfi_adjust_cfa_offset (FRAME_MIN_SIZE)
 	cfi_offset (lr, FRAME_LR_SAVE)
@@ -36,5 +39,8 @@  ENTRY(_mcount)
 	ld		 r0, FRAME_MIN_SIZE+FRAME_LR_SAVE(r1)
 	mtlr		 r0
 	addi		 r1,r1,FRAME_MIN_SIZE
+#ifdef  __ROP_PROTECT__
+        hashchk 0, FRAME_ROP_SAVE(r1)
+#endif
 	blr
 END(_mcount)
diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h
index b5c70e526e..a545b409eb 100644
--- a/sysdeps/powerpc/powerpc64/sysdep.h
+++ b/sysdeps/powerpc/powerpc64/sysdep.h
@@ -32,7 +32,7 @@ 
 #define FRAME_PARM_SAVE		48
 #else
 #define FRAME_ROP_SAVE		-8
-#define FRAME_MIN_SIZE		32
+#define FRAME_MIN_SIZE		48 /* Includes space for the ROP save slot */
 #define FRAME_MIN_SIZE_PARM	112 /* Includes space for the ROP save slot */
 #define FRAME_TOC_SAVE		24
 #define FRAME_PARM_SAVE		32