[PATCH 1/2] FRV: Fix FRV signal handling

public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH 1/2] FRV: Fix FRV signal handling
@ 2005-11-29 21:18 David Howells
  2005-11-29 21:18 ` [PATCH 2/2] Handle TIF_RESTORE_SIGMASK for i386 David Howells
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: David Howells @ 2005-11-29 21:18 UTC (permalink / raw)
  To: torvalds, akpm, dalomar; +Cc: linux-kernel, uclinux-dev

The attached patch makes FRV signal handling work properly:

 (1) After do_notify_resume() has been called, the work flags must be checked
     again (there may be another signal to deliver or the process might require
     rescheduling for instance).

 (2) After the signal frame is set up on the userspace stack, ptrace() should
     be given an opportunity to single-step into the signal handler.

 (3) The error state from setting up a signal frame should be passed back up
     the call chain.

 (4) The segfault handler shouldn't be preemptively reset in the arch if we
     fail to deliver a SEGV signal: force_sig() will take care of that.

Signed-Off-By: David Howells <dhowells@redhat.com>
---
warthog>diffstat -p1 frv-signal-2615rc2.diff
 arch/frv/kernel/entry.S  |    2 -
 arch/frv/kernel/signal.c |   73 ++++++++++++++++++++++++++++-------------------
 2 files changed, 46 insertions(+), 29 deletions(-)

diff -uNrp linux-2.6.15-rc2-frv-shmem/arch/frv/kernel/entry.S linux-2.6.15-rc2-frv-signal/arch/frv/kernel/entry.S
--- linux-2.6.15-rc2-frv-shmem/arch/frv/kernel/entry.S	2005-03-02 12:07:44.000000000 +0000
+++ linux-2.6.15-rc2-frv-signal/arch/frv/kernel/entry.S	2005-11-29 16:01:40.000000000 +0000
@@ -1076,7 +1076,7 @@ __entry_work_notifysig:
 	LEDS		0x6410
 	ori.p		gr4,#0,gr8
 	call		do_notify_resume
-	bra		__entry_return_direct
+	bra		__entry_resume_userspace
 
 	# perform syscall entry tracing
 __syscall_trace_entry:
diff -uNrp linux-2.6.15-rc2-frv-shmem/arch/frv/kernel/signal.c linux-2.6.15-rc2-frv-signal/arch/frv/kernel/signal.c
--- linux-2.6.15-rc2-frv-shmem/arch/frv/kernel/signal.c	2005-11-01 13:18:57.000000000 +0000
+++ linux-2.6.15-rc2-frv-signal/arch/frv/kernel/signal.c	2005-11-29 16:41:11.000000000 +0000
@@ -297,7 +297,8 @@ static inline void __user *get_sigframe(
 /*
  *
  */
-static void setup_frame(int sig, struct k_sigaction *ka, sigset_t *set, struct pt_regs * regs)
+static int setup_frame(int sig, struct k_sigaction *ka, sigset_t *set,
+		       struct pt_regs *regs)
 {
 	struct sigframe __user *frame;
 	int rsig;
@@ -362,26 +363,30 @@ static void setup_frame(int sig, struct 
 
 	set_fs(USER_DS);
 
+	/* the tracer may want to single-step inside the handler */
+	if (test_thread_flag(TIF_SINGLESTEP))
+		ptrace_notify(SIGTRAP);
+
 #if DEBUG_SIG
 	printk("SIG deliver %d (%s:%d): sp=%p pc=%lx ra=%p\n",
-		sig, current->comm, current->pid, frame, regs->pc, frame->pretcode);
+	       sig, current->comm, current->pid, frame, regs->pc,
+	       frame->pretcode);
 #endif
 
-	return;
+	return 1;
 
 give_sigsegv:
-	if (sig == SIGSEGV)
-		ka->sa.sa_handler = SIG_DFL;
-
 	force_sig(SIGSEGV, current);
+	return 0;
+
 } /* end setup_frame() */
 
 /*****************************************************************************/
 /*
  *
  */
-static void setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
-			   sigset_t *set, struct pt_regs * regs)
+static int setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
+			  sigset_t *set, struct pt_regs * regs)
 {
 	struct rt_sigframe __user *frame;
 	int rsig;
@@ -457,17 +462,21 @@ static void setup_rt_frame(int sig, stru
 
 	set_fs(USER_DS);
 
+	/* the tracer may want to single-step inside the handler */
+	if (test_thread_flag(TIF_SINGLESTEP))
+		ptrace_notify(SIGTRAP);
+
 #if DEBUG_SIG
 	printk("SIG deliver %d (%s:%d): sp=%p pc=%lx ra=%p\n",
-		sig, current->comm, current->pid, frame, regs->pc, frame->pretcode);
+		sig, current->comm, current->pid, frame, regs->pc,
+	       frame->pretcode);
 #endif
 
-	return;
+	return 1;
 
 give_sigsegv:
-	if (sig == SIGSEGV)
-		ka->sa.sa_handler = SIG_DFL;
 	force_sig(SIGSEGV, current);
+	return 0;
 
 } /* end setup_rt_frame() */
 
@@ -475,10 +484,12 @@ give_sigsegv:
 /*
  * OK, we're invoking a handler
  */
-static void handle_signal(unsigned long sig, siginfo_t *info,
-			  struct k_sigaction *ka, sigset_t *oldset,
-			  struct pt_regs *regs)
+static int handle_signal(unsigned long sig, siginfo_t *info,
+			 struct k_sigaction *ka, sigset_t *oldset,
+			 struct pt_regs *regs)
 {
+	int ret;
+
 	/* Are we from a system call? */
 	if (in_syscall(regs)) {
 		/* If so, check system call restarting.. */
@@ -493,6 +504,7 @@ static void handle_signal(unsigned long 
 				regs->gr8 = -EINTR;
 				break;
 			}
+
 			/* fallthrough */
 		case -ERESTARTNOINTR:
 			regs->gr8 = regs->orig_gr8;
@@ -502,16 +514,22 @@ static void handle_signal(unsigned long 
 
 	/* Set up the stack frame */
 	if (ka->sa.sa_flags & SA_SIGINFO)
-		setup_rt_frame(sig, ka, info, oldset, regs);
+		ret = setup_rt_frame(sig, ka, info, oldset, regs);
 	else
-		setup_frame(sig, ka, oldset, regs);
+		ret = setup_frame(sig, ka, oldset, regs);
+
+	if (ret) {
+		spin_lock_irq(&current->sighand->siglock);
+		sigorsets(&current->blocked, &current->blocked,
+			  &ka->sa.sa_mask);
+		if (!(ka->sa.sa_flags & SA_NODEFER))
+			sigaddset(&current->blocked, sig);
+		recalc_sigpending();
+		spin_unlock_irq(&current->sighand->siglock);
+	}
+
+	return ret;
 
-	spin_lock_irq(&current->sighand->siglock);
-	sigorsets(&current->blocked, &current->blocked, &ka->sa.sa_mask);
-	if (!(ka->sa.sa_flags & SA_NODEFER))
-		sigaddset(&current->blocked, sig);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
 } /* end handle_signal() */
 
 /*****************************************************************************/
@@ -542,12 +560,10 @@ int do_signal(struct pt_regs *regs, sigs
 		oldset = &current->blocked;
 
 	signr = get_signal_to_deliver(&info, &ka, regs, NULL);
-	if (signr > 0) {
-		handle_signal(signr, &info, &ka, oldset, regs);
-		return 1;
-	}
+	if (signr > 0)
+		return handle_signal(signr, &info, &ka, oldset, regs);
 
- no_signal:
+no_signal:
 	/* Did we come from a system call? */
 	if (regs->syscallno >= 0) {
 		/* Restart the system call - no handlers present */
@@ -565,6 +581,7 @@ int do_signal(struct pt_regs *regs, sigs
 	}
 
 	return 0;
+
 } /* end do_signal() */
 
 /*****************************************************************************/

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] Handle TIF_RESTORE_SIGMASK for i386
  2005-11-29 21:18 [PATCH 1/2] FRV: Fix FRV signal handling David Howells
@ 2005-11-29 21:18 ` David Howells
  2005-11-29 21:18 ` [PATCH 2/2] FRV: Improve signal handling David Howells
  2005-11-29 21:18 ` [PATCH 1/2] Handle TIF_RESTORE_SIGMASK for FRV David Howells
  2 siblings, 0 replies; 4+ messages in thread
From: David Howells @ 2005-11-29 21:18 UTC (permalink / raw)
  To: torvalds, akpm, dalomar; +Cc: linux-kernel, uclinux-dev

The attached patch handles TIF_RESTORE_SIGMASK as added by David Woodhouse's
patch entitled:

	[PATCH] 2/3 Add TIF_RESTORE_SIGMASK support for arch/powerpc
	[PATCH] 3/3 Generic sys_rt_sigsuspend

It does the following:

 (1) Declares TIF_RESTORE_SIGMASK for i386.

 (2) Invokes it over to do_signal() when TIF_RESTORE_SIGMASK is set.

 (3) Makes do_signal() support TIF_RESTORE_SIGMASK, using the signal mask saved
     in current->saved_sigmask.

 (4) Discards sys_rt_sigsuspend() from the arch, using the generic one instead.

 (5) Makes sys_sigsuspend() save the signal mask and set TIF_RESTORE_SIGMASK
     rather than attempting to fudge the return registers.

 (6) Makes sys_sigsuspend() return -ERESTARTNOHAND rather than looping
     intrinsically.

 (7) Makes setup_frame(), setup_rt_frame() and handle_signal() return 0 or
     -EFAULT rather than true/false to be consistent with the rest of the
     kernel.

Due to the fact do_signal() is then only called from one place:

 (8) Makes do_signal() no longer have a return value is it was just being
     ignored; force_sig() takes care of this.

 (9) Discards the old sigmask argument to do_signal() as it's no longer
     necessary.

(10) Makes do_signal() static.

(11) Marks the second argument to do_notify_resume() as unused. The unused
     argument should remain in the middle as the arguments are passed in as
     registers, and the ordering is specific in entry.S

Given the way do_signal() is now no longer called from sys_{,rt_}sigsuspend(),
they no longer need access to the exception frame, and so can just take
arguments normally.

This patch depends on sys_rt_sigsuspend patch.

Signed-Off-By: David Howells <dhowells@redhat.com>
---
warthog>diffstat -p1 i386-tifrestore-2615rc2.diff
 arch/i386/kernel/signal.c      |  109 ++++++++++++++++++-----------------------
 include/asm-i386/signal.h      |    1 
 include/asm-i386/thread_info.h |    2 
 include/asm-i386/unistd.h      |    1 
 4 files changed, 51 insertions(+), 62 deletions(-)

diff -uNrp linux-2.6.15-rc2-frv-dosignal/arch/i386/kernel/signal.c linux-2.6.15-rc2-frv-tifrestore/arch/i386/kernel/signal.c
--- linux-2.6.15-rc2-frv-dosignal/arch/i386/kernel/signal.c	2005-11-01 13:18:57.000000000 +0000
+++ linux-2.6.15-rc2-frv-tifrestore/arch/i386/kernel/signal.c	2005-11-29 18:43:49.000000000 +0000
@@ -37,51 +37,17 @@
 asmlinkage int
 sys_sigsuspend(int history0, int history1, old_sigset_t mask)
 {
-	struct pt_regs * regs = (struct pt_regs *) &history0;
-	sigset_t saveset;
-
 	mask &= _BLOCKABLE;
 	spin_lock_irq(&current->sighand->siglock);
-	saveset = current->blocked;
+	current->saved_sigmask = current->blocked;
 	siginitset(&current->blocked, mask);
 	recalc_sigpending();
 	spin_unlock_irq(&current->sighand->siglock);
 
-	regs->eax = -EINTR;
-	while (1) {
-		current->state = TASK_INTERRUPTIBLE;
-		schedule();
-		if (do_signal(regs, &saveset))
-			return -EINTR;
-	}
-}
-
-asmlinkage int
-sys_rt_sigsuspend(struct pt_regs regs)
-{
-	sigset_t saveset, newset;
-
-	/* XXX: Don't preclude handling different sized sigset_t's.  */
-	if (regs.ecx != sizeof(sigset_t))
-		return -EINVAL;
-
-	if (copy_from_user(&newset, (sigset_t __user *)regs.ebx, sizeof(newset)))
-		return -EFAULT;
-	sigdelsetmask(&newset, ~_BLOCKABLE);
-
-	spin_lock_irq(&current->sighand->siglock);
-	saveset = current->blocked;
-	current->blocked = newset;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
-
-	regs.eax = -EINTR;
-	while (1) {
-		current->state = TASK_INTERRUPTIBLE;
-		schedule();
-		if (do_signal(&regs, &saveset))
-			return -EINTR;
-	}
+	current->state = TASK_INTERRUPTIBLE;
+	schedule();
+	set_thread_flag(TIF_RESTORE_SIGMASK);
+	return -ERESTARTNOHAND;
 }
 
 asmlinkage int 
@@ -433,11 +399,11 @@ static int setup_frame(int sig, struct k
 		current->comm, current->pid, frame, regs->eip, frame->pretcode);
 #endif
 
-	return 1;
+	return 0;
 
 give_sigsegv:
 	force_sigsegv(sig, current);
-	return 0;
+	return -EFAULT;
 }
 
 static int setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
@@ -527,11 +493,11 @@ static int setup_rt_frame(int sig, struc
 		current->comm, current->pid, frame, regs->eip, frame->pretcode);
 #endif
 
-	return 1;
+	return 0;
 
 give_sigsegv:
 	force_sigsegv(sig, current);
-	return 0;
+	return -EFAULT;
 }
 
 /*
@@ -581,7 +547,7 @@ handle_signal(unsigned long sig, siginfo
 	else
 		ret = setup_frame(sig, ka, oldset, regs);
 
-	if (ret) {
+	if (ret == 0) {
 		spin_lock_irq(&current->sighand->siglock);
 		sigorsets(&current->blocked,&current->blocked,&ka->sa.sa_mask);
 		if (!(ka->sa.sa_flags & SA_NODEFER))
@@ -598,11 +564,12 @@ handle_signal(unsigned long sig, siginfo
  * want to handle. Thus you cannot kill init even with a SIGKILL even by
  * mistake.
  */
-int fastcall do_signal(struct pt_regs *regs, sigset_t *oldset)
+static void fastcall do_signal(struct pt_regs *regs)
 {
 	siginfo_t info;
 	int signr;
 	struct k_sigaction ka;
+	sigset_t *oldset;
 
 	/*
 	 * We want the common case to go fast, which
@@ -613,12 +580,14 @@ int fastcall do_signal(struct pt_regs *r
  	 * CS suffices.
 	 */
 	if (!user_mode(regs))
-		return 1;
+		return;
 
 	if (try_to_freeze())
 		goto no_signal;
 
-	if (!oldset)
+	if (test_thread_flag(TIF_RESTORE_SIGMASK))
+		oldset = &current->saved_sigmask;
+	else
 		oldset = &current->blocked;
 
 	signr = get_signal_to_deliver(&info, &ka, regs, NULL);
@@ -628,38 +597,55 @@ int fastcall do_signal(struct pt_regs *r
 		 * have been cleared if the watchpoint triggered
 		 * inside the kernel.
 		 */
-		if (unlikely(current->thread.debugreg[7])) {
+		if (unlikely(current->thread.debugreg[7]))
 			set_debugreg(current->thread.debugreg[7], 7);
-		}
 
 		/* Whee!  Actually deliver the signal.  */
-		return handle_signal(signr, &info, &ka, oldset, regs);
+		if (handle_signal(signr, &info, &ka, oldset, regs) == 0) {
+			/* a signal was successfully delivered; the saved
+			 * sigmask will have been stored in the signal frame,
+			 * and will be restored by sigreturn, so we can simply
+			 * clear the TIF_RESTORE_SIGMASK flag */
+			if (test_thread_flag(TIF_RESTORE_SIGMASK))
+				clear_thread_flag(TIF_RESTORE_SIGMASK);
+		}
+
+		return;
 	}
 
- no_signal:
+no_signal:
 	/* Did we come from a system call? */
 	if (regs->orig_eax >= 0) {
 		/* Restart the system call - no handlers present */
-		if (regs->eax == -ERESTARTNOHAND ||
-		    regs->eax == -ERESTARTSYS ||
-		    regs->eax == -ERESTARTNOINTR) {
+		switch (regs->eax) {
+		case -ERESTARTNOHAND:
+		case -ERESTARTSYS:
+		case -ERESTARTNOINTR:
 			regs->eax = regs->orig_eax;
 			regs->eip -= 2;
-		}
-		if (regs->eax == -ERESTART_RESTARTBLOCK){
+			break;
+
+		case -ERESTART_RESTARTBLOCK:
 			regs->eax = __NR_restart_syscall;
 			regs->eip -= 2;
+			break;
 		}
 	}
-	return 0;
+
+	/* if there's no signal to deliver, we just put the saved sigmask
+	 * back */
+	if (test_thread_flag(TIF_RESTORE_SIGMASK)) {
+		clear_thread_flag(TIF_RESTORE_SIGMASK);
+		sigprocmask(SIG_SETMASK, &current->saved_sigmask, NULL);
+	}
 }
 
 /*
  * notification of userspace execution resumption
- * - triggered by current->work.notify_resume
+ * - triggered by the TIF_WORK_MASK flags
  */
 __attribute__((regparm(3)))
-void do_notify_resume(struct pt_regs *regs, sigset_t *oldset,
+void do_notify_resume(struct pt_regs *regs, void *_unused,
 		      __u32 thread_info_flags)
 {
 	/* Pending single-step? */
@@ -667,9 +653,10 @@ void do_notify_resume(struct pt_regs *re
 		regs->eflags |= TF_MASK;
 		clear_thread_flag(TIF_SINGLESTEP);
 	}
+
 	/* deal with pending signal delivery */
-	if (thread_info_flags & _TIF_SIGPENDING)
-		do_signal(regs,oldset);
+	if (thread_info_flags & (_TIF_SIGPENDING | _TIF_RESTORE_SIGMASK))
+		do_signal(regs);
 	
 	clear_thread_flag(TIF_IRET);
 }
diff -uNrp linux-2.6.15-rc2-frv-dosignal/include/asm-i386/signal.h linux-2.6.15-rc2-frv-tifrestore/include/asm-i386/signal.h
--- linux-2.6.15-rc2-frv-dosignal/include/asm-i386/signal.h	2005-11-23 12:09:19.000000000 +0000
+++ linux-2.6.15-rc2-frv-tifrestore/include/asm-i386/signal.h	2005-11-29 18:36:53.000000000 +0000
@@ -218,7 +218,6 @@ static __inline__ int sigfindinword(unsi
 }
 
 struct pt_regs;
-extern int FASTCALL(do_signal(struct pt_regs *regs, sigset_t *oldset));
 
 #define ptrace_signal_deliver(regs, cookie)		\
 	do {						\
diff -uNrp linux-2.6.15-rc2-frv-dosignal/include/asm-i386/thread_info.h linux-2.6.15-rc2-frv-tifrestore/include/asm-i386/thread_info.h
--- linux-2.6.15-rc2-frv-dosignal/include/asm-i386/thread_info.h	2005-11-01 13:19:17.000000000 +0000
+++ linux-2.6.15-rc2-frv-tifrestore/include/asm-i386/thread_info.h	2005-11-29 18:22:21.000000000 +0000
@@ -142,6 +142,7 @@ register unsigned long current_stack_poi
 #define TIF_SYSCALL_EMU		6	/* syscall emulation active */
 #define TIF_SYSCALL_AUDIT	7	/* syscall auditing active */
 #define TIF_SECCOMP		8	/* secure computing */
+#define TIF_RESTORE_SIGMASK	9	/* restore signal mask in do_signal() */
 #define TIF_POLLING_NRFLAG	16	/* true if poll_idle() is polling TIF_NEED_RESCHED */
 #define TIF_MEMDIE		17
 
@@ -154,6 +155,7 @@ register unsigned long current_stack_poi
 #define _TIF_SYSCALL_EMU	(1<<TIF_SYSCALL_EMU)
 #define _TIF_SYSCALL_AUDIT	(1<<TIF_SYSCALL_AUDIT)
 #define _TIF_SECCOMP		(1<<TIF_SECCOMP)
+#define _TIF_RESTORE_SIGMASK	(1<<TIF_RESTORE_SIGMASK)
 #define _TIF_POLLING_NRFLAG	(1<<TIF_POLLING_NRFLAG)
 
 /* work to do on interrupt/exception return */
diff -uNrp linux-2.6.15-rc2-frv-dosignal/include/asm-i386/unistd.h linux-2.6.15-rc2-frv-tifrestore/include/asm-i386/unistd.h
--- linux-2.6.15-rc2-frv-dosignal/include/asm-i386/unistd.h	2005-11-23 12:09:19.000000000 +0000
+++ linux-2.6.15-rc2-frv-tifrestore/include/asm-i386/unistd.h	2005-11-29 18:21:26.000000000 +0000
@@ -416,6 +416,7 @@ __syscall_return(type,__res); \
 #define __ARCH_WANT_SYS_SIGPENDING
 #define __ARCH_WANT_SYS_SIGPROCMASK
 #define __ARCH_WANT_SYS_RT_SIGACTION
+#define __ARCH_WANT_SYS_RT_SIGSUSPEND
 #endif
 
 #ifdef __KERNEL_SYSCALLS__

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 2/2] FRV: Improve signal handling
  2005-11-29 21:18 [PATCH 1/2] FRV: Fix FRV signal handling David Howells
  2005-11-29 21:18 ` [PATCH 2/2] Handle TIF_RESTORE_SIGMASK for i386 David Howells
@ 2005-11-29 21:18 ` David Howells
  2005-11-29 21:18 ` [PATCH 1/2] Handle TIF_RESTORE_SIGMASK for FRV David Howells
  2 siblings, 0 replies; 4+ messages in thread
From: David Howells @ 2005-11-29 21:18 UTC (permalink / raw)
  To: torvalds, akpm, dalomar; +Cc: linux-kernel, uclinux-dev

The attached patch improves the signal handling:

 (1) It makes do_signal() static as it isn't called from anywhere outside of
     the arch code.

 (2) It removes the regs argument to all the static functions within that file,
     using __frame instead (which is the same thing held in a global register).

Signed-Off-By: David Howells <dhowells@redhat.com>
---
warthog>diffstat -p1 frv-dosignal-2615rc2.diff
 arch/frv/kernel/signal.c |  102 +++++++++++++++++++++++------------------------
 include/asm-frv/signal.h |    1 
 2 files changed, 50 insertions(+), 53 deletions(-)

diff -uNrp linux-2.6.15-rc2-frv-signal/arch/frv/kernel/signal.c linux-2.6.15-rc2-frv-dosignal/arch/frv/kernel/signal.c
--- linux-2.6.15-rc2-frv-signal/arch/frv/kernel/signal.c	2005-11-29 16:41:11.000000000 +0000
+++ linux-2.6.15-rc2-frv-dosignal/arch/frv/kernel/signal.c	2005-11-29 16:55:12.000000000 +0000
@@ -35,7 +35,7 @@ struct fdpic_func_descriptor {
 	unsigned long	GOT;
 };
 
-asmlinkage int do_signal(struct pt_regs *regs, sigset_t *oldset);
+static int do_signal(sigset_t *oldset);
 
 /*
  * Atomically swap in the new signal mask, and wait for a signal.
@@ -55,7 +55,7 @@ asmlinkage int sys_sigsuspend(int histor
 	while (1) {
 		current->state = TASK_INTERRUPTIBLE;
 		schedule();
-		if (do_signal(__frame, &saveset))
+		if (do_signal(&saveset))
 			/* return the signal number as the return value of this function
 			 * - this is an utterly evil hack. syscalls should not invoke do_signal()
 			 *   as entry.S sets regs->gr8 to the return value of the system call
@@ -91,7 +91,7 @@ asmlinkage int sys_rt_sigsuspend(sigset_
 	while (1) {
 		current->state = TASK_INTERRUPTIBLE;
 		schedule();
-		if (do_signal(__frame, &saveset))
+		if (do_signal(&saveset))
 			/* return the signal number as the return value of this function
 			 * - this is an utterly evil hack. syscalls should not invoke do_signal()
 			 *   as entry.S sets regs->gr8 to the return value of the system call
@@ -276,13 +276,12 @@ static int setup_sigcontext(struct sigco
  * Determine which stack to use..
  */
 static inline void __user *get_sigframe(struct k_sigaction *ka,
-					struct pt_regs *regs,
 					size_t frame_size)
 {
 	unsigned long sp;
 
 	/* Default to using normal stack */
-	sp = regs->sp;
+	sp = __frame->sp;
 
 	/* This is the X/Open sanctioned signal stack switching.  */
 	if (ka->sa.sa_flags & SA_ONSTACK) {
@@ -291,19 +290,19 @@ static inline void __user *get_sigframe(
 	}
 
 	return (void __user *) ((sp - frame_size) & ~7UL);
+
 } /* end get_sigframe() */
 
 /*****************************************************************************/
 /*
  *
  */
-static int setup_frame(int sig, struct k_sigaction *ka, sigset_t *set,
-		       struct pt_regs *regs)
+static int setup_frame(int sig, struct k_sigaction *ka, sigset_t *set)
 {
 	struct sigframe __user *frame;
 	int rsig;
 
-	frame = get_sigframe(ka, regs, sizeof(*frame));
+	frame = get_sigframe(ka, sizeof(*frame));
 
 	if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame)))
 		goto give_sigsegv;
@@ -347,18 +346,18 @@ static int setup_frame(int sig, struct k
 	}
 
 	/* set up registers for signal handler */
-	regs->sp   = (unsigned long) frame;
-	regs->lr   = (unsigned long) &frame->retcode;
-	regs->gr8  = sig;
+	__frame->sp   = (unsigned long) frame;
+	__frame->lr   = (unsigned long) &frame->retcode;
+	__frame->gr8  = sig;
 
 	if (get_personality & FDPIC_FUNCPTRS) {
 		struct fdpic_func_descriptor __user *funcptr =
 			(struct fdpic_func_descriptor *) ka->sa.sa_handler;
-		__get_user(regs->pc, &funcptr->text);
-		__get_user(regs->gr15, &funcptr->GOT);
+		__get_user(__frame->pc, &funcptr->text);
+		__get_user(__frame->gr15, &funcptr->GOT);
 	} else {
-		regs->pc   = (unsigned long) ka->sa.sa_handler;
-		regs->gr15 = 0;
+		__frame->pc   = (unsigned long) ka->sa.sa_handler;
+		__frame->gr15 = 0;
 	}
 
 	set_fs(USER_DS);
@@ -369,7 +368,7 @@ static int setup_frame(int sig, struct k
 
 #if DEBUG_SIG
 	printk("SIG deliver %d (%s:%d): sp=%p pc=%lx ra=%p\n",
-	       sig, current->comm, current->pid, frame, regs->pc,
+	       sig, current->comm, current->pid, frame, __frame->pc,
 	       frame->pretcode);
 #endif
 
@@ -386,12 +385,12 @@ give_sigsegv:
  *
  */
 static int setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
-			  sigset_t *set, struct pt_regs * regs)
+			  sigset_t *set)
 {
 	struct rt_sigframe __user *frame;
 	int rsig;
 
-	frame = get_sigframe(ka, regs, sizeof(*frame));
+	frame = get_sigframe(ka, sizeof(*frame));
 
 	if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame)))
 		goto give_sigsegv;
@@ -414,7 +413,7 @@ static int setup_rt_frame(int sig, struc
 	if (__put_user(0, &frame->uc.uc_flags) ||
 	    __put_user(0, &frame->uc.uc_link) ||
 	    __put_user((void*)current->sas_ss_sp, &frame->uc.uc_stack.ss_sp) ||
-	    __put_user(sas_ss_flags(regs->sp), &frame->uc.uc_stack.ss_flags) ||
+	    __put_user(sas_ss_flags(__frame->sp), &frame->uc.uc_stack.ss_flags) ||
 	    __put_user(current->sas_ss_size, &frame->uc.uc_stack.ss_size))
 		goto give_sigsegv;
 
@@ -445,19 +444,19 @@ static int setup_rt_frame(int sig, struc
 	}
 
 	/* Set up registers for signal handler */
-	regs->sp  = (unsigned long) frame;
-	regs->lr   = (unsigned long) &frame->retcode;
-	regs->gr8 = sig;
-	regs->gr9 = (unsigned long) &frame->info;
+	__frame->sp  = (unsigned long) frame;
+	__frame->lr   = (unsigned long) &frame->retcode;
+	__frame->gr8 = sig;
+	__frame->gr9 = (unsigned long) &frame->info;
 
 	if (get_personality & FDPIC_FUNCPTRS) {
 		struct fdpic_func_descriptor *funcptr =
 			(struct fdpic_func_descriptor __user *) ka->sa.sa_handler;
-		__get_user(regs->pc, &funcptr->text);
-		__get_user(regs->gr15, &funcptr->GOT);
+		__get_user(__frame->pc, &funcptr->text);
+		__get_user(__frame->gr15, &funcptr->GOT);
 	} else {
-		regs->pc   = (unsigned long) ka->sa.sa_handler;
-		regs->gr15 = 0;
+		__frame->pc   = (unsigned long) ka->sa.sa_handler;
+		__frame->gr15 = 0;
 	}
 
 	set_fs(USER_DS);
@@ -468,7 +467,7 @@ static int setup_rt_frame(int sig, struc
 
 #if DEBUG_SIG
 	printk("SIG deliver %d (%s:%d): sp=%p pc=%lx ra=%p\n",
-		sig, current->comm, current->pid, frame, regs->pc,
+	       sig, current->comm, current->pid, frame, __frame->pc,
 	       frame->pretcode);
 #endif
 
@@ -485,38 +484,37 @@ give_sigsegv:
  * OK, we're invoking a handler
  */
 static int handle_signal(unsigned long sig, siginfo_t *info,
-			 struct k_sigaction *ka, sigset_t *oldset,
-			 struct pt_regs *regs)
+			 struct k_sigaction *ka, sigset_t *oldset)
 {
 	int ret;
 
 	/* Are we from a system call? */
-	if (in_syscall(regs)) {
+	if (in_syscall(__frame)) {
 		/* If so, check system call restarting.. */
-		switch (regs->gr8) {
+		switch (__frame->gr8) {
 		case -ERESTART_RESTARTBLOCK:
 		case -ERESTARTNOHAND:
-			regs->gr8 = -EINTR;
+			__frame->gr8 = -EINTR;
 			break;
 
 		case -ERESTARTSYS:
 			if (!(ka->sa.sa_flags & SA_RESTART)) {
-				regs->gr8 = -EINTR;
+				__frame->gr8 = -EINTR;
 				break;
 			}
 
 			/* fallthrough */
 		case -ERESTARTNOINTR:
-			regs->gr8 = regs->orig_gr8;
-			regs->pc -= 4;
+			__frame->gr8 = __frame->orig_gr8;
+			__frame->pc -= 4;
 		}
 	}
 
 	/* Set up the stack frame */
 	if (ka->sa.sa_flags & SA_SIGINFO)
-		ret = setup_rt_frame(sig, ka, info, oldset, regs);
+		ret = setup_rt_frame(sig, ka, info, oldset);
 	else
-		ret = setup_frame(sig, ka, oldset, regs);
+		ret = setup_frame(sig, ka, oldset);
 
 	if (ret) {
 		spin_lock_irq(&current->sighand->siglock);
@@ -538,7 +536,7 @@ static int handle_signal(unsigned long s
  * want to handle. Thus you cannot kill init even with a SIGKILL even by
  * mistake.
  */
-int do_signal(struct pt_regs *regs, sigset_t *oldset)
+static int do_signal(sigset_t *oldset)
 {
 	struct k_sigaction ka;
 	siginfo_t info;
@@ -550,7 +548,7 @@ int do_signal(struct pt_regs *regs, sigs
 	 * kernel mode. Just return without doing anything
 	 * if so.
 	 */
-	if (!user_mode(regs))
+	if (!user_mode(__frame))
 		return 1;
 
 	if (try_to_freeze())
@@ -559,24 +557,24 @@ int do_signal(struct pt_regs *regs, sigs
 	if (!oldset)
 		oldset = &current->blocked;
 
-	signr = get_signal_to_deliver(&info, &ka, regs, NULL);
+	signr = get_signal_to_deliver(&info, &ka, __frame, NULL);
 	if (signr > 0)
-		return handle_signal(signr, &info, &ka, oldset, regs);
+		return handle_signal(signr, &info, &ka, oldset);
 
 no_signal:
 	/* Did we come from a system call? */
-	if (regs->syscallno >= 0) {
+	if (__frame->syscallno >= 0) {
 		/* Restart the system call - no handlers present */
-		if (regs->gr8 == -ERESTARTNOHAND ||
-		    regs->gr8 == -ERESTARTSYS ||
-		    regs->gr8 == -ERESTARTNOINTR) {
-			regs->gr8 = regs->orig_gr8;
-			regs->pc -= 4;
+		if (__frame->gr8 == -ERESTARTNOHAND ||
+		    __frame->gr8 == -ERESTARTSYS ||
+		    __frame->gr8 == -ERESTARTNOINTR) {
+			__frame->gr8 = __frame->orig_gr8;
+			__frame->pc -= 4;
 		}
 
-		if (regs->gr8 == -ERESTART_RESTARTBLOCK){
-			regs->gr8 = __NR_restart_syscall;
-			regs->pc -= 4;
+		if (__frame->gr8 == -ERESTART_RESTARTBLOCK){
+			__frame->gr8 = __NR_restart_syscall;
+			__frame->pc -= 4;
 		}
 	}
 
@@ -597,6 +595,6 @@ asmlinkage void do_notify_resume(__u32 t
 
 	/* deal with pending signal delivery */
 	if (thread_info_flags & _TIF_SIGPENDING)
-		do_signal(__frame, NULL);
+		do_signal(NULL);
 
 } /* end do_notify_resume() */
diff -uNrp linux-2.6.15-rc2-frv-signal/include/asm-frv/signal.h linux-2.6.15-rc2-frv-dosignal/include/asm-frv/signal.h
--- linux-2.6.15-rc2-frv-signal/include/asm-frv/signal.h	2005-06-22 13:52:26.000000000 +0100
+++ linux-2.6.15-rc2-frv-dosignal/include/asm-frv/signal.h	2005-11-29 16:54:26.000000000 +0000
@@ -151,7 +151,6 @@ typedef struct sigaltstack {
 	size_t ss_size;
 } stack_t;
 
-extern int do_signal(struct pt_regs *regs, sigset_t *oldset);
 #define ptrace_signal_deliver(regs, cookie) do { } while (0)
 
 #ifdef __KERNEL__

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] Handle TIF_RESTORE_SIGMASK for FRV
  2005-11-29 21:18 [PATCH 1/2] FRV: Fix FRV signal handling David Howells
  2005-11-29 21:18 ` [PATCH 2/2] Handle TIF_RESTORE_SIGMASK for i386 David Howells
  2005-11-29 21:18 ` [PATCH 2/2] FRV: Improve signal handling David Howells
@ 2005-11-29 21:18 ` David Howells
  2 siblings, 0 replies; 4+ messages in thread
From: David Howells @ 2005-11-29 21:18 UTC (permalink / raw)
  To: torvalds, akpm, dalomar; +Cc: linux-kernel, uclinux-dev

The attached patch handles TIF_RESTORE_SIGMASK as added by David Woodhouse's
patch entitled:

	[PATCH] 2/3 Add TIF_RESTORE_SIGMASK support for arch/powerpc
	[PATCH] 3/3 Generic sys_rt_sigsuspend

It does the following:

 (1) Declares TIF_RESTORE_SIGMASK for FRV.

 (2) Invokes it over to do_signal() when TIF_RESTORE_SIGMASK is set.

 (3) Makes do_signal() support TIF_RESTORE_SIGMASK, using the signal mask saved
     in current->saved_sigmask.

 (4) Discards sys_rt_sigsuspend() from the arch, using the generic one instead.

 (5) Makes sys_sigsuspend() save the signal mask and set TIF_RESTORE_SIGMASK
     rather than attempting to fudge the return registers.

 (6) Makes sys_sigsuspend() return -ERESTARTNOHAND rather than looping
     intrinsically.

 (7) Makes setup_frame(), setup_rt_frame() and handle_signal() return 0 or
     -EFAULT rather than true/false to be consistent with the rest of the
      kernel.

Due to the fact do_signal() is then only called from one place:

 (8) Make do_signal() no longer have a return value is it was just being
     ignored; force_sig() takes care of this.

 (9) Discards the old sigmask argument to do_signal() as it's no longer
     necessary.

This patch depends on the FRV signalling patches as well as the
sys_rt_sigsuspend patch.

Signed-Off-By: David Howells <dhowells@redhat.com>
---
warthog>diffstat -p1 frv-tifrestore-2615rc2.diff
 arch/frv/kernel/signal.c      |  120 +++++++++++++++---------------------------
 include/asm-frv/thread_info.h |    2 
 include/asm-frv/unistd.h      |    1 
 3 files changed, 47 insertions(+), 76 deletions(-)

diff -uNrp linux-2.6.15-rc2-frv-dosignal/arch/frv/kernel/signal.c linux-2.6.15-rc2-frv-tifrestore/arch/frv/kernel/signal.c
--- linux-2.6.15-rc2-frv-dosignal/arch/frv/kernel/signal.c	2005-11-29 16:55:12.000000000 +0000
+++ linux-2.6.15-rc2-frv-tifrestore/arch/frv/kernel/signal.c	2005-11-29 18:54:24.000000000 +0000
@@ -35,74 +35,22 @@ struct fdpic_func_descriptor {
 	unsigned long	GOT;
 };
 
-static int do_signal(sigset_t *oldset);
-
 /*
  * Atomically swap in the new signal mask, and wait for a signal.
  */
 asmlinkage int sys_sigsuspend(int history0, int history1, old_sigset_t mask)
 {
-	sigset_t saveset;
-
 	mask &= _BLOCKABLE;
 	spin_lock_irq(&current->sighand->siglock);
-	saveset = current->blocked;
+	current->saved_sigmask = current->blocked;
 	siginitset(&current->blocked, mask);
 	recalc_sigpending();
 	spin_unlock_irq(&current->sighand->siglock);
 
-	__frame->gr8 = -EINTR;
-	while (1) {
-		current->state = TASK_INTERRUPTIBLE;
-		schedule();
-		if (do_signal(&saveset))
-			/* return the signal number as the return value of this function
-			 * - this is an utterly evil hack. syscalls should not invoke do_signal()
-			 *   as entry.S sets regs->gr8 to the return value of the system call
-			 * - we can't just use sigpending() as we'd have to discard SIG_IGN signals
-			 *   and call waitpid() if SIGCHLD needed discarding
-			 * - this only works on the i386 because it passes arguments to the signal
-			 *   handler on the stack, and the return value in EAX is effectively
-			 *   discarded
-			 */
-			return __frame->gr8;
-	}
-}
-
-asmlinkage int sys_rt_sigsuspend(sigset_t __user *unewset, size_t sigsetsize)
-{
-	sigset_t saveset, newset;
-
-	/* XXX: Don't preclude handling different sized sigset_t's.  */
-	if (sigsetsize != sizeof(sigset_t))
-		return -EINVAL;
-
-	if (copy_from_user(&newset, unewset, sizeof(newset)))
-		return -EFAULT;
-	sigdelsetmask(&newset, ~_BLOCKABLE);
-
-	spin_lock_irq(&current->sighand->siglock);
-	saveset = current->blocked;
-	current->blocked = newset;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
-
-	__frame->gr8 = -EINTR;
-	while (1) {
-		current->state = TASK_INTERRUPTIBLE;
-		schedule();
-		if (do_signal(&saveset))
-			/* return the signal number as the return value of this function
-			 * - this is an utterly evil hack. syscalls should not invoke do_signal()
-			 *   as entry.S sets regs->gr8 to the return value of the system call
-			 * - we can't just use sigpending() as we'd have to discard SIG_IGN signals
-			 *   and call waitpid() if SIGCHLD needed discarding
-			 * - this only works on the i386 because it passes arguments to the signal
-			 *   handler on the stack, and the return value in EAX is effectively
-			 *   discarded
-			 */
-			return __frame->gr8;
-	}
+	current->state = TASK_INTERRUPTIBLE;
+	schedule();
+	set_thread_flag(TIF_RESTORE_SIGMASK);
+	return -ERESTARTNOHAND;
 }
 
 asmlinkage int sys_sigaction(int sig,
@@ -372,11 +320,11 @@ static int setup_frame(int sig, struct k
 	       frame->pretcode);
 #endif
 
-	return 1;
+	return 0;
 
 give_sigsegv:
 	force_sig(SIGSEGV, current);
-	return 0;
+	return -EFAULT;
 
 } /* end setup_frame() */
 
@@ -471,11 +419,11 @@ static int setup_rt_frame(int sig, struc
 	       frame->pretcode);
 #endif
 
-	return 1;
+	return 0;
 
 give_sigsegv:
 	force_sig(SIGSEGV, current);
-	return 0;
+	return -EFAULT;
 
 } /* end setup_rt_frame() */
 
@@ -516,7 +464,7 @@ static int handle_signal(unsigned long s
 	else
 		ret = setup_frame(sig, ka, oldset);
 
-	if (ret) {
+	if (ret == 0) {
 		spin_lock_irq(&current->sighand->siglock);
 		sigorsets(&current->blocked, &current->blocked,
 			  &ka->sa.sa_mask);
@@ -536,10 +484,11 @@ static int handle_signal(unsigned long s
  * want to handle. Thus you cannot kill init even with a SIGKILL even by
  * mistake.
  */
-static int do_signal(sigset_t *oldset)
+static void do_signal(void)
 {
 	struct k_sigaction ka;
 	siginfo_t info;
+	sigset_t *oldset;
 	int signr;
 
 	/*
@@ -549,43 +498,62 @@ static int do_signal(sigset_t *oldset)
 	 * if so.
 	 */
 	if (!user_mode(__frame))
-		return 1;
+		return;
 
 	if (try_to_freeze())
 		goto no_signal;
 
-	if (!oldset)
+	if (test_thread_flag(TIF_RESTORE_SIGMASK))
+		oldset = &current->saved_sigmask;
+	else
 		oldset = &current->blocked;
 
 	signr = get_signal_to_deliver(&info, &ka, __frame, NULL);
-	if (signr > 0)
-		return handle_signal(signr, &info, &ka, oldset);
+	if (signr > 0) {
+		if (handle_signal(signr, &info, &ka, oldset) == 0) {
+			/* a signal was successfully delivered; the saved
+			 * sigmask will have been stored in the signal frame,
+			 * and will be restored by sigreturn, so we can simply
+			 * clear the TIF_RESTORE_SIGMASK flag */
+			if (test_thread_flag(TIF_RESTORE_SIGMASK))
+				clear_thread_flag(TIF_RESTORE_SIGMASK);
+		}
+
+		return;
+	}
 
 no_signal:
 	/* Did we come from a system call? */
 	if (__frame->syscallno >= 0) {
 		/* Restart the system call - no handlers present */
-		if (__frame->gr8 == -ERESTARTNOHAND ||
-		    __frame->gr8 == -ERESTARTSYS ||
-		    __frame->gr8 == -ERESTARTNOINTR) {
+		switch (__frame->gr8) {
+		case -ERESTARTNOHAND:
+		case -ERESTARTSYS:
+		case -ERESTARTNOINTR:
 			__frame->gr8 = __frame->orig_gr8;
 			__frame->pc -= 4;
-		}
+			break;
 
-		if (__frame->gr8 == -ERESTART_RESTARTBLOCK){
+		case -ERESTART_RESTARTBLOCK:
 			__frame->gr8 = __NR_restart_syscall;
 			__frame->pc -= 4;
+			break;
 		}
 	}
 
-	return 0;
+	/* if there's no signal to deliver, we just put the saved sigmask
+	 * back */
+	if (test_thread_flag(TIF_RESTORE_SIGMASK)) {
+		clear_thread_flag(TIF_RESTORE_SIGMASK);
+		sigprocmask(SIG_SETMASK, &current->saved_sigmask, NULL);
+	}
 
 } /* end do_signal() */
 
 /*****************************************************************************/
 /*
  * notification of userspace execution resumption
- * - triggered by current->work.notify_resume
+ * - triggered by the TIF_WORK_MASK flags
  */
 asmlinkage void do_notify_resume(__u32 thread_info_flags)
 {
@@ -594,7 +562,7 @@ asmlinkage void do_notify_resume(__u32 t
 		clear_thread_flag(TIF_SINGLESTEP);
 
 	/* deal with pending signal delivery */
-	if (thread_info_flags & _TIF_SIGPENDING)
-		do_signal(NULL);
+	if (thread_info_flags & (_TIF_SIGPENDING | _TIF_RESTORE_SIGMASK))
+		do_signal();
 
 } /* end do_notify_resume() */
diff -uNrp linux-2.6.15-rc2-frv-dosignal/include/asm-frv/thread_info.h linux-2.6.15-rc2-frv-tifrestore/include/asm-frv/thread_info.h
--- linux-2.6.15-rc2-frv-dosignal/include/asm-frv/thread_info.h	2005-11-29 16:41:11.000000000 +0000
+++ linux-2.6.15-rc2-frv-tifrestore/include/asm-frv/thread_info.h	2005-11-29 17:39:05.000000000 +0000
@@ -131,6 +131,7 @@ register struct thread_info *__current_t
 #define TIF_NEED_RESCHED	3	/* rescheduling necessary */
 #define TIF_SINGLESTEP		4	/* restore singlestep on return to user mode */
 #define TIF_IRET		5	/* return with iret */
+#define TIF_RESTORE_SIGMASK	6	/* restore signal mask in do_signal() */
 #define TIF_POLLING_NRFLAG	16	/* true if poll_idle() is polling TIF_NEED_RESCHED */
 #define TIF_MEMDIE		17	/* OOM killer killed process */
 
@@ -140,6 +141,7 @@ register struct thread_info *__current_t
 #define _TIF_NEED_RESCHED	(1 << TIF_NEED_RESCHED)
 #define _TIF_SINGLESTEP		(1 << TIF_SINGLESTEP)
 #define _TIF_IRET		(1 << TIF_IRET)
+#define _TIF_RESTORE_SIGMASK	(1 << TIF_RESTORE_SIGMASK)
 #define _TIF_POLLING_NRFLAG	(1 << TIF_POLLING_NRFLAG)
 
 #define _TIF_WORK_MASK		0x0000FFFE	/* work to do on interrupt/exception return */
diff -uNrp linux-2.6.15-rc2-frv-dosignal/include/asm-frv/unistd.h linux-2.6.15-rc2-frv-tifrestore/include/asm-frv/unistd.h
--- linux-2.6.15-rc2-frv-dosignal/include/asm-frv/unistd.h	2005-06-22 13:52:26.000000000 +0100
+++ linux-2.6.15-rc2-frv-tifrestore/include/asm-frv/unistd.h	2005-11-29 18:02:04.000000000 +0000
@@ -486,6 +486,7 @@ static inline pid_t wait(int * wait_stat
 /* #define __ARCH_WANT_SYS_SIGPENDING */
 #define __ARCH_WANT_SYS_SIGPROCMASK
 #define __ARCH_WANT_SYS_RT_SIGACTION
+#define __ARCH_WANT_SYS_RT_SIGSUSPEND
 #endif
 
 /*

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2005-11-29 21:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-11-29 21:18 [PATCH 1/2] FRV: Fix FRV signal handling David Howells
2005-11-29 21:18 ` [PATCH 2/2] Handle TIF_RESTORE_SIGMASK for i386 David Howells
2005-11-29 21:18 ` [PATCH 2/2] FRV: Improve signal handling David Howells
2005-11-29 21:18 ` [PATCH 1/2] Handle TIF_RESTORE_SIGMASK for FRV David Howells

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox