Re: [PATCH RFC 1/1] linux-user/s390x: save/restore condition code state during signal handling

[Richard Henderson]

On 6/10/21 11:58 AM, Jonathan Albrecht wrote:
> @@ -65,6 +65,10 @@ typedef struct {
>       uint8_t callee_used_stack[__SIGNAL_FRAMESIZE];
>       target_sigcontext sc;
>       target_sigregs sregs;
> +    uint32_t scc_op;
> +    uint64_t scc_src;
> +    uint64_t scc_dst;
> +    uint64_t scc_vr;
>       int signo;

Nope.  The layout of the stack frame is fixed by the kernel. Moreover, all of 
these variables are internal to qemu; you should only be exposing architectural 
state.

The bug is in save_sigregs:

>     __put_user(env->psw.mask, &sregs->regs.psw.mask);

This should use get_psw_mask(), currently declared in target/s390x/internal.h 
instead of cpu.h.

and correspondingly, in restore_sigregs:

>     __get_user(env->psw.mask, &sc->regs.psw.mask);
>     __get_user(env->psw.addr, &sc->regs.psw.addr);

this should use load_psw, and in addition it should not be allowing completely 
arbitrary changes to psw.mask.  From the kernel:

>         /* Use regs->psw.mask instead of PSW_USER_BITS to preserve PER bit. */
>         regs->psw.mask = (regs->psw.mask & ~(PSW_MASK_USER | PSW_MASK_RI)) |
>                 (user_sregs.regs.psw.mask & (PSW_MASK_USER | PSW_MASK_RI));
>         /* Check for invalid user address space control. */
>         if ((regs->psw.mask & PSW_MASK_ASC) == PSW_ASC_HOME)
>                 regs->psw.mask = PSW_ASC_PRIMARY |
>                         (regs->psw.mask & ~PSW_MASK_ASC);
>         /* Check for invalid amode */
>         if (regs->psw.mask & PSW_MASK_EA)
>                 regs->psw.mask |= PSW_MASK_BA;



r~