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

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

[Max Filippov]

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;
+    }
+
     if (!lock_user_struct(VERIFY_WRITE, frame, frame_addr, 0)) {
         goto give_sigsegv;
     }
@@ -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;
+        }
     } else {
         /* Not used, but retain for ABI compatibility. */
         install_sigtramp(frame->retcode);
         ra = default_rt_sigreturn;
     }
     memset(env->regs, 0, sizeof(env->regs));
-    env->pc = ka->_sa_handler;
+    env->pc = handler;
     env->regs[1] = frame_addr;
     env->sregs[WINDOW_BASE] = 0;
     env->sregs[WINDOW_START] = 1;
@@ -212,6 +233,9 @@ void setup_rt_frame(int sig, struct target_sigaction *ka,
     env->regs[base + 3] = frame_addr + offsetof(struct target_rt_sigframe,
                                                 info);
     env->regs[base + 4] = frame_addr + offsetof(struct target_rt_sigframe, uc);
+    if (is_fdpic) {
+        env->regs[base + 11] = handler_fdpic_GOT;
+    }
     unlock_user_struct(frame, frame_addr, 1);
     return;
 
-- 
2.39.2

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~

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

[Max Filippov]

On Sun, Nov 12, 2023 at 8:51 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> 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

It's WIP, available at https://github.com/jcmvbkbc/uclibc-ng-xtensa
branch xtensa-1.0.44-fdpic

> 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.

That's the case. AFAU the restorer field is not for public use and the function
used as a restorer by the uclibc does not care about the GOT pointer.

-- 
Thanks.
-- Max

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

[Richard Henderson]

On 11/12/23 09:02, Max Filippov wrote:
> On Sun, Nov 12, 2023 at 8:51 AM Richard Henderson
>> 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
> 
> It's WIP, available at https://github.com/jcmvbkbc/uclibc-ng-xtensa
> branch xtensa-1.0.44-fdpic
> 
>> 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.
> 
> That's the case. AFAU the restorer field is not for public use and the function
> used as a restorer by the uclibc does not care about the GOT pointer.

Right-o, thanks for the pointer.


r~