diff mbox

[v2,05/13] signal/all: remove return value from restore_sigcontext

Message ID 1402048017-30604-6-git-send-email-riku.voipio@linaro.org
State Accepted
Headers show

Commit Message

Riku Voipio June 6, 2014, 9:46 a.m. UTC 
From: Riku Voipio <riku.voipio@linaro.org>

make most implementations of restore_sigcontext void and
remove checking it's return value from functions calling
restore_sigcontext.

The exception is the X86 version of the function that is
too different from others to deal in this way.

Signed-off-by: Riku Voipio <riku.voipio@linaro.org>
---
 linux-user/signal.c | 65 +++++++++++++----------------------------------------
 1 file changed, 16 insertions(+), 49 deletions(-)

Comments

Peter Maydell June 7, 2014, 9:30 p.m. UTC | #1 
On 6 June 2014 10:46,  <riku.voipio@linaro.org> wrote:
> From: Riku Voipio <riku.voipio@linaro.org>
>
> make most implementations of restore_sigcontext void and
> remove checking it's return value from functions calling
> restore_sigcontext.
>
> The exception is the X86 version of the function that is
> too different from others to deal in this way.
>
> Signed-off-by: Riku Voipio <riku.voipio@linaro.org>
> ---
>  linux-user/signal.c | 65 +++++++++++++----------------------------------------
>  1 file changed, 16 insertions(+), 49 deletions(-)
>
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 421bd48..7b828bf 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -1546,12 +1546,6 @@ static const abi_ulong retcodes[4] = {
>         SWI_SYS_RT_SIGRETURN,   SWI_THUMB_RT_SIGRETURN
>  };
>
> -
> -static inline int valid_user_regs(CPUARMState *regs)
> -{
> -    return 1;
> -}

I don't think we should remove this function -- there are a number
of checks we should be making here for correct behaviour to
ensure that the guest hasn't passed us a bogus CPSR. Compare
the kernel's version of this function:

http://lxr.linux.no/#linux+v3.14.5/arch/arm/include/asm/ptrace.h#L46

We basically want all those checks (except we can drop the
ones related to 26-bit CPUs and M-profile). We don't need to
implement them in this series but we shouldn't remove the function
and code path which are the right places to add them later.

thanks
-- PMM
diff mbox

Patch

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 421bd48..7b828bf 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1546,12 +1546,6 @@  static const abi_ulong retcodes[4] = {
 	SWI_SYS_RT_SIGRETURN,	SWI_THUMB_RT_SIGRETURN
 };
 
-
-static inline int valid_user_regs(CPUARMState *regs)
-{
-    return 1;
-}
-
 static void
 setup_sigcontext(struct target_sigcontext *sc, /*struct _fpstate *fpstate,*/
                  CPUARMState *env, abi_ulong mask)
@@ -1842,10 +1836,9 @@  static void setup_rt_frame(int usig, struct target_sigaction *ka,
     }
 }
 
-static int
+static void
 restore_sigcontext(CPUARMState *env, struct target_sigcontext *sc)
 {
-	int err = 0;
         uint32_t cpsr;
 
     __get_user(env->regs[0], &sc->arm_r0);
@@ -1868,10 +1861,6 @@  restore_sigcontext(CPUARMState *env, struct target_sigcontext *sc)
     __get_user(cpsr, &sc->arm_cpsr);
         cpsr_write(env, cpsr, CPSR_USER | CPSR_EXEC);
 #endif
-
-	err |= !valid_user_regs(env);
-
-	return err;
 }
 
 static long do_sigreturn_v1(CPUARMState *env)
@@ -1905,8 +1894,7 @@  static long do_sigreturn_v1(CPUARMState *env)
         target_to_host_sigset_internal(&host_set, &set);
         do_sigprocmask(SIG_SETMASK, &host_set, NULL);
 
-	if (restore_sigcontext(env, &frame->sc))
-		goto badframe;
+    restore_sigcontext(env, &frame->sc);
 
 #if 0
 	/* Send SIGTRAP if we're single-stepping */
@@ -1986,8 +1974,7 @@  static int do_sigframe_return_v2(CPUARMState *env, target_ulong frame_addr,
     target_to_host_sigset(&host_set, &uc->tuc_sigmask);
     do_sigprocmask(SIG_SETMASK, &host_set, NULL);
 
-    if (restore_sigcontext(env, &uc->tuc_mcontext))
-        return 1;
+    restore_sigcontext(env, &uc->tuc_mcontext);
 
     /* Restore coprocessor signal frame */
     regspace = uc->tuc_regspace;
@@ -2077,8 +2064,7 @@  static long do_rt_sigreturn_v1(CPUARMState *env)
         target_to_host_sigset(&host_set, &frame->uc.tuc_sigmask);
         do_sigprocmask(SIG_SETMASK, &host_set, NULL);
 
-	if (restore_sigcontext(env, &frame->uc.tuc_mcontext))
-		goto badframe;
+    restore_sigcontext(env, &frame->uc.tuc_mcontext);
 
 	if (do_sigaltstack(frame_addr + offsetof(struct rt_sigframe_v1, uc.tuc_stack), 0, get_sp_from_cpustate(env)) == -EFAULT)
 		goto badframe;
@@ -2889,10 +2875,9 @@  static inline void setup_sigcontext(CPUMIPSState *regs,
     }
 }
 
-static inline int
+static inline void
 restore_sigcontext(CPUMIPSState *regs, struct target_sigcontext *sc)
 {
-    int err = 0;
     int i;
 
     __get_user(regs->CP0_EPC, &sc->sc_pc);
@@ -2919,8 +2904,6 @@  restore_sigcontext(CPUMIPSState *regs, struct target_sigcontext *sc)
     for (i = 0; i < 32; ++i) {
         __get_user(regs->active_fpu.fpr[i].d, &sc->sc_fpregs[i]);
     }
-
-    return err;
 }
 
 /*
@@ -3031,8 +3014,7 @@  long do_sigreturn(CPUMIPSState *regs)
     target_to_host_sigset_internal(&blocked, &target_set);
     do_sigprocmask(SIG_SETMASK, &blocked, NULL);
 
-    if (restore_sigcontext(regs, &frame->sf_sc))
-   	goto badframe;
+    restore_sigcontext(regs, &frame->sf_sc);
 
 #if 0
     /*
@@ -3135,8 +3117,7 @@  long do_rt_sigreturn(CPUMIPSState *env)
     target_to_host_sigset(&blocked, &frame->rs_uc.tuc_sigmask);
     do_sigprocmask(SIG_SETMASK, &blocked, NULL);
 
-    if (restore_sigcontext(env, &frame->rs_uc.tuc_mcontext))
-        goto badframe;
+    restore_sigcontext(env, &frame->rs_uc.tuc_mcontext);
 
     if (do_sigaltstack(frame_addr +
 		       offsetof(struct target_rt_sigframe, rs_uc.tuc_stack),
@@ -3249,10 +3230,9 @@  static void setup_sigcontext(struct target_sigcontext *sc,
     __put_user(mask, &sc->oldmask);
 }
 
-static int restore_sigcontext(CPUSH4State *regs, struct target_sigcontext *sc,
+static void restore_sigcontext(CPUSH4State *regs, struct target_sigcontext *sc,
                               target_ulong *r0_p)
 {
-    unsigned int err = 0;
     int i;
 
 #define COPY(x)         __get_user(regs->x, &sc->sc_##x)
@@ -3277,7 +3257,6 @@  static int restore_sigcontext(CPUSH4State *regs, struct target_sigcontext *sc,
 
     regs->tra = -1;         /* disable syscall checks */
     __get_user(*r0_p, &sc->sc_gregs[0]);
-    return err;
 }
 
 static void setup_frame(int sig, struct target_sigaction *ka,
@@ -3422,8 +3401,7 @@  long do_sigreturn(CPUSH4State *regs)
     target_to_host_sigset_internal(&blocked, &target_set);
     do_sigprocmask(SIG_SETMASK, &blocked, NULL);
 
-    if (restore_sigcontext(regs, &frame->sc, &r0))
-        goto badframe;
+    restore_sigcontext(regs, &frame->sc, &r0);
 
     unlock_user_struct(frame, frame_addr, 0);
     return r0;
@@ -3451,8 +3429,7 @@  long do_rt_sigreturn(CPUSH4State *regs)
     target_to_host_sigset(&blocked, &frame->uc.tuc_sigmask);
     do_sigprocmask(SIG_SETMASK, &blocked, NULL);
 
-    if (restore_sigcontext(regs, &frame->uc.tuc_mcontext, &r0))
-        goto badframe;
+    restore_sigcontext(regs, &frame->uc.tuc_mcontext, &r0);
 
     if (do_sigaltstack(frame_addr +
 		       offsetof(struct target_rt_sigframe, uc.tuc_stack),
@@ -5086,10 +5063,9 @@  static void setup_sigcontext(struct target_sigcontext *sc, CPUM68KState *env,
     __put_user(env->pc, &sc->sc_pc);
 }
 
-static int
+static void
 restore_sigcontext(CPUM68KState *env, struct target_sigcontext *sc, int *pd0)
 {
-    int err = 0;
     int temp;
 
     __get_user(env->aregs[7], &sc->sc_usp);
@@ -5101,8 +5077,6 @@  restore_sigcontext(CPUM68KState *env, struct target_sigcontext *sc, int *pd0)
     env->sr = (env->sr & 0xff00) | (temp & 0xff);
 
     *pd0 = tswapl(sc->sc_d0);
-
-    return err;
 }
 
 /*
@@ -5343,8 +5317,7 @@  long do_sigreturn(CPUM68KState *env)
 
     /* restore registers */
 
-    if (restore_sigcontext(env, &frame->sc, &d0))
-        goto badframe;
+    restore_sigcontext(env, &frame->sc, &d0);
 
     unlock_user_struct(frame, frame_addr, 0);
     return d0;
@@ -5462,11 +5435,11 @@  static void setup_sigcontext(struct target_sigcontext *sc, CPUAlphaState *env,
     __put_user(0, &sc->sc_traparg_a2); /* FIXME */
 }
 
-static int restore_sigcontext(CPUAlphaState *env,
+static void restore_sigcontext(CPUAlphaState *env,
                               struct target_sigcontext *sc)
 {
     uint64_t fpcr;
-    int i, err = 0;
+    int i;
 
     __get_user(env->pc, &sc->sc_pc);
 
@@ -5479,8 +5452,6 @@  static int restore_sigcontext(CPUAlphaState *env,
 
     __get_user(fpcr, &sc->sc_fpcr);
     cpu_alpha_store_fpcr(env, fpcr);
-
-    return err;
 }
 
 static inline abi_ulong get_sigframe(struct target_sigaction *sa,
@@ -5614,9 +5585,7 @@  long do_sigreturn(CPUAlphaState *env)
     target_to_host_sigset_internal(&set, &target_set);
     do_sigprocmask(SIG_SETMASK, &set, NULL);
 
-    if (restore_sigcontext(env, sc)) {
-        goto badframe;
-    }
+    restore_sigcontext(env, sc);
     unlock_user_struct(sc, sc_addr, 0);
     return env->ir[IR_V0];
 
@@ -5637,9 +5606,7 @@  long do_rt_sigreturn(CPUAlphaState *env)
     target_to_host_sigset(&set, &frame->uc.tuc_sigmask);
     do_sigprocmask(SIG_SETMASK, &set, NULL);
 
-    if (restore_sigcontext(env, &frame->uc.tuc_mcontext)) {
-        goto badframe;
-    }
+    restore_sigcontext(env, &frame->uc.tuc_mcontext);
     if (do_sigaltstack(frame_addr + offsetof(struct target_rt_sigframe,
                                              uc.tuc_stack),
                        0, env->ir[IR_SP]) == -EFAULT) {