Re: [PATCH v18 02/13] linux-user: Add LoongArch signal support

[Richard Henderson <richard.henderson@linaro.org>]

On 6/20/22 02:33, Song Gao wrote:
> +static int restore_sigcontext(CPULoongArchState *env,
> +                               struct target_sigcontext *sc)
> +{
> +    int i;
> +    int ret = 0;
> +    struct extctx_layout extctx;
> +
> +    memset(&extctx, 0, sizeof(struct extctx_layout));
> +
> +    __get_user(extctx.flags, &sc->sc_flags);
> +
> +    ret = parse_extcontext(sc, &extctx);
> +    if (ret < 0) {
> +        goto bad;
> +    }
> +
> +    __get_user(env->pc, &sc->sc_pc);
> +    for (i = 1; i < 32; ++i) {
> +        __get_user(env->gpr[i], &sc->sc_regs[i]);
> +    }
> +
> +    if (extctx.fpu.addr) {
> +        copy_fpu_from_sigcontext(env, &extctx);
> +        restore_fp_status(env);
> +    }
> +bad:
> +    return ret;
> +}

This is missing lock_user/unlock_user somewhere.
You can't use the double-underscore __get/__put_user without having done that.

You can use the non-underscore get_user in parse_extcontext, and separately lock the 
target_fpu_context.  Failures must goto invalid.


> +void setup_rt_frame(int sig, struct target_sigaction *ka,
> +                    target_siginfo_t *info,
> +                    target_sigset_t *set, CPULoongArchState *env)
> +{
> +    struct target_rt_sigframe *frame;
> +    struct extctx_layout extctx;
> +    abi_ulong frame_addr;
> +    int i;
> +
> +    frame_addr = get_sigframe(ka, env, sizeof(*frame), &extctx);
> +    trace_user_setup_rt_frame(env, frame_addr);
> +    if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) {
> +        goto give_sigsegv;
> +    }

Similarly, this lock...

> +
> +    tswap_siginfo(&frame->rs_info, info);
> +
> +    __put_user(0, &frame->rs_uc.tuc_flags);
> +    __put_user(0, &frame->rs_uc.tuc_link);
> +    target_save_altstack(&frame->rs_uc.tuc_stack, env);
> +
> +    setup_sigcontext(env, &frame->rs_uc.tuc_mcontext, &extctx);

... fails to cover the extra memory allocated for extctx.

This is why I suggested statically allocating the extra
pieces of the signal frame *on write*.  You obviously
cannot rely on the signal frame being identical on
signal return -- the guest is allowed to create any valid
context to give to rt_sigreturn.


r~