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));
     }
-- 
2.30.2

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

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
>