diff mbox series

[v2] linux-user/s390x: Use the guest pointer for the sigreturn stub

Message ID 20210324155530.52239-1-krebbel@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series [v2] linux-user/s390x: Use the guest pointer for the sigreturn stub | expand

Commit Message

Andreas Krebbel March 24, 2021, 3:55 p.m. UTC
When setting up the pointer for the sigreturn stub in the return
address register (r14) we have to use the guest frame pointer instead
of the host frame pointer.

Note: This only caused problems if Qemu has been built with
--disable-pie (as it is in distros nowadays). Otherwise guest_base
defaults to 0 hiding the actual problem.

Signed-off-by: Andreas Krebbel <krebbel@linux.ibm.com>
---
 linux-user/s390x/signal.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Laurent Vivier March 24, 2021, 5:53 p.m. UTC | #1
Le 24/03/2021 à 16:55, Andreas Krebbel a écrit :
> When setting up the pointer for the sigreturn stub in the return
> address register (r14) we have to use the guest frame pointer instead
> of the host frame pointer.
> 
> Note: This only caused problems if Qemu has been built with
> --disable-pie (as it is in distros nowadays). Otherwise guest_base
> defaults to 0 hiding the actual problem.
> 
> Signed-off-by: Andreas Krebbel <krebbel@linux.ibm.com>
> ---
>  linux-user/s390x/signal.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
> index ecfa2a14a9..e9bf865074 100644
> --- a/linux-user/s390x/signal.c
> +++ b/linux-user/s390x/signal.c
> @@ -213,7 +213,8 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
>      if (ka->sa_flags & TARGET_SA_RESTORER) {
>          env->regs[14] = (unsigned long) ka->sa_restorer | PSW_ADDR_AMODE;
>      } else {
> -        env->regs[14] = (unsigned long) frame->retcode | PSW_ADDR_AMODE;
> +        env->regs[14] = (target_ulong) (frame_addr + offsetof(rt_sigframe, retcode))
> +                        | PSW_ADDR_AMODE;
>          __put_user(S390_SYSCALL_OPCODE | TARGET_NR_rt_sigreturn,
>                     (uint16_t *)(frame->retcode));
>      }
> 

Reviewed-by: Laurent Vivier <laurent@vivier.eu>

but if you want to send a v3:
- to be consistent with lines below, use "offsetof(typeof(*frame), ..."
- in the line above, you can remove the (unsigned long) of the sa_restorer as it is an abi_ulong,
- don't send the "v2" as a reply to the v1 as it can be hidden in the mail thread and missed by the
maintainer :)

Thanks,
Laurent
Andreas Krebbel March 24, 2021, 6:48 p.m. UTC | #2
On 3/24/21 6:53 PM, Laurent Vivier wrote:
> Le 24/03/2021 à 16:55, Andreas Krebbel a écrit :
>> When setting up the pointer for the sigreturn stub in the return
>> address register (r14) we have to use the guest frame pointer instead
>> of the host frame pointer.
>>
>> Note: This only caused problems if Qemu has been built with
>> --disable-pie (as it is in distros nowadays). Otherwise guest_base
>> defaults to 0 hiding the actual problem.
>>
>> Signed-off-by: Andreas Krebbel <krebbel@linux.ibm.com>
>> ---
>>  linux-user/s390x/signal.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
>> index ecfa2a14a9..e9bf865074 100644
>> --- a/linux-user/s390x/signal.c
>> +++ b/linux-user/s390x/signal.c
>> @@ -213,7 +213,8 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
>>      if (ka->sa_flags & TARGET_SA_RESTORER) {
>>          env->regs[14] = (unsigned long) ka->sa_restorer | PSW_ADDR_AMODE;
>>      } else {
>> -        env->regs[14] = (unsigned long) frame->retcode | PSW_ADDR_AMODE;
>> +        env->regs[14] = (target_ulong) (frame_addr + offsetof(rt_sigframe, retcode))
>> +                        | PSW_ADDR_AMODE;
>>          __put_user(S390_SYSCALL_OPCODE | TARGET_NR_rt_sigreturn,
>>                     (uint16_t *)(frame->retcode));
>>      }
>>
> 
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> 
> but if you want to send a v3:

Sure, will do.

> - to be consistent with lines below, use "offsetof(typeof(*frame), ..."
> - in the line above, you can remove the (unsigned long) of the sa_restorer as it is an abi_ulong,

The (target_ulong) cast could probably go away as well since frame_addr is also abi_ulong.

> - don't send the "v2" as a reply to the v1 as it can be hidden in the mail thread and missed by the
> maintainer :)

Ok.

Andreas

> 
> Thanks,
> Laurent
>
diff mbox series

Patch

diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
index ecfa2a14a9..e9bf865074 100644
--- a/linux-user/s390x/signal.c
+++ b/linux-user/s390x/signal.c
@@ -213,7 +213,8 @@  void setup_rt_frame(int sig, struct target_sigaction *ka,
     if (ka->sa_flags & TARGET_SA_RESTORER) {
         env->regs[14] = (unsigned long) ka->sa_restorer | PSW_ADDR_AMODE;
     } else {
-        env->regs[14] = (unsigned long) frame->retcode | PSW_ADDR_AMODE;
+        env->regs[14] = (target_ulong) (frame_addr + offsetof(rt_sigframe, retcode))
+                        | PSW_ADDR_AMODE;
         __put_user(S390_SYSCALL_OPCODE | TARGET_NR_rt_sigreturn,
                    (uint16_t *)(frame->retcode));
     }