| Message ID | 20241017125811.447961-2-iii@linux.ibm.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | linux-user/ppc: Fix sigmask endianness issue in sigreturn | expand |
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~
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>
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
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.
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);
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(-)