[PATCH] powerpc64le: ROP changes for clone/clone3

[Peter Bergner]

On 10/9/24 2:37 PM, Adhemerval Zanella Netto wrote:
> Do we really need ROP-mitigation on clone/clone3? The initial stack
> frame is only created so the child can save its TOC (so the initial
> thread function seems as a normal execution flow), and the LR won't
> be used to return after the thread function returns (pthread_create
> will issue exit).

I think you mean so the child can save its LR, since the LR is saved
in the caller's stack frame.  If the child needs to save its TOC,
then that is stored in the child's own frame.  But understood about
why there is a parent frame.

Looking at the disassembly of __clone, I do see some bltlr+ and
bnslr+ instructions following the restore the the LR, so it does
seem to be used.  Maybe an error path?  So I don't think we can
just remove the save/store of the LR altogether.  But you know this
code better than I do though.

That said, in your commit c579f48edb "Remove cached PID/TID in clone",
you removed the only use of r29, but you failed to remove the saving
and restoring of r29 or the copy of r5 into r29.  That is serendipitous,
as that leaves the non-volatile r29 free to use to save the LR across
the scv instruction as Florian suggested, so ROP wouldn't be required
at all.

clone3.S isn't as easy, since there is no saved/restored non-volatile
reg already available, but we could add saving/restoring of r31 similar
to clone.S and then use that to save/restore the LR.

Does the following untested patch to clone.S look correct to you?

I assume we don't care whether the kernel needs to use the same
non-volatile register so it may save our LR value via the non-volatile
reg on the kernel's stack frame, since that frame isn't accessible
to user code to corrupt?

Peter



diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S b/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S
index 164311d2bd..e57cb6e82e 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S
@@ -56,7 +56,6 @@ ENTRY (__clone)
 
        /* Save fn, args, stack across syscall.  */
        mr      r30,r3                  /* Function in r30.  */
-       mr      r29,r5                  /* Flags in r29.  */
        mr      r31,r6                  /* Argument in r31.  */
 
        /* 'flags' argument is first parameter to clone syscall.
@@ -77,14 +76,12 @@ ENTRY (__clone)
        CHECK_SCV_SUPPORT r28 0f
        /* This is equivalent to DO_CALL_SCV, but we cannot use the macro here
        because it uses CFI directives and we just called cfi_endproc.  */
-       mflr    r9
-       std     r9,FRAME_LR_SAVE(r1)
+       mflr    r29
        .machine "push"
        .machine "power9"
        scv     0
        .machine "pop"
-       ld      r9,FRAME_LR_SAVE(r1)
-       mtlr    r9
+       mtlr    r29
 
        /* Check for child process.  */
        /* When using scv, error is indicated by negative r3.  */