On Fri, 24 Sept 2021 at 17:59, Richard Henderson
<richard.hender...@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.hender...@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
