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