[PATCH v3 05/27] linux-user/arm: Implement setup_sigtramp

Richard Henderson posted 27 patches 4 years, 4 months ago
Maintainers: Aurelien Jarno <aurelien@aurel32.net>, Chris Wulff <crwulff@gmail.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Laurent Vivier <laurent@vivier.eu>, Thomas Huth <thuth@redhat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Marek Vasut <marex@denx.de>, Cornelia Huck <cohuck@redhat.com>, Jiaxun Yang <jiaxun.yang@flygoat.com>
There is a newer version of this series
[PATCH v3 05/27] linux-user/arm: Implement setup_sigtramp
Posted by Richard Henderson 4 years, 4 months ago
Update the trampoline code to match the kernel: this uses
sp-relative accesses rather than pc-relative.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/arm/target_signal.h |   2 +
 linux-user/arm/signal.c        | 177 ++++++++++++++++++++++++---------
 2 files changed, 130 insertions(+), 49 deletions(-)

diff --git a/linux-user/arm/target_signal.h b/linux-user/arm/target_signal.h
index 0998dd6dfa..1e7fb0cecb 100644
--- a/linux-user/arm/target_signal.h
+++ b/linux-user/arm/target_signal.h
@@ -22,4 +22,6 @@ typedef struct target_sigaltstack {
 #include "../generic/signal.h"
 
 #define TARGET_ARCH_HAS_SETUP_FRAME
+#define TARGET_ARCH_HAS_SIGTRAMP_PAGE 1
+
 #endif /* ARM_TARGET_SIGNAL_H */
diff --git a/linux-user/arm/signal.c b/linux-user/arm/signal.c
index ed7d1d80bb..ccfae43028 100644
--- a/linux-user/arm/signal.c
+++ b/linux-user/arm/signal.c
@@ -103,39 +103,52 @@ struct rt_sigframe
     abi_ulong retcode[4];
 };
 
-/*
- * For ARM syscalls, we encode the syscall number into the instruction.
- */
-#define SWI_SYS_SIGRETURN       (0xef000000|(TARGET_NR_sigreturn + ARM_SYSCALL_BASE))
-#define SWI_SYS_RT_SIGRETURN    (0xef000000|(TARGET_NR_rt_sigreturn + ARM_SYSCALL_BASE))
+static abi_ptr sigreturn_fdpic_tramp;
 
 /*
- * For Thumb syscalls, we pass the syscall number via r7.  We therefore
- * need two 16-bit instructions.
+ * EABI syscalls pass the number via r7.
+ * Note that the kernel still adds the OABI syscall number to the trap,
+ * presumably for backward ABI compatibility with unwinders.
  */
-#define SWI_THUMB_SIGRETURN     (0xdf00 << 16 | 0x2700 | (TARGET_NR_sigreturn))
-#define SWI_THUMB_RT_SIGRETURN  (0xdf00 << 16 | 0x2700 | (TARGET_NR_rt_sigreturn))
+#define ARM_MOV_R7_IMM(X)       (0xe3a07000 | (X))
+#define ARM_SWI_SYS(X)          (0xef000000 | (X) | ARM_SYSCALL_BASE)
 
-static const abi_ulong retcodes[4] = {
-        SWI_SYS_SIGRETURN,      SWI_THUMB_SIGRETURN,
-        SWI_SYS_RT_SIGRETURN,   SWI_THUMB_RT_SIGRETURN
-};
+#define THUMB_MOVS_R7_IMM(X)    (0x2700 | (X))
+#define THUMB_SWI_SYS           0xdf00
+
+static void write_arm_sigreturn(uint32_t *rc, int syscall)
+{
+    __put_user(ARM_MOV_R7_IMM(syscall), rc);
+    __put_user(ARM_SWI_SYS(syscall), rc + 1);
+}
+
+static void write_thumb_sigreturn(uint32_t *rc, int syscall)
+{
+    __put_user(THUMB_SWI_SYS << 16 | THUMB_MOVS_R7_IMM(syscall), rc);
+}
 
 /*
- * Stub needed to make sure the FD register (r9) contains the right
- * value.
+ * Stub needed to make sure the FD register (r9) contains the right value.
+ * Use the same instruction sequence as the kernel.
  */
-static const unsigned long sigreturn_fdpic_codes[3] = {
-    0xe59fc004, /* ldr r12, [pc, #4] to read function descriptor */
-    0xe59c9004, /* ldr r9, [r12, #4] to setup GOT */
-    0xe59cf000  /* ldr pc, [r12] to jump into restorer */
-};
+static void write_arm_fdpic_sigreturn(uint32_t *rc, int ofs)
+{
+    assert(ofs <= 0xfff);
+    __put_user(0xe59d3000 | ofs, rc + 0);   /* ldr r3, [sp, #ofs] */
+    __put_user(0xe8930908, rc + 1);         /* ldm r3, { r3, r9 } */
+    __put_user(0xe12fff13, rc + 2);         /* bx  r3 */
+}
 
-static const unsigned long sigreturn_fdpic_thumb_codes[3] = {
-    0xc008f8df, /* ldr r12, [pc, #8] to read function descriptor */
-    0x9004f8dc, /* ldr r9, [r12, #4] to setup GOT */
-    0xf000f8dc  /* ldr pc, [r12] to jump into restorer */
-};
+static void write_thumb_fdpic_sigreturn(void *vrc, int ofs)
+{
+    uint16_t *rc = vrc;
+
+    assert((ofs & ~0x3fc) == 0);
+    __put_user(0x9b00 | (ofs >> 2), rc + 0);      /* ldr r3, [sp, #ofs] */
+    __put_user(0xcb0c, rc + 1);                   /* ldm r3, { r2, r3 } */
+    __put_user(0x4699, rc + 2);                   /* mov r9, r3 */
+    __put_user(0x4710, rc + 3);                   /* bx  r2 */
+}
 
 static inline int valid_user_regs(CPUARMState *regs)
 {
@@ -184,13 +197,12 @@ get_sigframe(struct target_sigaction *ka, CPUARMState *regs, int framesize)
 
 static int
 setup_return(CPUARMState *env, struct target_sigaction *ka,
-             abi_ulong *rc, abi_ulong frame_addr, int usig, abi_ulong rc_addr)
+             abi_ulong *rc, abi_ulong frame_addr, int usig, abi_ulong rc_ofs)
 {
     abi_ulong handler = 0;
     abi_ulong handler_fdpic_GOT = 0;
     abi_ulong retcode;
-
-    int thumb;
+    int thumb, retcode_idx;
     int is_fdpic = info_is_fdpic(((TaskState *)thread_cpu->opaque)->info);
 
     if (is_fdpic) {
@@ -208,6 +220,7 @@ setup_return(CPUARMState *env, struct target_sigaction *ka,
     }
 
     thumb = handler & 1;
+    retcode_idx = thumb + (ka->sa_flags & TARGET_SA_SIGINFO ? 2 : 0);
 
     uint32_t cpsr = cpsr_read(env);
 
@@ -225,37 +238,39 @@ setup_return(CPUARMState *env, struct target_sigaction *ka,
 
     if (ka->sa_flags & TARGET_SA_RESTORER) {
         if (is_fdpic) {
-            /* For FDPIC we ensure that the restorer is called with a
-             * correct r9 value.  For that we need to write code on
-             * the stack that sets r9 and jumps back to restorer
-             * value.
+            /*
+             * For FDPIC we ensure that the restorer is called with a
+             * correct r9 value.  For that we need a special trampoline
+             * the reads the function descriptor from the frame,
+             * sets r9 and jumps back to restorer value.
+             *
+             * This on-stack code is unused but retained for ABI compliance.
              */
             if (thumb) {
-                __put_user(sigreturn_fdpic_thumb_codes[0], rc);
-                __put_user(sigreturn_fdpic_thumb_codes[1], rc + 1);
-                __put_user(sigreturn_fdpic_thumb_codes[2], rc + 2);
-                __put_user((abi_ulong)ka->sa_restorer, rc + 3);
+                write_thumb_fdpic_sigreturn(rc, rc_ofs + 12);
             } else {
-                __put_user(sigreturn_fdpic_codes[0], rc);
-                __put_user(sigreturn_fdpic_codes[1], rc + 1);
-                __put_user(sigreturn_fdpic_codes[2], rc + 2);
-                __put_user((abi_ulong)ka->sa_restorer, rc + 3);
+                write_arm_fdpic_sigreturn(rc, rc_ofs + 12);
             }
+            __put_user((abi_ulong)ka->sa_restorer, &rc[3]);
 
-            retcode = rc_addr + thumb;
+            /* Each trampoline variant consumes a 12-byte slot. */
+            retcode = sigreturn_fdpic_tramp + retcode_idx * 12 + thumb;
         } else {
             retcode = ka->sa_restorer;
         }
     } else {
-        unsigned int idx = thumb;
+        int syscall = (ka->sa_flags & TARGET_SA_SIGINFO
+                       ? TARGET_NR_rt_sigreturn : TARGET_NR_sigreturn);
 
-        if (ka->sa_flags & TARGET_SA_SIGINFO) {
-            idx += 2;
+        /* This on-stack code is unused but retained for ABI compliance. */
+        if (thumb) {
+            write_thumb_sigreturn(rc, syscall);
+        } else {
+            write_arm_sigreturn(rc, syscall);
         }
 
-        __put_user(retcodes[idx], rc);
-
-        retcode = rc_addr + thumb;
+        /* Each trampoline variant consumes 8-byte slot. */
+        retcode = default_sigreturn + retcode_idx * 8 + thumb;
     }
 
     env->regs[0] = usig;
@@ -352,7 +367,7 @@ void setup_frame(int usig, struct target_sigaction *ka,
     setup_sigframe(&frame->uc, set, regs);
 
     if (setup_return(regs, ka, frame->retcode, frame_addr, usig,
-                     frame_addr + offsetof(struct sigframe, retcode))) {
+                     offsetof(struct sigframe, retcode))) {
         goto sigsegv;
     }
 
@@ -383,7 +398,7 @@ void setup_rt_frame(int usig, struct target_sigaction *ka,
     setup_sigframe(&frame->uc, set, env);
 
     if (setup_return(env, ka, frame->retcode, frame_addr, usig,
-                     frame_addr + offsetof(struct rt_sigframe, retcode))) {
+                     offsetof(struct rt_sigframe, retcode))) {
         goto sigsegv;
     }
 
@@ -591,3 +606,67 @@ badframe:
     force_sig(TARGET_SIGSEGV);
     return -TARGET_QEMU_ESIGRETURN;
 }
+
+void setup_sigtramp(abi_ulong sigtramp_page)
+{
+    enum {
+        SIGFRAME_FDPIC_OFS = offsetof(struct sigframe, retcode[3]),
+        RT_SIGFRAME_FDPIC_OFS = offsetof(struct rt_sigframe, retcode[3]),
+    };
+
+    uint32_t total_size = 4 * 8 + 4 * 12;
+    uint32_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, total_size, 0);
+    uint32_t i = 0;
+
+    assert(tramp != NULL);
+
+    default_sigreturn = sigtramp_page;
+    write_arm_sigreturn(&tramp[i], TARGET_NR_sigreturn);
+    i += 2;
+    write_thumb_sigreturn(&tramp[i], TARGET_NR_sigreturn);
+    i += 2;
+    write_arm_sigreturn(&tramp[i], TARGET_NR_rt_sigreturn);
+    i += 2;
+    write_thumb_sigreturn(&tramp[i], TARGET_NR_rt_sigreturn);
+    i += 2;
+
+    /*
+     * FDPIC require trampolines to call sa_restorer, and different
+     * from the pc-relative versions we write to the stack.
+     *
+     * ARM versions use:
+     *    ldr   r3, [sp, #ofs]
+     *    ldr   r9, [r3, #4]
+     *    ldr   pc, [r3, #0]
+     *
+     * Thumb versions use:
+     *    ldr   r3, [sp, #ofs]
+     *    ldmia r3, {r2, r3}
+     *    mov   r9, r3
+     *    bx    r2
+     */
+    sigreturn_fdpic_tramp = sigtramp_page + i * 4;
+
+    /* ARM sigframe */
+    write_arm_fdpic_sigreturn(tramp + i,
+                              offsetof(struct sigframe, retcode[3]));
+    i += 3;
+
+    /* Thumb sigframe */
+    write_thumb_fdpic_sigreturn(tramp + i,
+                                offsetof(struct sigframe, retcode[3]));
+    i += 3;
+
+    /* ARM rt_sigframe */
+    write_arm_fdpic_sigreturn(tramp + i,
+                              offsetof(struct rt_sigframe, retcode[3]));
+    i += 3;
+
+    /* Thumb rt_sigframe */
+    write_thumb_fdpic_sigreturn(tramp + i,
+                                offsetof(struct rt_sigframe, retcode[3]));
+    i += 3;
+
+    assert(i * 4 == total_size);
+    unlock_user(tramp, sigtramp_page, total_size);
+}
-- 
2.25.1


Re: [PATCH v3 05/27] linux-user/arm: Implement setup_sigtramp
Posted by Peter Maydell 4 years, 4 months ago
On Fri, 24 Sept 2021 at 17:59, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Update the trampoline code to match the kernel: this uses
> sp-relative accesses rather than pc-relative.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

These functions must write at most 8 bytes:

> +static void write_arm_sigreturn(uint32_t *rc, int syscall)
> +{
> +    __put_user(ARM_MOV_R7_IMM(syscall), rc);
> +    __put_user(ARM_SWI_SYS(syscall), rc + 1);
> +}
> +
> +static void write_thumb_sigreturn(uint32_t *rc, int syscall)
> +{
> +    __put_user(THUMB_SWI_SYS << 16 | THUMB_MOVS_R7_IMM(syscall), rc);
> +}
>
>  /*
> - * Stub needed to make sure the FD register (r9) contains the right
> - * value.
> + * Stub needed to make sure the FD register (r9) contains the right value.
> + * Use the same instruction sequence as the kernel.
>   */
> -static const unsigned long sigreturn_fdpic_codes[3] = {
> -    0xe59fc004, /* ldr r12, [pc, #4] to read function descriptor */
> -    0xe59c9004, /* ldr r9, [r12, #4] to setup GOT */
> -    0xe59cf000  /* ldr pc, [r12] to jump into restorer */
> -};

...and these must write at most 12 bytes. But nothing states
or asserts that.

> +static void write_arm_fdpic_sigreturn(uint32_t *rc, int ofs)
> +{
> +    assert(ofs <= 0xfff);
> +    __put_user(0xe59d3000 | ofs, rc + 0);   /* ldr r3, [sp, #ofs] */
> +    __put_user(0xe8930908, rc + 1);         /* ldm r3, { r3, r9 } */
> +    __put_user(0xe12fff13, rc + 2);         /* bx  r3 */
> +}
>
> -static const unsigned long sigreturn_fdpic_thumb_codes[3] = {
> -    0xc008f8df, /* ldr r12, [pc, #8] to read function descriptor */
> -    0x9004f8dc, /* ldr r9, [r12, #4] to setup GOT */
> -    0xf000f8dc  /* ldr pc, [r12] to jump into restorer */
> -};
> +static void write_thumb_fdpic_sigreturn(void *vrc, int ofs)
> +{
> +    uint16_t *rc = vrc;
> +
> +    assert((ofs & ~0x3fc) == 0);
> +    __put_user(0x9b00 | (ofs >> 2), rc + 0);      /* ldr r3, [sp, #ofs] */
> +    __put_user(0xcb0c, rc + 1);                   /* ldm r3, { r2, r3 } */
> +    __put_user(0x4699, rc + 2);                   /* mov r9, r3 */
> +    __put_user(0x4710, rc + 3);                   /* bx  r2 */
> +}
>

> -            retcode = rc_addr + thumb;
> +            /* Each trampoline variant consumes a 12-byte slot. */
> +            retcode = sigreturn_fdpic_tramp + retcode_idx * 12 + thumb;
>          } else {
>              retcode = ka->sa_restorer;
>          }
>      } else {

> -
> -        retcode = rc_addr + thumb;
> +        /* Each trampoline variant consumes 8-byte slot. */
> +        retcode = default_sigreturn + retcode_idx * 8 + thumb;

These 12 and 8 magic numbers correspond to the maximum sequence sizes
above...

> +void setup_sigtramp(abi_ulong sigtramp_page)
> +{
> +    enum {
> +        SIGFRAME_FDPIC_OFS = offsetof(struct sigframe, retcode[3]),
> +        RT_SIGFRAME_FDPIC_OFS = offsetof(struct rt_sigframe, retcode[3]),
> +    };
> +
> +    uint32_t total_size = 4 * 8 + 4 * 12;
> +    uint32_t *tramp = lock_user(VERIFY_WRITE, sigtramp_page, total_size, 0);
> +    uint32_t i = 0;
> +
> +    assert(tramp != NULL);
> +
> +    default_sigreturn = sigtramp_page;
> +    write_arm_sigreturn(&tramp[i], TARGET_NR_sigreturn);
> +    i += 2;
> +    write_thumb_sigreturn(&tramp[i], TARGET_NR_sigreturn);
> +    i += 2;
> +    write_arm_sigreturn(&tramp[i], TARGET_NR_rt_sigreturn);
> +    i += 2;
> +    write_thumb_sigreturn(&tramp[i], TARGET_NR_rt_sigreturn);
> +    i += 2;

...and these "+=2" and the "+=3" later do as well, but with
a count of 32-bit words rather than bytes. I think it would be
useful to at least have some defined constants for the lengths
rather than hard-coded 8,12,2,3, and comments that the write_
functions must not write more than however-many bytes.

> +
> +    /*
> +     * FDPIC require trampolines to call sa_restorer, and different
> +     * from the pc-relative versions we write to the stack.
> +     *
> +     * ARM versions use:
> +     *    ldr   r3, [sp, #ofs]
> +     *    ldr   r9, [r3, #4]
> +     *    ldr   pc, [r3, #0]

This comment doesn't match the code that write_arm_fdpic_sigreturn()
now generates. The "different from the pc-relative versions we
write from the stack" bit doesn't seem to be right either, given
we call the same functions in both places to write the code.

> +     *
> +     * Thumb versions use:
> +     *    ldr   r3, [sp, #ofs]
> +     *    ldmia r3, {r2, r3}
> +     *    mov   r9, r3
> +     *    bx    r2
> +     */
> +    sigreturn_fdpic_tramp = sigtramp_page + i * 4;
> +
> +    /* ARM sigframe */
> +    write_arm_fdpic_sigreturn(tramp + i,
> +                              offsetof(struct sigframe, retcode[3]));
> +    i += 3;
> +
> +    /* Thumb sigframe */
> +    write_thumb_fdpic_sigreturn(tramp + i,
> +                                offsetof(struct sigframe, retcode[3]));
> +    i += 3;
> +
> +    /* ARM rt_sigframe */
> +    write_arm_fdpic_sigreturn(tramp + i,
> +                              offsetof(struct rt_sigframe, retcode[3]));
> +    i += 3;
> +
> +    /* Thumb rt_sigframe */
> +    write_thumb_fdpic_sigreturn(tramp + i,
> +                                offsetof(struct rt_sigframe, retcode[3]));
> +    i += 3;
> +
> +    assert(i * 4 == total_size);
> +    unlock_user(tramp, sigtramp_page, total_size);
> +}
> --
> 2.25.1

thanks
-- PMM