All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] linux-user,sh4: fix signal retcode address
@ 2015-11-23 10:38 Laurent Vivier
  2015-11-23 10:45 ` [Qemu-devel] [PATCH] linux-user, sh4: " John Paul Adrian Glaubitz
  2015-12-18 15:08 ` Laurent Vivier
  0 siblings, 2 replies; 4+ messages in thread
From: Laurent Vivier @ 2015-11-23 10:38 UTC (permalink / raw)
  To: qemu-devel; +Cc: riku.voipio, Laurent Vivier, glaubitz

To return from a signal, setup_frame() puts an instruction to
be executed in the stack. This sequence calls the syscall sigreturn().

The address of the instruction must be set in the PR register
to be executed.

This patch fixes this: the current code sets the register to the address
of the instruction in the host address space (which can be 64bit whereas
PR is only 32bit), but the virtual CPU can't access this address space,
so we put in PR the address of the instruction in the guest address space.

This patch also removes an useless variable (ret) in the modified functions.

Signed-off-by: Laurent Vivier <laurent@vivier.eu>
---
 linux-user/signal.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 55e5405..5e8f6d8 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -3215,7 +3215,6 @@ static void setup_frame(int sig, struct target_sigaction *ka,
     struct target_sigframe *frame;
     abi_ulong frame_addr;
     int i;
-    int err = 0;
 
     frame_addr = get_sigframe(ka, regs->gregs[15], sizeof(*frame));
     if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0))
@@ -3233,15 +3232,14 @@ static void setup_frame(int sig, struct target_sigaction *ka,
         regs->pr = (unsigned long) ka->sa_restorer;
     } else {
         /* Generate return code (system call to sigreturn) */
+        abi_ulong retcode_addr = frame_addr +
+                                 offsetof(struct target_sigframe, retcode);
         __put_user(MOVW(2), &frame->retcode[0]);
         __put_user(TRAP_NOARG, &frame->retcode[1]);
         __put_user((TARGET_NR_sigreturn), &frame->retcode[2]);
-        regs->pr = (unsigned long) frame->retcode;
+        regs->pr = (unsigned long) retcode_addr;
     }
 
-    if (err)
-        goto give_sigsegv;
-
     /* Set up registers for signal handler */
     regs->gregs[15] = frame_addr;
     regs->gregs[4] = sig; /* Arg for signal handler */
@@ -3264,7 +3262,6 @@ static void setup_rt_frame(int sig, struct target_sigaction *ka,
     struct target_rt_sigframe *frame;
     abi_ulong frame_addr;
     int i;
-    int err = 0;
 
     frame_addr = get_sigframe(ka, regs->gregs[15], sizeof(*frame));
     if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0))
@@ -3293,15 +3290,14 @@ static void setup_rt_frame(int sig, struct target_sigaction *ka,
         regs->pr = (unsigned long) ka->sa_restorer;
     } else {
         /* Generate return code (system call to sigreturn) */
+        abi_ulong retcode_addr = frame_addr +
+                                 offsetof(struct target_rt_sigframe, retcode);
         __put_user(MOVW(2), &frame->retcode[0]);
         __put_user(TRAP_NOARG, &frame->retcode[1]);
         __put_user((TARGET_NR_rt_sigreturn), &frame->retcode[2]);
-        regs->pr = (unsigned long) frame->retcode;
+        regs->pr = (unsigned long) retcode_addr;
     }
 
-    if (err)
-        goto give_sigsegv;
-
     /* Set up registers for signal handler */
     regs->gregs[15] = frame_addr;
     regs->gregs[4] = sig; /* Arg for signal handler */
-- 
2.4.3

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user, sh4: fix signal retcode address
  2015-11-23 10:38 [Qemu-devel] [PATCH] linux-user,sh4: fix signal retcode address Laurent Vivier
@ 2015-11-23 10:45 ` John Paul Adrian Glaubitz
  2015-12-18 15:08 ` Laurent Vivier
  1 sibling, 0 replies; 4+ messages in thread
From: John Paul Adrian Glaubitz @ 2015-11-23 10:45 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel; +Cc: riku.voipio

On 11/23/2015 11:38 AM, Laurent Vivier wrote:
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>

Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

This patch fixes crashes of qemu-sh4* on amd64 for me [1].
I also haven't seen any regressions ever since.

Cheers,
Adrian

> [1] https://bugs.launchpad.net/ubuntu/+source/qemu-linaro/+bug/1254824

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user, sh4: fix signal retcode address
  2015-11-23 10:38 [Qemu-devel] [PATCH] linux-user,sh4: fix signal retcode address Laurent Vivier
  2015-11-23 10:45 ` [Qemu-devel] [PATCH] linux-user, sh4: " John Paul Adrian Glaubitz
@ 2015-12-18 15:08 ` Laurent Vivier
  2015-12-19 10:23   ` John Paul Adrian Glaubitz
  1 sibling, 1 reply; 4+ messages in thread
From: Laurent Vivier @ 2015-12-18 15:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: riku.voipio, glaubitz

Ping ?

Le 23/11/2015 11:38, Laurent Vivier a écrit :
> To return from a signal, setup_frame() puts an instruction to
> be executed in the stack. This sequence calls the syscall sigreturn().
> 
> The address of the instruction must be set in the PR register
> to be executed.
> 
> This patch fixes this: the current code sets the register to the address
> of the instruction in the host address space (which can be 64bit whereas
> PR is only 32bit), but the virtual CPU can't access this address space,
> so we put in PR the address of the instruction in the guest address space.
> 
> This patch also removes an useless variable (ret) in the modified functions.
> 
> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  linux-user/signal.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/linux-user/signal.c b/linux-user/signal.c
> index 55e5405..5e8f6d8 100644
> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -3215,7 +3215,6 @@ static void setup_frame(int sig, struct target_sigaction *ka,
>      struct target_sigframe *frame;
>      abi_ulong frame_addr;
>      int i;
> -    int err = 0;
>  
>      frame_addr = get_sigframe(ka, regs->gregs[15], sizeof(*frame));
>      if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0))
> @@ -3233,15 +3232,14 @@ static void setup_frame(int sig, struct target_sigaction *ka,
>          regs->pr = (unsigned long) ka->sa_restorer;
>      } else {
>          /* Generate return code (system call to sigreturn) */
> +        abi_ulong retcode_addr = frame_addr +
> +                                 offsetof(struct target_sigframe, retcode);
>          __put_user(MOVW(2), &frame->retcode[0]);
>          __put_user(TRAP_NOARG, &frame->retcode[1]);
>          __put_user((TARGET_NR_sigreturn), &frame->retcode[2]);
> -        regs->pr = (unsigned long) frame->retcode;
> +        regs->pr = (unsigned long) retcode_addr;
>      }
>  
> -    if (err)
> -        goto give_sigsegv;
> -
>      /* Set up registers for signal handler */
>      regs->gregs[15] = frame_addr;
>      regs->gregs[4] = sig; /* Arg for signal handler */
> @@ -3264,7 +3262,6 @@ static void setup_rt_frame(int sig, struct target_sigaction *ka,
>      struct target_rt_sigframe *frame;
>      abi_ulong frame_addr;
>      int i;
> -    int err = 0;
>  
>      frame_addr = get_sigframe(ka, regs->gregs[15], sizeof(*frame));
>      if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0))
> @@ -3293,15 +3290,14 @@ static void setup_rt_frame(int sig, struct target_sigaction *ka,
>          regs->pr = (unsigned long) ka->sa_restorer;
>      } else {
>          /* Generate return code (system call to sigreturn) */
> +        abi_ulong retcode_addr = frame_addr +
> +                                 offsetof(struct target_rt_sigframe, retcode);
>          __put_user(MOVW(2), &frame->retcode[0]);
>          __put_user(TRAP_NOARG, &frame->retcode[1]);
>          __put_user((TARGET_NR_rt_sigreturn), &frame->retcode[2]);
> -        regs->pr = (unsigned long) frame->retcode;
> +        regs->pr = (unsigned long) retcode_addr;
>      }
>  
> -    if (err)
> -        goto give_sigsegv;
> -
>      /* Set up registers for signal handler */
>      regs->gregs[15] = frame_addr;
>      regs->gregs[4] = sig; /* Arg for signal handler */
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH] linux-user, sh4: fix signal retcode address
  2015-12-18 15:08 ` Laurent Vivier
@ 2015-12-19 10:23   ` John Paul Adrian Glaubitz
  0 siblings, 0 replies; 4+ messages in thread
From: John Paul Adrian Glaubitz @ 2015-12-19 10:23 UTC (permalink / raw)
  To: Laurent Vivier, qemu-devel; +Cc: riku.voipio

Ping here as well: https://bugs.launchpad.net/qemu/+bug/1516408

Adrian

On 12/18/2015 04:08 PM, Laurent Vivier wrote:
> Ping ?
> 
> Le 23/11/2015 11:38, Laurent Vivier a écrit :
>> To return from a signal, setup_frame() puts an instruction to
>> be executed in the stack. This sequence calls the syscall sigreturn().
>>
>> The address of the instruction must be set in the PR register
>> to be executed.
>>
>> This patch fixes this: the current code sets the register to the address
>> of the instruction in the host address space (which can be 64bit whereas
>> PR is only 32bit), but the virtual CPU can't access this address space,
>> so we put in PR the address of the instruction in the guest address space.
>>
>> This patch also removes an useless variable (ret) in the modified functions.
>>
>> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
>> ---
>>  linux-user/signal.c | 16 ++++++----------
>>  1 file changed, 6 insertions(+), 10 deletions(-)
>>
>> diff --git a/linux-user/signal.c b/linux-user/signal.c
>> index 55e5405..5e8f6d8 100644
>> --- a/linux-user/signal.c
>> +++ b/linux-user/signal.c
>> @@ -3215,7 +3215,6 @@ static void setup_frame(int sig, struct target_sigaction *ka,
>>      struct target_sigframe *frame;
>>      abi_ulong frame_addr;
>>      int i;
>> -    int err = 0;
>>  
>>      frame_addr = get_sigframe(ka, regs->gregs[15], sizeof(*frame));
>>      if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0))
>> @@ -3233,15 +3232,14 @@ static void setup_frame(int sig, struct target_sigaction *ka,
>>          regs->pr = (unsigned long) ka->sa_restorer;
>>      } else {
>>          /* Generate return code (system call to sigreturn) */
>> +        abi_ulong retcode_addr = frame_addr +
>> +                                 offsetof(struct target_sigframe, retcode);
>>          __put_user(MOVW(2), &frame->retcode[0]);
>>          __put_user(TRAP_NOARG, &frame->retcode[1]);
>>          __put_user((TARGET_NR_sigreturn), &frame->retcode[2]);
>> -        regs->pr = (unsigned long) frame->retcode;
>> +        regs->pr = (unsigned long) retcode_addr;
>>      }
>>  
>> -    if (err)
>> -        goto give_sigsegv;
>> -
>>      /* Set up registers for signal handler */
>>      regs->gregs[15] = frame_addr;
>>      regs->gregs[4] = sig; /* Arg for signal handler */
>> @@ -3264,7 +3262,6 @@ static void setup_rt_frame(int sig, struct target_sigaction *ka,
>>      struct target_rt_sigframe *frame;
>>      abi_ulong frame_addr;
>>      int i;
>> -    int err = 0;
>>  
>>      frame_addr = get_sigframe(ka, regs->gregs[15], sizeof(*frame));
>>      if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0))
>> @@ -3293,15 +3290,14 @@ static void setup_rt_frame(int sig, struct target_sigaction *ka,
>>          regs->pr = (unsigned long) ka->sa_restorer;
>>      } else {
>>          /* Generate return code (system call to sigreturn) */
>> +        abi_ulong retcode_addr = frame_addr +
>> +                                 offsetof(struct target_rt_sigframe, retcode);
>>          __put_user(MOVW(2), &frame->retcode[0]);
>>          __put_user(TRAP_NOARG, &frame->retcode[1]);
>>          __put_user((TARGET_NR_rt_sigreturn), &frame->retcode[2]);
>> -        regs->pr = (unsigned long) frame->retcode;
>> +        regs->pr = (unsigned long) retcode_addr;
>>      }
>>  
>> -    if (err)
>> -        goto give_sigsegv;
>> -
>>      /* Set up registers for signal handler */
>>      regs->gregs[15] = frame_addr;
>>      regs->gregs[4] = sig; /* Arg for signal handler */
>>


-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer - glaubitz@debian.org
`. `'   Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-12-19 10:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-23 10:38 [Qemu-devel] [PATCH] linux-user,sh4: fix signal retcode address Laurent Vivier
2015-11-23 10:45 ` [Qemu-devel] [PATCH] linux-user, sh4: " John Paul Adrian Glaubitz
2015-12-18 15:08 ` Laurent Vivier
2015-12-19 10:23   ` John Paul Adrian Glaubitz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.