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

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

Posted by Andreas Krebbel 4 years, 10 months ago

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

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

Posted by Laurent Vivier 4 years, 10 months ago

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

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

Posted by Andreas Krebbel 4 years, 10 months ago

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
>