[Qemu-devel] [PATCH 10/11] linux-user/aarch64: Reset btype for signal handlers

Richard Henderson posted 11 patches 7 years, 1 month ago
Maintainers: Laurent Vivier <laurent@vivier.eu>, Riku Voipio <riku.voipio@iki.fi>, Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
[Qemu-devel] [PATCH 10/11] linux-user/aarch64: Reset btype for signal handlers
Posted by Richard Henderson 7 years, 1 month ago
It does not make sense for a SIGILL handler to enter with the
btype set as for the indirect branch that caused the SIGILL.

Nor does it make sense to return from a handler with BTYPE set.
This could be argued to be the handler's job, setting BTYPE
within ucontext->uc_mcontext.pstate, but handling this here
while the ABI is undiscussed.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/aarch64/signal.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c
index f84a9cf28a..1fb229e696 100644
--- a/linux-user/aarch64/signal.c
+++ b/linux-user/aarch64/signal.c
@@ -218,6 +218,8 @@ static void target_restore_general_frame(CPUARMState *env,
     __get_user(env->pc, &sf->uc.tuc_mcontext.pc);
     __get_user(pstate, &sf->uc.tuc_mcontext.pstate);
     pstate_write(env, pstate);
+    /* Reset btype that might have been there going into the frame.  */
+    env->btype = 0;
 }
 
 static void target_restore_fpsimd_record(CPUARMState *env,
@@ -510,6 +512,8 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
     env->xregs[29] = frame_addr + fr_ofs;
     env->pc = ka->_sa_handler;
     env->xregs[30] = return_addr;
+    /* Reset btype going into the signal handler.  */
+    env->btype = 0;
     if (info) {
         tswap_siginfo(&frame->info, info);
         env->xregs[1] = frame_addr + offsetof(struct target_rt_sigframe, info);
-- 
2.17.2


Re: [Qemu-devel] [PATCH 10/11] linux-user/aarch64: Reset btype for signal handlers
Posted by Peter Maydell 7 years ago
On Thu, 10 Jan 2019 at 12:18, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> It does not make sense for a SIGILL handler to enter with the
> btype set as for the indirect branch that caused the SIGILL.
>
> Nor does it make sense to return from a handler with BTYPE set.
> This could be argued to be the handler's job, setting BTYPE
> within ucontext->uc_mcontext.pstate, but handling this here
> while the ABI is undiscussed.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/aarch64/signal.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c
> index f84a9cf28a..1fb229e696 100644
> --- a/linux-user/aarch64/signal.c
> +++ b/linux-user/aarch64/signal.c
> @@ -218,6 +218,8 @@ static void target_restore_general_frame(CPUARMState *env,
>      __get_user(env->pc, &sf->uc.tuc_mcontext.pc);
>      __get_user(pstate, &sf->uc.tuc_mcontext.pstate);
>      pstate_write(env, pstate);
> +    /* Reset btype that might have been there going into the frame.  */
> +    env->btype = 0;

Conceptually we should do this the way the kernel would, by
sanitizing the value of "pstate" before passing it to
pstate_write(). This is done in valid_native_regs() in
arch/arm64/kernel/ptrace.c and forbids other things like
messing with the DAIF bits or the mode bits.

>  }
>
>  static void target_restore_fpsimd_record(CPUARMState *env,
> @@ -510,6 +512,8 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
>      env->xregs[29] = frame_addr + fr_ofs;
>      env->pc = ka->_sa_handler;
>      env->xregs[30] = return_addr;
> +    /* Reset btype going into the signal handler.  */
> +    env->btype = 0;
>      if (info) {
>          tswap_siginfo(&frame->info, info);
>          env->xregs[1] = frame_addr + offsetof(struct target_rt_sigframe, info);
> --
> 2.17.2

thanks
-- PMM