| Message ID | ZMLfYE3fYCUhnaEE@p100 |
|---|---|
| State | New |
| Headers | show |
| Series | linux-user/armeb: Fix __kernel_cmpxchg() for armeb | expand |
On 7/27/23 14:19, Helge Deller wrote: > Words are stored in big endian in the guest memory for armeb. > > Commit 7f4f0d9ea870 ("linux-user/arm: Implement __kernel_cmpxchg with > host atomics") switched to use qatomic_cmpxchg() to swap a word with the > memory content, but missed to endianess-swap the oldval and newval > values when emulating an armeb CPU. > > The bug can be verified with qemu >= v7.2 on any little-endian host, > when starting the armeb binary of the upx program, which just hangs > without this patch. > > Signed-off-by: Helge Deller <deller@gmx.de> > Cc: Richard Henderson <richard.henderson@linaro.org> > Cc: Peter Maydell <peter.maydell@linaro.org> > Cc: qemu-stable@nongnu.org > Reported-by: "Markus F.X.J. Oberhumer" <markus@oberhumer.com> > Reported-by: John Reiser <jreiser@BitWagon.com> > Closes: https://github.com/upx/upx/issues/687 > > diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c > index a992423257..ff0bff7c63 100644 > --- a/linux-user/arm/cpu_loop.c > +++ b/linux-user/arm/cpu_loop.c > @@ -117,8 +117,8 @@ static void arm_kernel_cmpxchg32_helper(CPUARMState *env) > { > uint32_t oldval, newval, val, addr, cpsr, *host_addr; > > - oldval = env->regs[0]; > - newval = env->regs[1]; > + oldval = tswap32(env->regs[0]); > + newval = tswap32(env->regs[1]); > addr = env->regs[2]; There's a similar bug with arm_kernel_cmpxchg64_helper just below, but Reviewed-by: Richard Henderson <richard.henderson@linaro.org> as far as this goes. r~
On 27/7/23 23:19, Helge Deller wrote: > Words are stored in big endian in the guest memory for armeb. > > Commit 7f4f0d9ea870 ("linux-user/arm: Implement __kernel_cmpxchg with > host atomics") switched to use qatomic_cmpxchg() to swap a word with the > memory content, but missed to endianess-swap the oldval and newval > values when emulating an armeb CPU. > > The bug can be verified with qemu >= v7.2 on any little-endian host, > when starting the armeb binary of the upx program, which just hangs > without this patch. > > Signed-off-by: Helge Deller <deller@gmx.de> > Cc: Richard Henderson <richard.henderson@linaro.org> > Cc: Peter Maydell <peter.maydell@linaro.org> > Cc: qemu-stable@nongnu.org > Reported-by: "Markus F.X.J. Oberhumer" <markus@oberhumer.com> > Reported-by: John Reiser <jreiser@BitWagon.com> > Closes: https://github.com/upx/upx/issues/687 Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
31.07.2023 12:26, Philippe Mathieu-Daudé wrote: > On 27/7/23 23:19, Helge Deller wrote: >> Words are stored in big endian in the guest memory for armeb. .. > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> There was a v3 of this patch already, fwiw. /mjt
diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c index a992423257..ff0bff7c63 100644 --- a/linux-user/arm/cpu_loop.c +++ b/linux-user/arm/cpu_loop.c @@ -117,8 +117,8 @@ static void arm_kernel_cmpxchg32_helper(CPUARMState *env) { uint32_t oldval, newval, val, addr, cpsr, *host_addr; - oldval = env->regs[0]; - newval = env->regs[1]; + oldval = tswap32(env->regs[0]); + newval = tswap32(env->regs[1]); addr = env->regs[2]; mmap_lock();
Words are stored in big endian in the guest memory for armeb. Commit 7f4f0d9ea870 ("linux-user/arm: Implement __kernel_cmpxchg with host atomics") switched to use qatomic_cmpxchg() to swap a word with the memory content, but missed to endianess-swap the oldval and newval values when emulating an armeb CPU. The bug can be verified with qemu >= v7.2 on any little-endian host, when starting the armeb binary of the upx program, which just hangs without this patch. Signed-off-by: Helge Deller <deller@gmx.de> Cc: Richard Henderson <richard.henderson@linaro.org> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: qemu-stable@nongnu.org Reported-by: "Markus F.X.J. Oberhumer" <markus@oberhumer.com> Reported-by: John Reiser <jreiser@BitWagon.com> Closes: https://github.com/upx/upx/issues/687