* [PATCH v2] powerpc64le: _init/_fini file changes for ROP
@ 2024-10-30 21:18 Sachin Monga
2024-11-04 19:11 ` Peter Bergner
2024-11-14 14:58 ` [PATCH v3] " Sachin Monga
0 siblings, 2 replies; 3+ messages in thread
From: Sachin Monga @ 2024-10-30 21:18 UTC (permalink / raw)
To: libc-alpha; +Cc: bergner, murphyp, Sachin Monga
The ROP instructions were added in ISA 3.1 (ie, Power10), however they
were defined so that if executed on older cpus, they would behave as
nops. This allows us to emit them on older cpus and they'd just be
ignored, but if run on a Power10, then the binary would be ROP protected.
-mrop-protect is needed to compile glibc for ROP enablement.
However, power7 and earlier files should not use it.
Hash instructions use negative offsets so the default position
of ROP pointer is FRAME_ROP_SAVE from caller's SP.
Modified FRAME_MIN_SIZE_PARM to 112 for elfAbi2 to reserve
additional 16 bytes for ROP save slot and padding.
Signed-off-by: Sachin Monga <smonga@linux.ibm.com>
---
The patch was built on powerpc64le-linux and regression tested
with no errors.
sysdeps/powerpc/powerpc64/crti.S | 6 ++++++
sysdeps/powerpc/powerpc64/crtn.S | 6 ++++++
sysdeps/powerpc/powerpc64/multiarch/Makefile | 2 ++
sysdeps/powerpc/powerpc64/sysdep.h | 4 ++--
4 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/sysdeps/powerpc/powerpc64/crti.S b/sysdeps/powerpc/powerpc64/crti.S
index 71bdddfb3b..e977bc4b9c 100644
--- a/sysdeps/powerpc/powerpc64/crti.S
+++ b/sysdeps/powerpc/powerpc64/crti.S
@@ -68,6 +68,9 @@ BODY_LABEL (_init):
LOCALENTRY(_init)
mflr 0
std 0, FRAME_LR_SAVE(r1)
+#ifdef __ROP_PROTECT__
+ hashst 0, FRAME_ROP_SAVE(r1)
+#endif
stdu r1, -FRAME_MIN_SIZE_PARM(r1)
#if PREINIT_FUNCTION_WEAK
addis r9, r2, .LC0@toc@ha
@@ -87,4 +90,7 @@ BODY_LABEL (_fini):
LOCALENTRY(_fini)
mflr 0
std 0, FRAME_LR_SAVE(r1)
+#ifdef __ROP_PROTECT__
+ hashst 0, FRAME_ROP_SAVE(r1)
+#endif
stdu r1, -FRAME_MIN_SIZE_PARM(r1)
diff --git a/sysdeps/powerpc/powerpc64/crtn.S b/sysdeps/powerpc/powerpc64/crtn.S
index 4e91231f2c..a37e159950 100644
--- a/sysdeps/powerpc/powerpc64/crtn.S
+++ b/sysdeps/powerpc/powerpc64/crtn.S
@@ -42,10 +42,16 @@
addi r1, r1, FRAME_MIN_SIZE_PARM
ld r0, FRAME_LR_SAVE(r1)
mtlr r0
+#ifdef __ROP_PROTECT__
+ hashchk 0, FRAME_ROP_SAVE(r1)
+#endif
blr
.section .fini,"ax",@progbits
addi r1, r1, FRAME_MIN_SIZE_PARM
ld r0, FRAME_LR_SAVE(r1)
mtlr r0
+#ifdef __ROP_PROTECT__
+ hashchk 0, FRAME_ROP_SAVE(r1)
+#endif
blr
diff --git a/sysdeps/powerpc/powerpc64/multiarch/Makefile b/sysdeps/powerpc/powerpc64/multiarch/Makefile
index b847c19049..840e517dad 100644
--- a/sysdeps/powerpc/powerpc64/multiarch/Makefile
+++ b/sysdeps/powerpc/powerpc64/multiarch/Makefile
@@ -38,7 +38,9 @@ sysdep_routines += memchr-power10 memcmp-power10 memcpy-power10 \
strlen-power9 strncpy-power9 stpncpy-power9 strlen-power10
endif
CFLAGS-strncase-power7.c += -mcpu=power7 -funroll-loops
+CFLAGS-strncase-power7.c := $(filter-out -mrop-protect, $(CFLAGS-strncase-power7))
CFLAGS-strncase_l-power7.c += -mcpu=power7 -funroll-loops
+CFLAGS-strncase_l-power7.c := $(filter-out -mrop-protect, $(CFLAGS-strncase-power7_l))
endif
# Called during static initialization
diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h
index c439b06121..ba614172ed 100644
--- a/sysdeps/powerpc/powerpc64/sysdep.h
+++ b/sysdeps/powerpc/powerpc64/sysdep.h
@@ -24,15 +24,15 @@
/* Stack frame offsets. */
#define FRAME_BACKCHAIN 0
#define FRAME_CR_SAVE 8
+#define FRAME_ROP_SAVE -8 /* Default ROP slot */
#define FRAME_LR_SAVE 16
+#define FRAME_MIN_SIZE_PARM 112 /* ++ROP ++Padding for _CALL_ELF=2 */
#if _CALL_ELF != 2
#define FRAME_MIN_SIZE 112
-#define FRAME_MIN_SIZE_PARM 112
#define FRAME_TOC_SAVE 40
#define FRAME_PARM_SAVE 48
#else
#define FRAME_MIN_SIZE 32
-#define FRAME_MIN_SIZE_PARM 96
#define FRAME_TOC_SAVE 24
#define FRAME_PARM_SAVE 32
#endif
--
2.47.0
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] powerpc64le: _init/_fini file changes for ROP
2024-10-30 21:18 [PATCH v2] powerpc64le: _init/_fini file changes for ROP Sachin Monga
@ 2024-11-04 19:11 ` Peter Bergner
2024-11-14 14:58 ` [PATCH v3] " Sachin Monga
1 sibling, 0 replies; 3+ messages in thread
From: Peter Bergner @ 2024-11-04 19:11 UTC (permalink / raw)
To: Sachin Monga, libc-alpha; +Cc: murphyp
On 10/30/24 4:18 PM, Sachin Monga wrote:
> Modified FRAME_MIN_SIZE_PARM to 112 for elfAbi2 to reserve
s/elfAbi2/ELFv2/
> diff --git a/sysdeps/powerpc/powerpc64/multiarch/Makefile b/sysdeps/powerpc/powerpc64/multiarch/Makefile
> index b847c19049..840e517dad 100644
> --- a/sysdeps/powerpc/powerpc64/multiarch/Makefile
> +++ b/sysdeps/powerpc/powerpc64/multiarch/Makefile
> @@ -38,7 +38,9 @@ sysdep_routines += memchr-power10 memcmp-power10 memcpy-power10 \
> strlen-power9 strncpy-power9 stpncpy-power9 strlen-power10
> endif
> CFLAGS-strncase-power7.c += -mcpu=power7 -funroll-loops
> +CFLAGS-strncase-power7.c := $(filter-out -mrop-protect, $(CFLAGS-strncase-power7))
> CFLAGS-strncase_l-power7.c += -mcpu=power7 -funroll-loops
> +CFLAGS-strncase_l-power7.c := $(filter-out -mrop-protect, $(CFLAGS-strncase-power7_l))
> endif
This removes using -mrop-protect from the power7 tuned files, but how does that
stop us from using -mrop-protect on the power4 through power6 specific files?
I believe you're currently enabling ROP in the build by adding -mrop-protect to
CFLAGS. Would it be better to do something similar to how we build for a specific
cpu? Ie, instead of adding -mcpu=XXX to CFLAGS, we use the --with-cpu=XXX config
option. Maybe we could do something similar via --enable-... or --with-rop={yes,no}?
I'm unsure if that would end up being similar in the end, but it is worth checking on.
> diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h
> index c439b06121..ba614172ed 100644
> --- a/sysdeps/powerpc/powerpc64/sysdep.h
> +++ b/sysdeps/powerpc/powerpc64/sysdep.h
> @@ -24,15 +24,15 @@
> /* Stack frame offsets. */
> #define FRAME_BACKCHAIN 0
> #define FRAME_CR_SAVE 8
> +#define FRAME_ROP_SAVE -8 /* Default ROP slot */
> #define FRAME_LR_SAVE 16
> +#define FRAME_MIN_SIZE_PARM 112 /* ++ROP ++Padding for _CALL_ELF=2 */
> #if _CALL_ELF != 2
> #define FRAME_MIN_SIZE 112
> -#define FRAME_MIN_SIZE_PARM 112
> #define FRAME_TOC_SAVE 40
> #define FRAME_PARM_SAVE 48
> #else
> #define FRAME_MIN_SIZE 32
> -#define FRAME_MIN_SIZE_PARM 96
> #define FRAME_TOC_SAVE 24
> #define FRAME_PARM_SAVE 32
> #endif
Please don't combine FRAME_MIN_SIZE_PARM into a single entry outside the
_CALL_ELF test. I know the ELFv1 and ELFv2 values are now the same, but
the plan is to eventually enable this from non ELFv2 compiles too, in which
case we'll have to update its FRAME_MIN_SIZE_PARM and then they'll diverge
again. Better to just keep the separate macros for now. Also please move
the FRAME_ROP_SAVE definition to the ELFv2 section. This should catch any
current non ELFv2 usage of that and cause an error. If the ELFv1 value of
that ends up being the same, then we can combine it later.
Peter
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v3] powerpc64le: _init/_fini file changes for ROP
2024-10-30 21:18 [PATCH v2] powerpc64le: _init/_fini file changes for ROP Sachin Monga
2024-11-04 19:11 ` Peter Bergner
@ 2024-11-14 14:58 ` Sachin Monga
1 sibling, 0 replies; 3+ messages in thread
From: Sachin Monga @ 2024-11-14 14:58 UTC (permalink / raw)
To: bergner, libc-alpha; +Cc: sachin.monga, murphyp, Sachin Monga
The ROP instructions were added in ISA 3.1 (ie, Power10), however they
were defined so that if executed on older cpus, they would behave as
nops. This allows us to emit them on older cpus and they'd just be
ignored, but if run on a Power10, then the binary would be ROP protected.
-mrop-protect is needed to compile glibc for ROP enablement.
However, power7 and earlier files should not use it.
Hash instructions use negative offsets so the default position
of ROP pointer is FRAME_ROP_SAVE from caller's SP.
Modified FRAME_MIN_SIZE_PARM to 112 for elfAbi2 to reserve
additional 16 bytes for ROP save slot and padding.
Signed-off-by: Sachin Monga <smonga@linux.ibm.com>
---
The patch is reg tested.
>This removes using -mrop-protect from the power7 tuned files, but how does that
>stop us from using -mrop-protect on the power4 through power6 specific files?
>I believe you're currently enabling ROP in the build by adding -mrop-protect to
>CFLAGS. Would it be better to do something similar to how we build for a specific
>cpu? Ie, instead of adding -mcpu=XXX to CFLAGS, we use the --with-cpu=XXX config
>option. Maybe we could do something similar via --enable-... or --with-rop={yes,no}?
>I'm unsure if that would end up being similar in the end, but it is worth checking on.
I agree. As discussed offline,I'll share a seperate patch on enablement of ROP through
configure script.
>Please don't combine FRAME_MIN_SIZE_PARM into a single entry outside the
>_CALL_ELF test. I know the ELFv1 and ELFv2 values are now the same, but
>the plan is to eventually enable this from non ELFv2 compiles too, in which
>case we'll have to update its FRAME_MIN_SIZE_PARM and then they'll diverge
>again. Better to just keep the separate macros for now. Also please move
>the FRAME_ROP_SAVE definition to the ELFv2 section. This should catch any
>current non ELFv2 usage of that and cause an error. If the ELFv1 value of
>that ends up being the same, then we can combine it later.
This is incorporated in the below revised patch.
sysdeps/powerpc/powerpc64/crti.S | 6 ++++++
sysdeps/powerpc/powerpc64/crtn.S | 6 ++++++
sysdeps/powerpc/powerpc64/multiarch/Makefile | 2 ++
sysdeps/powerpc/powerpc64/sysdep.h | 3 ++-
4 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/sysdeps/powerpc/powerpc64/crti.S b/sysdeps/powerpc/powerpc64/crti.S
index 71bdddfb3b..e977bc4b9c 100644
--- a/sysdeps/powerpc/powerpc64/crti.S
+++ b/sysdeps/powerpc/powerpc64/crti.S
@@ -68,6 +68,9 @@ BODY_LABEL (_init):
LOCALENTRY(_init)
mflr 0
std 0, FRAME_LR_SAVE(r1)
+#ifdef __ROP_PROTECT__
+ hashst 0, FRAME_ROP_SAVE(r1)
+#endif
stdu r1, -FRAME_MIN_SIZE_PARM(r1)
#if PREINIT_FUNCTION_WEAK
addis r9, r2, .LC0@toc@ha
@@ -87,4 +90,7 @@ BODY_LABEL (_fini):
LOCALENTRY(_fini)
mflr 0
std 0, FRAME_LR_SAVE(r1)
+#ifdef __ROP_PROTECT__
+ hashst 0, FRAME_ROP_SAVE(r1)
+#endif
stdu r1, -FRAME_MIN_SIZE_PARM(r1)
diff --git a/sysdeps/powerpc/powerpc64/crtn.S b/sysdeps/powerpc/powerpc64/crtn.S
index 4e91231f2c..a37e159950 100644
--- a/sysdeps/powerpc/powerpc64/crtn.S
+++ b/sysdeps/powerpc/powerpc64/crtn.S
@@ -42,10 +42,16 @@
addi r1, r1, FRAME_MIN_SIZE_PARM
ld r0, FRAME_LR_SAVE(r1)
mtlr r0
+#ifdef __ROP_PROTECT__
+ hashchk 0, FRAME_ROP_SAVE(r1)
+#endif
blr
.section .fini,"ax",@progbits
addi r1, r1, FRAME_MIN_SIZE_PARM
ld r0, FRAME_LR_SAVE(r1)
mtlr r0
+#ifdef __ROP_PROTECT__
+ hashchk 0, FRAME_ROP_SAVE(r1)
+#endif
blr
diff --git a/sysdeps/powerpc/powerpc64/multiarch/Makefile b/sysdeps/powerpc/powerpc64/multiarch/Makefile
index b847c19049..840e517dad 100644
--- a/sysdeps/powerpc/powerpc64/multiarch/Makefile
+++ b/sysdeps/powerpc/powerpc64/multiarch/Makefile
@@ -38,7 +38,9 @@ sysdep_routines += memchr-power10 memcmp-power10 memcpy-power10 \
strlen-power9 strncpy-power9 stpncpy-power9 strlen-power10
endif
CFLAGS-strncase-power7.c += -mcpu=power7 -funroll-loops
+CFLAGS-strncase-power7.c := $(filter-out -mrop-protect, $(CFLAGS-strncase-power7))
CFLAGS-strncase_l-power7.c += -mcpu=power7 -funroll-loops
+CFLAGS-strncase_l-power7.c := $(filter-out -mrop-protect, $(CFLAGS-strncase-power7_l))
endif
# Called during static initialization
diff --git a/sysdeps/powerpc/powerpc64/sysdep.h b/sysdeps/powerpc/powerpc64/sysdep.h
index c439b06121..8467f2b4e9 100644
--- a/sysdeps/powerpc/powerpc64/sysdep.h
+++ b/sysdeps/powerpc/powerpc64/sysdep.h
@@ -31,8 +31,9 @@
#define FRAME_TOC_SAVE 40
#define FRAME_PARM_SAVE 48
#else
+#define FRAME_ROP_SAVE -8 /* Default ROP slot */
#define FRAME_MIN_SIZE 32
-#define FRAME_MIN_SIZE_PARM 96
+#define FRAME_MIN_SIZE_PARM 112 /* ++ROP ++Padding for _CALL_ELF=2 */
#define FRAME_TOC_SAVE 24
#define FRAME_PARM_SAVE 32
#endif
--
2.47.0
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-11-14 14:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-10-30 21:18 [PATCH v2] powerpc64le: _init/_fini file changes for ROP Sachin Monga
2024-11-04 19:11 ` Peter Bergner
2024-11-14 14:58 ` [PATCH v3] " Sachin Monga
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).