Re: [PATCH] linux-user/arm: Reset CPSR_E when entering a signal handler

[Amanieu d'Antras]

On Fri, May 15, 2020 at 7:34 PM Peter Maydell <address@hidden> wrote:
>
> On Thu, 7 May 2020 at 21:25, Amanieu d'Antras <address@hidden> wrote:
> >
> > This fixes signal handlers running with the wrong endianness if the
> > interrupted code used SETEND to dynamically switch endianness.
> >
> > Signed-off-by: Amanieu d'Antras <address@hidden>
> > ---
> >  linux-user/arm/signal.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/linux-user/arm/signal.c b/linux-user/arm/signal.c
> > index d96fc27ce1..8aca5f61b7 100644
> > --- a/linux-user/arm/signal.c
> > +++ b/linux-user/arm/signal.c
> > @@ -244,6 +244,12 @@ setup_return(CPUARMState *env, struct target_sigaction 
> > *ka,
> >      } else {
> >          cpsr &= ~CPSR_T;
> >      }
> > +    cpsr &= ~CPSR_E;
> > +#ifdef TARGET_WORDS_BIGENDIAN
> > +    if (env->cp15.sctlr_el[1] & SCTLR_E0E) {
> > +        cpsr |= CPSR_E;
> > +    }
> > +#endif
> >
> >      if (ka->sa_flags & TARGET_SA_RESTORER) {
> >          if (is_fdpic) {
> > @@ -287,7 +293,8 @@ setup_return(CPUARMState *env, struct target_sigaction 
> > *ka,
> >      env->regs[13] = frame_addr;
> >      env->regs[14] = retcode;
> >      env->regs[15] = handler & (thumb ? ~1 : ~3);
> > -    cpsr_write(env, cpsr, CPSR_IT | CPSR_T, CPSRWriteByInstr);
> > +    cpsr_write(env, cpsr, CPSR_IT | CPSR_T | CPSR_E, CPSRWriteByInstr);
> > +    arm_rebuild_hflags(env);
>
> I was just looking at the signal code's handling of CPSR for a different
> reason, and I noticed that at the moment we don't allow CPSR.E to be
> updated from the signal frame when the signal handler returns
> (because CPSR_USER doesn't contain CPSR_E and that's what we
> use in restore_sigcontext() to define what bits from the frame we
> allow updating). Don't you find that when the interrupted code
> returns from the signal handler that it ends up running with the
> wrong endianness (ie the endianness the handler used) ?

I actually found this while trying to test the SETEND instruction
under risu. The signal handler was crashing because it loaded a
pointer with the wrong endianness, which was pretty obvious. However I
missed the fact that code was now running with the wrong endianness
after returning from the signal handler since I had both the master
and the apprentice running under qemu-arm.

> I'm going to fix this by putting CPSR_E in CPSR_USER, anyway.

You also need to call arm_rebuild_hflags() after modifying CPSR_E
otherwise the change doesn't take effect.

-- Amanieu