Re: [PATCH 1/1] linux-user/s390x: Apply h2g to address of sigreturn stub

[Andreas Krebbel]

On 3/24/21 11:28 AM, Laurent Vivier wrote:
> Le 24/03/2021 à 10:17, David Hildenbrand a écrit :
>> On 24.03.21 09:51, Andreas Krebbel wrote:
>>> The sigreturn SVC is put onto the stack by the emulation code.  Hence
>>> the address of it should not be subject to guest_base transformation
>>> when fetching it.
>>>
>>> The fix applies h2g to the address when writing it into the return
>>> address register to nullify the transformation applied to it later.
>>>
>>> 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 | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/linux-user/s390x/signal.c b/linux-user/s390x/signal.c
>>> index ecfa2a14a9..1412376958 100644
>>> --- a/linux-user/s390x/signal.c
>>> +++ b/linux-user/s390x/signal.c
>>> @@ -152,7 +152,7 @@ void setup_frame(int sig, struct target_sigaction *ka,
>>>           env->regs[14] = (unsigned long)
>>>                   ka->sa_restorer | PSW_ADDR_AMODE;
>>>       } else {
>>> -        env->regs[14] = (frame_addr + offsetof(sigframe, retcode))
>>> +        env->regs[14] = h2g(frame_addr + offsetof(sigframe, retcode))
>>>                           | PSW_ADDR_AMODE;
> 
> Well, it really doesn't sound good as frame_addr is a guest address (and 
> sa_restorer is too)

I would expect the sa_restorer address to actually point into the guest code 
section.

> 
> Where is the code that does the g2h() you want to nullify?

That's on the code path which usually fetches instructions from memory. In 
cpu_lduw_code called via:

s390x_tr_translate_insn->translate_one->extract_insn->ld_code2->cpu_lduw_code


Btw. Power also uses h2g while setting up the trampoline address:

...
    save_user_regs(env, mctx);
    encode_trampoline(TARGET_NR_rt_sigreturn, trampptr);

    /* The kernel checks for the presence of a VDSO here.  We don't
       emulate a vdso, so use a sigreturn system call.  */
    env->lr = (target_ulong) h2g(trampptr);
...

> 
> Thanks,
> Laurent
> 

Andreas