Re: [PATCH] linux-user: xtensa: fix signal delivery in FDPIC

[Richard Henderson]

On 11/11/23 03:22, Max Filippov wrote:
> In FDPIC signal handlers are passed around as FD pointers. Actual code
> address and GOT pointer must be fetched from memory by the QEMU code
> that implements kernel signal delivery functionality. This change is
> equivalent to the following kernel change:
> 9c2cc74fb31e ("xtensa: fix signal delivery to FDPIC process")
>
> Cc: qemu-stable@nongnu.org
> Fixes: d2796be69d7c ("linux-user: add support for xtensa FDPIC")
> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
> ---
>   linux-user/xtensa/signal.c | 28 ++++++++++++++++++++++++++--
>   1 file changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/xtensa/signal.c b/linux-user/xtensa/signal.c
> index f5fb8b5cbebe..32dcfa522919 100644
> --- a/linux-user/xtensa/signal.c
> +++ b/linux-user/xtensa/signal.c
> @@ -157,6 +157,9 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
>   {
>       abi_ulong frame_addr;
>       struct target_rt_sigframe *frame;
> +    int is_fdpic = info_is_fdpic(((TaskState *)thread_cpu->opaque)->info);
> +    abi_ulong handler = 0;
> +    abi_ulong handler_fdpic_GOT = 0;
>       uint32_t ra;
>       bool abi_call0;
>       unsigned base;
> @@ -165,6 +168,17 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
>       frame_addr = get_sigframe(ka, env, sizeof(*frame));
>       trace_user_setup_rt_frame(env, frame_addr);
>   
> +    if (is_fdpic) {
> +        abi_ulong funcdesc_ptr = ka->_sa_handler;
> +
> +        if (get_user_ual(handler, funcdesc_ptr)
> +            || get_user_ual(handler_fdpic_GOT, funcdesc_ptr + 4)) {
> +            goto give_sigsegv;
> +        }
> +    } else {
> +        handler = ka->_sa_handler;
> +    }

This part is ok, with the last hunk, because it's taking care of the fd for the 
handler.

> @@ -185,14 +199,21 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
>       }
>   
>       if (ka->sa_flags & TARGET_SA_RESTORER) {
> -        ra = ka->sa_restorer;
> +        if (is_fdpic) {
> +            if (get_user_ual(ra, ka->sa_restorer)) {
> +                unlock_user_struct(frame, frame_addr, 0);
> +                goto give_sigsegv;
> +            }
> +        } else {
> +            ra = ka->sa_restorer;
> +        }

This part is questionable.  It does match the kernel, so as far as that goes,

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

However, it does not handle the GOT register for the restorer, like we do on ARM.  That 
said, I can't find any libc sources for xtensa, or at least that aren't out of date by a 
decade, so I can't tell if libc *knows* the got register won't be loaded, and it doesn't 
matter because it only uses the sigreturn syscall.


r~