linux-user/ppc: Fix sigmask endianness issue in sigreturn

[PATCH 1/2] linux-user/ppc: Fix sigmask endianness issue in sigreturn

Posted by Ilya Leoshkevich 1 year, 3 months ago

do_setcontext() copies the target sigmask without endianness handling
and then uses target_to_host_sigset_internal(), which expects a
byte-swapped one. Use target_to_host_sigset() instead.

Fixes: bcd4933a23f1 ("linux-user: ppc signal handling")
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 linux-user/ppc/signal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
index a1d8c0bccc1..24e5a02a782 100644
--- a/linux-user/ppc/signal.c
+++ b/linux-user/ppc/signal.c
@@ -628,7 +628,7 @@ static int do_setcontext(struct target_ucontext *ucp, CPUPPCState *env, int sig)
     if (!lock_user_struct(VERIFY_READ, mcp, mcp_addr, 1))
         return 1;
 
-    target_to_host_sigset_internal(&blocked, &set);
+    target_to_host_sigset(&blocked, &set);
     set_sigmask(&blocked);
     restore_user_regs(env, mcp, sig);
 
-- 
2.47.0

Re: [PATCH 1/2] linux-user/ppc: Fix sigmask endianness issue in sigreturn

Posted by Michael Tokarev 1 year, 3 months ago

17.10.2024 15:54, Ilya Leoshkevich wrote:
> do_setcontext() copies the target sigmask without endianness handling
> and then uses target_to_host_sigset_internal(), which expects a
> byte-swapped one. Use target_to_host_sigset() instead.
> 
> Fixes: bcd4933a23f1 ("linux-user: ppc signal handling")
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   linux-user/ppc/signal.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
> index a1d8c0bccc1..24e5a02a782 100644
> --- a/linux-user/ppc/signal.c
> +++ b/linux-user/ppc/signal.c
> @@ -628,7 +628,7 @@ static int do_setcontext(struct target_ucontext *ucp, CPUPPCState *env, int sig)
>       if (!lock_user_struct(VERIFY_READ, mcp, mcp_addr, 1))
>           return 1;
>   
> -    target_to_host_sigset_internal(&blocked, &set);
> +    target_to_host_sigset(&blocked, &set);
>       set_sigmask(&blocked);
>       restore_user_regs(env, mcp, sig);

Shouldn't this change be picked for all stable releases too?
Yes, the issue is very old so probably does not have big impact,
but it looks like it should be okay to fix it finally?

I'm picking it up, please let me know if I shouldn't.

Thanks,

/mjt

Re: [PATCH 1/2] linux-user/ppc: Fix sigmask endianness issue in sigreturn

Posted by Ilya Leoshkevich 1 year, 3 months ago

On Fri, 2024-10-25 at 16:53 +0300, Michael Tokarev wrote:
> 17.10.2024 15:54, Ilya Leoshkevich wrote:
> > do_setcontext() copies the target sigmask without endianness
> > handling
> > and then uses target_to_host_sigset_internal(), which expects a
> > byte-swapped one. Use target_to_host_sigset() instead.
> > 
> > Fixes: bcd4933a23f1 ("linux-user: ppc signal handling")
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >   linux-user/ppc/signal.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
> > index a1d8c0bccc1..24e5a02a782 100644
> > --- a/linux-user/ppc/signal.c
> > +++ b/linux-user/ppc/signal.c
> > @@ -628,7 +628,7 @@ static int do_setcontext(struct target_ucontext
> > *ucp, CPUPPCState *env, int sig)
> >       if (!lock_user_struct(VERIFY_READ, mcp, mcp_addr, 1))
> >           return 1;
> >   
> > -    target_to_host_sigset_internal(&blocked, &set);
> > +    target_to_host_sigset(&blocked, &set);
> >       set_sigmask(&blocked);
> >       restore_user_regs(env, mcp, sig);
> 
> Shouldn't this change be picked for all stable releases too?
> Yes, the issue is very old so probably does not have big impact,
> but it looks like it should be okay to fix it finally?
> 
> I'm picking it up, please let me know if I shouldn't.
> 
> Thanks,
> 
> /mjt

I think it's worth taking it, since it leads to weird qemu-user
crashes. I actually intended to Cc: stable here and forgot to do it,
so thanks for bringing this up.

Re: [PATCH 1/2] linux-user/ppc: Fix sigmask endianness issue in sigreturn

Posted by Philippe Mathieu-Daudé 1 year, 3 months ago

On 17/10/24 09:54, Ilya Leoshkevich wrote:
> do_setcontext() copies the target sigmask without endianness handling
> and then uses target_to_host_sigset_internal(), which expects a
> byte-swapped one. Use target_to_host_sigset() instead.

These function names are confusing.

> Fixes: bcd4933a23f1 ("linux-user: ppc signal handling")
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>   linux-user/ppc/signal.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
> index a1d8c0bccc1..24e5a02a782 100644
> --- a/linux-user/ppc/signal.c
> +++ b/linux-user/ppc/signal.c
> @@ -628,7 +628,7 @@ static int do_setcontext(struct target_ucontext *ucp, CPUPPCState *env, int sig)
>       if (!lock_user_struct(VERIFY_READ, mcp, mcp_addr, 1))
>           return 1;
>   
> -    target_to_host_sigset_internal(&blocked, &set);
> +    target_to_host_sigset(&blocked, &set);
>       set_sigmask(&blocked);
>       restore_user_regs(env, mcp, sig);
>   

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 1/2] linux-user/ppc: Fix sigmask endianness issue in sigreturn

Posted by Richard Henderson 1 year, 3 months ago

On 10/17/24 05:54, Ilya Leoshkevich wrote:
> do_setcontext() copies the target sigmask without endianness handling
> and then uses target_to_host_sigset_internal(), which expects a
> byte-swapped one. Use target_to_host_sigset() instead.
> 
> Fixes: bcd4933a23f1 ("linux-user: ppc signal handling")
> Signed-off-by: Ilya Leoshkevich<iii@linux.ibm.com>
> ---
>   linux-user/ppc/signal.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

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

r~