From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43737) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a7gtY-0006OG-To for qemu-devel@nongnu.org; Sat, 12 Dec 2015 04:55:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a7gtV-0000Mu-Mn for qemu-devel@nongnu.org; Sat, 12 Dec 2015 04:55:16 -0500 Received: from outpost1.zedat.fu-berlin.de ([130.133.4.66]:58842) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a7gtV-0000Mm-Cz for qemu-devel@nongnu.org; Sat, 12 Dec 2015 04:55:13 -0500 Message-ID: <566BEEFD.5010809@fu-berlin.de> Date: Sat, 12 Dec 2015 10:55:09 +0100 From: Michael Karcher MIME-Version: 1.0 References: <1449694457-5843-1-git-send-email-karcher@physik.fu-berlin.de> <1449694457-5843-2-git-send-email-karcher@physik.fu-berlin.de> <5668A52C.1070408@vivier.eu> In-Reply-To: <5668A52C.1070408@vivier.eu> Content-Type: multipart/mixed; boundary="------------050907060907010301060502" Subject: Re: [Qemu-devel] [PATCH 1/1] Fix do_rt_sigreturn on m68k linux userspace emulation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laurent Vivier , Michael Karcher , Riku Voipio , qemu-devel@nongnu.org Cc: glaubitz@physik.fu-berlin.de This is a multi-part message in MIME format. --------------050907060907010301060502 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit On 09.12.2015 23:03, Laurent Vivier wrote: > > Le 09/12/2015 21:54, Michael Karcher a écrit : >> do_rt_sigreturn forgets to initialize the signal mask variable before >> trying to use it to restore the mask, so the signal mask is undefined >> after do_rt_sigreturn. This bug has been in all the time since >> 7181155d when do_rt_sigreturn was implemented for m68k. >> >> Signed-off-by: Michael Karcher >> --- >> linux-user/signal.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/linux-user/signal.c b/linux-user/signal.c >> index e03ed60..ae1014b 100644 >> --- a/linux-user/signal.c >> +++ b/linux-user/signal.c >> @@ -5260,11 +5260,14 @@ long do_rt_sigreturn(CPUM68KState *env) >> abi_ulong frame_addr = env->aregs[7] - 4; >> target_sigset_t target_set; >> sigset_t set; >> - int d0; >> + int d0, i; >> >> if (!lock_user_struct(VERIFY_READ, frame, frame_addr, 1)) >> goto badframe; >> >> + for(i = 0; i < TARGET_NSIG_WORDS; i++) { >> + target_set.sig[i] = frame->uc.tuc_sigmask.sig[i]; >> + } >> target_to_host_sigset_internal(&set, &target_set); >> do_sigprocmask(SIG_SETMASK, &set, NULL); >> >> > Nice catch. > > I agree with you that the current code is completely broken, but on the > other architectures, this operation seems to be done directly by > > target_to_host_sigset(&set, &frame->uc.tuc_sigmask); > > Could you have try with that ? target_to_host_sigset does endianness swapping, while my loop does not do it. I made a test program (attached to this mail) that tests mask behaviour with "classic" and "siginfo" handlers, and shows that this patch still doesn't fix the issue completely (output should be four times 1). Instead of re-masking the SIGCHLD (bit 16) in that example, it tries to mask SIGKILL (bit 8) which is due to the endian mismatch. I will send a fixed patch shortly. Feel free to extend the program to other architectures to test it there, too. BTW: documentation of the stack frame / signature for non-SA_SIGINFO signal handlers seems to be quite lacking. There is a remark in the sigaction manpage, but that one obviously only applies to i386... Thanks for your comments, Michael Karcher --------------050907060907010301060502 Content-Type: text/x-csrc; name="sigtest.c" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="sigtest.c" //#include #include #include #include #include //#include volatile int child_blocked; volatile sig_atomic_t got_it; #if defined(__i386) #define HAVE_LEGACY void handle(int signal, struct sigcontext sc) { child_blocked = sc.oldmask == (1 << (SIGCHLD-1)); got_it = 1; } #elif defined(__amd64) #define HAVE_LEGACY void handle(int signal, struct ucontext uc) { child_blocked = ((struct sigcontext*)&uc.uc_mcontext)->oldmask == (1 << (SIGCHLD-1)); got_it = 1; } #elif defined(__mc68000) #define HAVE_LEGACY void handle(int signal, int code, struct sigcontext *sc) { child_blocked = sc->sc_mask == (1 << (SIGCHLD-1)); got_it = 1; } #endif void handle_siginfo(int signal, siginfo_t * info, void *ctx) { child_blocked = sigismember(&((ucontext_t*)ctx)->uc_sigmask, SIGCHLD); got_it = 1; } int main(void) { struct sigaction sa; sigset_t set; #ifdef HAVE_LEGACY sa.sa_flags = 0; sa.sa_handler = handle; sigemptyset(&sa.sa_mask); sigaction(SIGUSR1, &sa, NULL); sigemptyset(&set); sigaddset(&set, SIGCHLD); sigprocmask(SIG_SETMASK, &set, NULL); got_it = 0; kill(getpid(), SIGUSR1); while(got_it == 0); sigprocmask(SIG_SETMASK, NULL, &set); printf("%d %d\n", child_blocked, sigismember(&set, SIGCHLD)); #endif got_it = 0; sa.sa_flags = SA_SIGINFO; sa.sa_sigaction = handle_siginfo; sigaction(SIGUSR1, &sa, NULL); sigemptyset(&set); sigaddset(&set, SIGCHLD); sigprocmask(SIG_SETMASK, &set, NULL); got_it = 0; kill(getpid(), SIGUSR1); while(got_it == 0); sigprocmask(SIG_SETMASK, NULL, &set); printf("%d %d\n", child_blocked, sigismember(&set, SIGCHLD)); } --------------050907060907010301060502--