Re: [PATCH] linux-user/arm: Reset CPSR_E when entering a signal handler
On Fri, May 15, 2020 at 7:34 PM Peter Maydell wrote:
>
> On Thu, 7 May 2020 at 21:25, Amanieu d'Antras 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
> > ---
> > 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
[PATCH v2] linux-user/arm: Reset CPSR_E when entering a signal handler
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
---
linux-user/arm/signal.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/linux-user/arm/signal.c b/linux-user/arm/signal.c
index d96fc27ce1..a475a103e9 100644
--- a/linux-user/arm/signal.c
+++ b/linux-user/arm/signal.c
@@ -244,6 +244,11 @@ setup_return(CPUARMState *env, struct target_sigaction *ka,
} else {
cpsr &= ~CPSR_T;
}
+if (env->cp15.sctlr_el[1] & SCTLR_E0E) {
+cpsr |= CPSR_E;
+} else {
+cpsr &= ~CPSR_E;
+}
if (ka->sa_flags & TARGET_SA_RESTORER) {
if (is_fdpic) {
@@ -287,7 +292,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);
return 0;
}
--
2.26.2
[PATCH] linux-user/arm: Reset CPSR_E when entering a signal handler
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
---
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);
return 0;
}
--
2.26.2
[PATCH] linux-user/riscv: Fix target_ucontext and target_sigcontext
These now match the field layout used by the kernel.
Signed-off-by: Amanieu d'Antras
---
linux-user/riscv/signal.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/linux-user/riscv/signal.c b/linux-user/riscv/signal.c
index 83ecc6f799..2b15e32a7b 100644
--- a/linux-user/riscv/signal.c
+++ b/linux-user/riscv/signal.c
@@ -32,7 +32,7 @@
struct target_sigcontext {
abi_long pc;
abi_long gpr[31]; /* x0 is not present, so all offsets must be -1 */
-uint64_t fpr[32];
+uint64_t fpr[32] __attribute__((aligned(16)));
uint32_t fcsr;
}; /* cf. riscv-linux:arch/riscv/include/uapi/asm/ptrace.h */
@@ -40,8 +40,9 @@ struct target_ucontext {
unsigned long uc_flags;
struct target_ucontext *uc_link;
target_stack_t uc_stack;
-struct target_sigcontext uc_mcontext;
target_sigset_t uc_sigmask;
+char __unused[1024 / 8 - sizeof(target_sigset_t)];
+struct target_sigcontext uc_mcontext;
};
struct target_rt_sigframe {
--
2.26.1
[Qemu-devel] [PATCH] linux-user: Fix fault address truncation AArch64
On AArch64 the si_addr field of siginfo_t is truncated to 32 bits
because the fault address passes through an uint32_t variable. This
is fixed by changing the variable to uint64_t.
Signed-off-by: Amanieu d'Antras aman...@gmail.com
---
linux-user/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/main.c b/linux-user/main.c
index 483eb3f..d63e093 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -1006,7 +1006,7 @@ void cpu_loop(CPUARMState *env)
CPUState *cs = CPU(arm_env_get_cpu(env));
int trapnr, sig;
target_siginfo_t info;
-uint32_t addr;
+uint64_t addr;
for (;;) {
cpu_exec_start(cs);
--
2.1.2
[Qemu-devel] [Bug 1287195] [NEW] validate_guest_space incorrectly enabled on AArch64
Public bug reported: When running linux-user targetting AArch64, validate_guest_space() in elfload.c reserves space in the guest address space for the ARM commpage. Since there is no commpage on AArch64, this function should be disable on that target. ** Affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1287195 Title: validate_guest_space incorrectly enabled on AArch64 Status in QEMU: New Bug description: When running linux-user targetting AArch64, validate_guest_space() in elfload.c reserves space in the guest address space for the ARM commpage. Since there is no commpage on AArch64, this function should be disable on that target. To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1287195/+subscriptions
