* [PATCH v2 1/1] target/ppc: don't byte swap ELFv2 signal handler
@ 2020-03-18 17:01 Vincent Fazio
2020-03-18 19:57 ` Richard Henderson
2020-03-19 5:35 ` David Gibson
0 siblings, 2 replies; 3+ messages in thread
From: Vincent Fazio @ 2020-03-18 17:01 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-trivial, Riku Voipio, Laurent Vivier, qemu-ppc,
Vincent Fazio, David Gibson
From: Vincent Fazio <vfazio@gmail.com>
Previously, the signal handler would be byte swapped if the target and
host CPU used different endianness. This would cause a SIGSEGV when
attempting to translate the opcode pointed to by the swapped address.
Thread 1 "qemu-ppc64" received signal SIGSEGV, Segmentation fault.
0x00000000600a9257 in ldl_he_p (ptr=0x4c2c061000000000) at qemu/include/qemu/bswap.h:351
351 __builtin_memcpy(&r, ptr, sizeof(r));
#0 0x00000000600a9257 in ldl_he_p (ptr=0x4c2c061000000000) at qemu/include/qemu/bswap.h:351
#1 0x00000000600a92fe in ldl_be_p (ptr=0x4c2c061000000000) at qemu/include/qemu/bswap.h:449
#2 0x00000000600c0790 in translator_ldl_swap at qemu/include/exec/translator.h:201
#3 0x000000006011c1ab in ppc_tr_translate_insn at qemu/target/ppc/translate.c:7856
#4 0x000000006005ae70 in translator_loop at qemu/accel/tcg/translator.c:102
Now, no swap is performed and execution continues properly.
Signed-off-by: Vincent Fazio <vfazio@gmail.com>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
Changes since v1:
- Drop host/target endianness callouts
- Drop unnecessary pointer cast
- Clarify commit message
linux-user/ppc/signal.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
index 5b82af6cb6..b8613c5e1b 100644
--- a/linux-user/ppc/signal.c
+++ b/linux-user/ppc/signal.c
@@ -567,10 +567,8 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
env->nip = tswapl(handler->entry);
env->gpr[2] = tswapl(handler->toc);
} else {
- /* ELFv2 PPC64 function pointers are entry points, but R12
- * must also be set */
- env->nip = tswapl((target_ulong) ka->_sa_handler);
- env->gpr[12] = env->nip;
+ /* ELFv2 PPC64 function pointers are entry points. R12 must also be set. */
+ env->gpr[12] = env->nip = ka->_sa_handler;
}
#else
env->nip = (target_ulong) ka->_sa_handler;
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2 1/1] target/ppc: don't byte swap ELFv2 signal handler
2020-03-18 17:01 [PATCH v2 1/1] target/ppc: don't byte swap ELFv2 signal handler Vincent Fazio
@ 2020-03-18 19:57 ` Richard Henderson
2020-03-19 5:35 ` David Gibson
1 sibling, 0 replies; 3+ messages in thread
From: Richard Henderson @ 2020-03-18 19:57 UTC (permalink / raw)
To: Vincent Fazio, qemu-devel
Cc: qemu-trivial, Riku Voipio, Laurent Vivier, qemu-ppc,
Vincent Fazio, David Gibson
On 3/18/20 10:01 AM, Vincent Fazio wrote:
> From: Vincent Fazio <vfazio@gmail.com>
>
> Previously, the signal handler would be byte swapped if the target and
> host CPU used different endianness. This would cause a SIGSEGV when
> attempting to translate the opcode pointed to by the swapped address.
>
> Thread 1 "qemu-ppc64" received signal SIGSEGV, Segmentation fault.
> 0x00000000600a9257 in ldl_he_p (ptr=0x4c2c061000000000) at qemu/include/qemu/bswap.h:351
> 351 __builtin_memcpy(&r, ptr, sizeof(r));
>
> #0 0x00000000600a9257 in ldl_he_p (ptr=0x4c2c061000000000) at qemu/include/qemu/bswap.h:351
> #1 0x00000000600a92fe in ldl_be_p (ptr=0x4c2c061000000000) at qemu/include/qemu/bswap.h:449
> #2 0x00000000600c0790 in translator_ldl_swap at qemu/include/exec/translator.h:201
> #3 0x000000006011c1ab in ppc_tr_translate_insn at qemu/target/ppc/translate.c:7856
> #4 0x000000006005ae70 in translator_loop at qemu/accel/tcg/translator.c:102
>
> Now, no swap is performed and execution continues properly.
>
> Signed-off-by: Vincent Fazio <vfazio@gmail.com>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
> Changes since v1:
> - Drop host/target endianness callouts
> - Drop unnecessary pointer cast
> - Clarify commit message
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2 1/1] target/ppc: don't byte swap ELFv2 signal handler
2020-03-18 17:01 [PATCH v2 1/1] target/ppc: don't byte swap ELFv2 signal handler Vincent Fazio
2020-03-18 19:57 ` Richard Henderson
@ 2020-03-19 5:35 ` David Gibson
1 sibling, 0 replies; 3+ messages in thread
From: David Gibson @ 2020-03-19 5:35 UTC (permalink / raw)
To: Vincent Fazio
Cc: qemu-trivial, Riku Voipio, qemu-devel, Laurent Vivier, qemu-ppc,
Vincent Fazio
[-- Attachment #1: Type: text/plain, Size: 2553 bytes --]
On Wed, Mar 18, 2020 at 12:01:16PM -0500, Vincent Fazio wrote:
> From: Vincent Fazio <vfazio@gmail.com>
>
> Previously, the signal handler would be byte swapped if the target and
> host CPU used different endianness. This would cause a SIGSEGV when
> attempting to translate the opcode pointed to by the swapped address.
>
> Thread 1 "qemu-ppc64" received signal SIGSEGV, Segmentation fault.
> 0x00000000600a9257 in ldl_he_p (ptr=0x4c2c061000000000) at qemu/include/qemu/bswap.h:351
> 351 __builtin_memcpy(&r, ptr, sizeof(r));
>
> #0 0x00000000600a9257 in ldl_he_p (ptr=0x4c2c061000000000) at qemu/include/qemu/bswap.h:351
> #1 0x00000000600a92fe in ldl_be_p (ptr=0x4c2c061000000000) at qemu/include/qemu/bswap.h:449
> #2 0x00000000600c0790 in translator_ldl_swap at qemu/include/exec/translator.h:201
> #3 0x000000006011c1ab in ppc_tr_translate_insn at qemu/target/ppc/translate.c:7856
> #4 0x000000006005ae70 in translator_loop at qemu/accel/tcg/translator.c:102
>
> Now, no swap is performed and execution continues properly.
Patch is good, but the message still needs some work. It should point
out that the necessary swap is already done at sigaction() time by the
__get_user().
>
> Signed-off-by: Vincent Fazio <vfazio@gmail.com>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
> Changes since v1:
> - Drop host/target endianness callouts
> - Drop unnecessary pointer cast
> - Clarify commit message
>
> linux-user/ppc/signal.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
> index 5b82af6cb6..b8613c5e1b 100644
> --- a/linux-user/ppc/signal.c
> +++ b/linux-user/ppc/signal.c
> @@ -567,10 +567,8 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
> env->nip = tswapl(handler->entry);
> env->gpr[2] = tswapl(handler->toc);
> } else {
> - /* ELFv2 PPC64 function pointers are entry points, but R12
> - * must also be set */
> - env->nip = tswapl((target_ulong) ka->_sa_handler);
> - env->gpr[12] = env->nip;
> + /* ELFv2 PPC64 function pointers are entry points. R12 must also be set. */
> + env->gpr[12] = env->nip = ka->_sa_handler;
> }
> #else
> env->nip = (target_ulong) ka->_sa_handler;
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-03-19 6:16 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-18 17:01 [PATCH v2 1/1] target/ppc: don't byte swap ELFv2 signal handler Vincent Fazio
2020-03-18 19:57 ` Richard Henderson
2020-03-19 5:35 ` David Gibson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).