[PATCH 00/41] signal: Use set_current_blocked()

[PATCH 00/41] signal: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

Hi Oleg,

I did a tree wide scan of all the code that modifies current->blocked
without using the set_current_blocked() accessor. This patch series is
the result of the conversion to the new accessor functions.

This series also helps to reduce the complexity of my signal
scalability patches that I posted in April, because we now have much
fewer callers of recalc_sigpending() (the signal scalability patches
introduce a new lock that needs to be held when calling
recalc_sigpending()).

This series is available in the 'oleg/set-current-blocked' branch at,

    git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/linux-2.6.git

P.S I've punted on fully converting ia64 to set_current_blocked()
because bits of the signal code are written in asm. I'll follow up
with a later patch that does the conversion once I've improved my ia64
foo (or coerced Tony Luck into doing it ;).

Matt Fleming (40):
  alpha: Use set_current_blocked()
  arm: Use set_current_blocked()
  avr32: Don't mask signals in the error path
  blackfin: Use set_current_blocked()
  cris: Use set_current_blocked()
  frv: Use set_current_blocked()
  h8300: Use set_current_blocked()
  ia64: Use set_current_blocked()
  m32r: Use set_current_blocked()
  m68k: Use set_current_blocked()
  microblaze: Don't reimplement force_sigsegv()
  microblaze: No need to reset handler if SA_ONESHOT
  microblaze: Fix signal masking
  microblaze: Use set_current_blocked()
  MIPS: Use set_current_blocked()
  mn10300: Use set_current_blocked()
  OpenRISC: Don't reimplement force_sigsegv()
  OpenRISC: No need to reset handler if SA_ONESHOT
  OpenRISC: Don't mask signals if we fail to setup signal stack
  OpenRISC: Use set_current_blocked()
  parisc: Use set_current_blocked()
  powerpc: Use set_current_blocked()
  score: Don't mask signals if we fail to setup signal stack
  score: Use set_current_blocked()
  sh: No need to reset handler if SA_ONESHOT
  sh: Use set_current_blocked()
  sparc: Use set_current_blocked()
  tile: Use set_current_blocked()
  um: Use set_current_blocked()
  um: Don't restore current->blocked on error
  unicore32: Use set_current_blocked()
  xtensa: Don't reimplement force_sigsegv()
  xtensa: No need to reset handler if SA_ONESHOT
  xtensa: Don't mask signals if we fail to setup signal stack
  xtensa: Use set_current_blocked()
  autofs4: Use set_current_blocked()
  coda: Use set_current_blocked()
  dlm: Remove another superfluous call to recalc_sigpending()
  ncpfs: Use set_current_blocked()
  exit: Use __set_task_blocked()

Oleg Nesterov (1):
  avr32: use set_current_blocked() in handle_signal/sys_rt_sigreturn

 arch/alpha/kernel/signal.c         |   31 ++++++++++-------------
 arch/arm/kernel/signal.c           |   25 ++++++++----------
 arch/avr32/kernel/signal.c         |   30 +++++++++-------------
 arch/blackfin/kernel/signal.c      |   16 ++++-------
 arch/cris/arch-v10/kernel/signal.c |   33 ++++++++++--------------
 arch/cris/arch-v32/kernel/signal.c |   37 ++++++++++-----------------
 arch/frv/kernel/signal.c           |   32 ++++++++++--------------
 arch/h8300/kernel/signal.c         |   36 ++++++++++-----------------
 arch/ia64/kernel/signal.c          |   18 ++++---------
 arch/m32r/kernel/signal.c          |   16 +++++-------
 arch/m68k/kernel/signal_mm.c       |   24 +++++++++---------
 arch/m68k/kernel/signal_no.c       |   30 +++++++++-------------
 arch/microblaze/kernel/signal.c    |   46 ++++++++++++++++-------------------
 arch/mips/kernel/signal.c          |   29 ++++++----------------
 arch/mips/kernel/signal32.c        |   20 +++------------
 arch/mips/kernel/signal_n32.c      |   10 +------
 arch/mn10300/kernel/signal.c       |   32 ++++++++++--------------
 arch/openrisc/kernel/signal.c      |   47 ++++++++++++++++-------------------
 arch/parisc/kernel/signal.c        |   15 ++++-------
 arch/powerpc/kernel/signal.c       |   16 ++++-------
 arch/powerpc/kernel/signal_32.c    |   11 ++++----
 arch/score/kernel/signal.c         |   19 +++++++-------
 arch/sh/kernel/signal_32.c         |   35 ++++++++++----------------
 arch/sh/kernel/signal_64.c         |   40 ++++++++++--------------------
 arch/sparc/kernel/signal32.c       |   19 ++++----------
 arch/sparc/kernel/signal_32.c      |   30 +++++++++-------------
 arch/sparc/kernel/signal_64.c      |   30 +++++++++-------------
 arch/tile/kernel/compat_signal.c   |    5 +---
 arch/tile/kernel/signal.c          |   16 ++++-------
 arch/um/kernel/signal.c            |   30 ++++++++++-------------
 arch/um/sys-i386/signal.c          |   12 +-------
 arch/um/sys-x86_64/signal.c        |    6 +----
 arch/unicore32/kernel/signal.c     |   15 ++++-------
 arch/xtensa/kernel/signal.c        |   37 ++++++++++-----------------
 fs/autofs4/waitq.c                 |   15 ++++-------
 fs/coda/upcall.c                   |   19 ++++++--------
 fs/dlm/user.c                      |    1 -
 fs/ncpfs/sock.c                    |   12 ++++-----
 kernel/exit.c                      |    7 ++++-
 39 files changed, 351 insertions(+), 551 deletions(-)

-- 
1.7.4.4

[PATCH 01/41] alpha: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@linux.intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check for shared signals we're about to block.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru>
Cc: Matt Turner <mattst88@gmail.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Matt Fleming <matt.fleming@linux.intel.com>
---
 arch/alpha/kernel/signal.c |   31 +++++++++++++------------------
 1 files changed, 13 insertions(+), 18 deletions(-)

diff --git a/arch/alpha/kernel/signal.c b/arch/alpha/kernel/signal.c
index 6f7feb5..ec6fe13 100644
--- a/arch/alpha/kernel/signal.c
+++ b/arch/alpha/kernel/signal.c
@@ -120,12 +120,13 @@ SYSCALL_DEFINE5(rt_sigaction, int, sig, const struct sigaction __user *, act,
  */
 SYSCALL_DEFINE1(sigsuspend, old_sigset_t, mask)
 {
-	mask &= _BLOCKABLE;
-	spin_lock_irq(&current->sighand->siglock);
+	sigset_t blocked;
+
 	current->saved_sigmask = current->blocked;
-	siginitset(&current->blocked, mask);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+
+	mask &= _BLOCKABLE;
+	siginitset(&blocked, mask);
+	set_current_blocked(&blocked);
 
 	current->state = TASK_INTERRUPTIBLE;
 	schedule();
@@ -238,10 +239,7 @@ do_sigreturn(struct sigcontext __user *sc, struct pt_regs *regs,
 		goto give_sigsegv;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(sc, regs, sw))
 		goto give_sigsegv;
@@ -276,10 +274,7 @@ do_rt_sigreturn(struct rt_sigframe __user *frame, struct pt_regs *regs,
 		goto give_sigsegv;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_block(&set);
 
 	if (restore_sigcontext(&frame->uc.uc_mcontext, regs, sw))
 		goto give_sigsegv;
@@ -502,12 +497,12 @@ handle_signal(int sig, struct k_sigaction *ka, siginfo_t *info,
 		ret = setup_frame(sig, ka, oldset, regs, sw);
 
 	if (ret == 0) {
-		spin_lock_irq(&current->sighand->siglock);
-		sigorsets(&current->blocked,&current->blocked,&ka->sa.sa_mask);
+		sigset_t blocked;
+
+		sigorsets(&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);
+			sigaddset(&blocked, sig);
+		set_current_blocked(&blocked);
 	}
 
 	return ret;
-- 
1.7.4.4

[PATCH 02/41] arm: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@linux.intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check for shared signals we're about to block.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Arnd Bergmann <arnd.bergmann@linaro.org>
Cc: Dave Martin <dave.martin@linaro.org>
Cc: Nicolas Pitre <nicolas.pitre@linaro.org>
CC: Will Deacon <will.deacon@arm.com>
Signed-off-by: Matt Fleming <matt.fleming@linux.intel.com>
---
 arch/arm/kernel/signal.c |   25 +++++++++++--------------
 1 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c
index 0340224..b97668a 100644
--- a/arch/arm/kernel/signal.c
+++ b/arch/arm/kernel/signal.c
@@ -66,12 +66,13 @@ const unsigned long syscall_restart_code[2] = {
  */
 asmlinkage int sys_sigsuspend(int restart, unsigned long oldmask, old_sigset_t mask)
 {
-	mask &= _BLOCKABLE;
-	spin_lock_irq(&current->sighand->siglock);
+	sigset_t blocked;
+
 	current->saved_sigmask = current->blocked;
-	siginitset(&current->blocked, mask);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+
+	mask &= _BLOCKABLE;
+	siginitset(&blocked, mask);
+	set_current_block(&blocked);
 
 	current->state = TASK_INTERRUPTIBLE;
 	schedule();
@@ -281,10 +282,7 @@ static int restore_sigframe(struct pt_regs *regs, struct sigframe __user *sf)
 	err = __copy_from_user(&set, &sf->uc.uc_sigmask, sizeof(set));
 	if (err == 0) {
 		sigdelsetmask(&set, ~_BLOCKABLE);
-		spin_lock_irq(&current->sighand->siglock);
-		current->blocked = set;
-		recalc_sigpending();
-		spin_unlock_irq(&current->sighand->siglock);
+		set_current_blocked(&set);
 	}
 
 	__get_user_error(regs->ARM_r0, &sf->uc.uc_mcontext.arm_r0, err);
@@ -607,6 +605,7 @@ handle_signal(unsigned long sig, struct k_sigaction *ka,
 {
 	struct thread_info *thread = current_thread_info();
 	struct task_struct *tsk = current;
+	sigset_t blocked;
 	int usig = sig;
 	int ret;
 
@@ -637,13 +636,11 @@ handle_signal(unsigned long sig, struct k_sigaction *ka,
 	/*
 	 * Block the signal if we were successful.
 	 */
-	spin_lock_irq(&tsk->sighand->siglock);
-	sigorsets(&tsk->blocked, &tsk->blocked,
+	sigorsets(&blocked, &tsk->blocked,
 		  &ka->sa.sa_mask);
 	if (!(ka->sa.sa_flags & SA_NODEFER))
-		sigaddset(&tsk->blocked, sig);
-	recalc_sigpending();
-	spin_unlock_irq(&tsk->sighand->siglock);
+		sigaddset(&blocked, sig);
+	set_current_blocked(&blocked);
 
 	return 0;
 }
-- 
1.7.4.4

[PATCH 03/41] avr32: Don't mask signals in the error path

[Matt Fleming]

From: Matt Fleming <matt.fleming@linux.intel.com>

The current handle_signal() implementation is broken - it will mask
signals if we fail to setup the signal stack frame, which isn't the
desired behaviour, we should only be masking signals if we succeed in
setting up the stack frame. It looks like this code was copied from
the old (broken) arm implementation but wasn't updated when the arm
code was fixed in commit a6c61e9dfdd0 ("[ARM] 3168/1: Update ARM
signal delivery and masking").

Cc: Haavard Skinnemoen <hskinnemoen@gmail.com>
Cc: Hans-Christian Egtvedt <egtvedt@samfundet.no>
Signed-off-by: Matt Fleming <matt.fleming@linux.intel.com>
---
 arch/avr32/kernel/signal.c |   25 ++++++++++++-------------
 1 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/arch/avr32/kernel/signal.c b/arch/avr32/kernel/signal.c
index 64f886f..9c075e1 100644
--- a/arch/avr32/kernel/signal.c
+++ b/arch/avr32/kernel/signal.c
@@ -238,22 +238,21 @@ handle_signal(unsigned long sig, struct k_sigaction *ka, siginfo_t *info,
 	 */
 	ret |= !valid_user_regs(regs);
 
+	if (ret != 0) {
+		force_sigsegv(sig, current);
+		return;
+	}
+
 	/*
-	 * Block the signal if we were unsuccessful.
+	 * Block the signal if we were successful.
 	 */
-	if (ret != 0 || !(ka->sa.sa_flags & SA_NODEFER)) {
-		spin_lock_irq(&current->sighand->siglock);
-		sigorsets(&current->blocked, &current->blocked,
-			  &ka->sa.sa_mask);
+	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);
-	}
-
-	if (ret == 0)
-		return;
-
-	force_sigsegv(sig, current);
+	recalc_sigpending();
+	spin_unlock_irq(&current->sighand->siglock);
 }
 
 /*
-- 
1.7.4.4

[PATCH 04/41] avr32: use set_current_blocked() in handle_signal/sys_rt_sigreturn

[Matt Fleming]

From: Oleg Nesterov <oleg@redhat.com>

It is wrong to change ->blocked directly, see e6fa16ab.
Change  handle_signal() and sys_rt_sigreturn() to use
the right helper, set_current_blocked().

Cc: Haavard Skinnemoen <hskinnemoen@gmail.com>
Cc: Hans-Christian Egtvedt <egtvedt@samfundet.no>
Reviewed-by: Matt Fleming <matt.fleming@intel.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 arch/avr32/kernel/signal.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/arch/avr32/kernel/signal.c b/arch/avr32/kernel/signal.c
index 9c075e1..06f4293 100644
--- a/arch/avr32/kernel/signal.c
+++ b/arch/avr32/kernel/signal.c
@@ -87,10 +87,7 @@ asmlinkage int sys_rt_sigreturn(struct pt_regs *regs)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(regs, &frame->uc.uc_mcontext))
 		goto badframe;
@@ -226,6 +223,7 @@ static inline void
 handle_signal(unsigned long sig, struct k_sigaction *ka, siginfo_t *info,
 	      sigset_t *oldset, struct pt_regs *regs, int syscall)
 {
+	sigset_t blocked;
 	int ret;
 
 	/*
@@ -246,13 +244,10 @@ handle_signal(unsigned long sig, struct k_sigaction *ka, siginfo_t *info,
 	/*
 	 * Block the signal if we were successful.
 	 */
-	spin_lock_irq(&current->sighand->siglock);
-	sigorsets(&current->blocked, &current->blocked,
-		  &ka->sa.sa_mask);
+	sigorsets(&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);
+		sigaddset(&blocked, sig);
+	set_current_blocked(&blocked);
 }
 
 /*
-- 
1.7.4.4

[PATCH 05/41] blackfin: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@linux.intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Matt Fleming <matt.fleming@linux.intel.com>
---
 arch/blackfin/kernel/signal.c |   16 ++++++----------
 1 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/arch/blackfin/kernel/signal.c b/arch/blackfin/kernel/signal.c
index d536f35..96b8666 100644
--- a/arch/blackfin/kernel/signal.c
+++ b/arch/blackfin/kernel/signal.c
@@ -99,10 +99,7 @@ asmlinkage int do_rt_sigreturn(unsigned long __unused)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_block(&set);
 
 	if (rt_restore_sigcontext(regs, &frame->uc.uc_mcontext, &r0))
 		goto badframe;
@@ -267,13 +264,12 @@ handle_signal(int sig, siginfo_t *info, struct k_sigaction *ka,
 	ret = setup_rt_frame(sig, ka, info, oldset, regs);
 
 	if (ret == 0) {
-		spin_lock_irq(&current->sighand->siglock);
-		sigorsets(&current->blocked, &current->blocked,
-			  &ka->sa.sa_mask);
+		sigset_t blocked;
+
+		sigorsets(&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);
+			sigaddset(&blocked, sig);
+		set_current_blocked(&blocked);
 	}
 	return ret;
 }
-- 
1.7.4.4

[PATCH 06/41] cris: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@linux.intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Mikael Starvik <starvik@axis.com>
Cc: Jesper Nilsson <jesper.nilsson@axis.com>
Signed-off-by: Matt Fleming <matt.fleming@linux.intel.com>
---
 arch/cris/arch-v10/kernel/signal.c |   33 +++++++++++++------------------
 arch/cris/arch-v32/kernel/signal.c |   37 +++++++++++++----------------------
 2 files changed, 28 insertions(+), 42 deletions(-)

diff --git a/arch/cris/arch-v10/kernel/signal.c b/arch/cris/arch-v10/kernel/signal.c
index e78fe49..9f8c29c 100644
--- a/arch/cris/arch-v10/kernel/signal.c
+++ b/arch/cris/arch-v10/kernel/signal.c
@@ -50,12 +50,14 @@ void do_signal(int canrestart, struct pt_regs *regs);
 int sys_sigsuspend(old_sigset_t mask, long r11, long r12, long r13, long mof,
 	long srp, struct pt_regs *regs)
 {
-	mask &= _BLOCKABLE;
-	spin_lock_irq(&current->sighand->siglock);
+	sigset_t blocked;
+
 	current->saved_sigmask = current->blocked;
-	siginitset(&current->blocked, mask);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+
+	mask &= _BLOCKABLE;
+	siginitset(&blocked, mask);
+	set_current_blocked(&blocked);
+
 	current->state = TASK_INTERRUPTIBLE;
 	schedule();
 	set_thread_flag(TIF_RESTORE_SIGMASK);
@@ -184,10 +186,7 @@ asmlinkage int sys_sigreturn(long r10, long r11, long r12, long r13, long mof,
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(regs, &frame->sc))
 		goto badframe;
@@ -223,10 +222,7 @@ asmlinkage int sys_rt_sigreturn(long r10, long r11, long r12, long r13,
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(regs, &frame->uc.uc_mcontext))
 		goto badframe;
@@ -469,13 +465,12 @@ static inline int handle_signal(int canrestart, unsigned long sig,
 		ret = setup_frame(sig, ka, oldset, regs);
 
 	if (ret == 0) {
-		spin_lock_irq(&current->sighand->siglock);
-		sigorsets(&current->blocked, &current->blocked,
-			&ka->sa.sa_mask);
+		sigset_t blocked;
+
+		sigorsets(&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);
+			sigaddset(&blocked, sig);
+		set_current_blocked(&blocked);
 	}
 	return ret;
 }
diff --git a/arch/cris/arch-v32/kernel/signal.c b/arch/cris/arch-v32/kernel/signal.c
index ce4ab1a..e493656 100644
--- a/arch/cris/arch-v32/kernel/signal.c
+++ b/arch/cris/arch-v32/kernel/signal.c
@@ -62,12 +62,14 @@ int
 sys_sigsuspend(old_sigset_t mask, long r11, long r12, long r13, long mof,
 	       long srp, struct pt_regs *regs)
 {
-	mask &= _BLOCKABLE;
-	spin_lock_irq(&current->sighand->siglock);
+	sigset_t blocked;
+
 	current->saved_sigmask = current->blocked;
-	siginitset(&current->blocked, mask);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+
+	mask &= _BLOCKABLE;
+	siginitset(&blocked, mask);
+	set_current_blocked(&blocked);
+
 	current->state = TASK_INTERRUPTIBLE;
 	schedule();
 	set_thread_flag(TIF_RESTORE_SIGMASK);
@@ -176,12 +178,7 @@ sys_sigreturn(long r10, long r11, long r12, long r13, long mof, long srp,
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-
-	current->blocked = set;
-
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(regs, &frame->sc))
 		goto badframe;
@@ -222,12 +219,7 @@ sys_rt_sigreturn(long r10, long r11, long r12, long r13, long mof, long srp,
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-
-	current->blocked = set;
-
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(regs, &frame->uc.uc_mcontext))
 		goto badframe;
@@ -516,13 +508,12 @@ handle_signal(int canrestart, unsigned long sig,
 		ka->sa.sa_handler = SIG_DFL;
 
 	if (ret == 0) {
-		spin_lock_irq(&current->sighand->siglock);
-		sigorsets(&current->blocked, &current->blocked,
-			&ka->sa.sa_mask);
+		sigset_t blocked;
+
+		sigorsets(&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);
+			sigaddset(&blocked, sig);
+		set_current_blocked(&blocked);
 	}
 
 	return ret;
-- 
1.7.4.4

[PATCH 07/41] frv: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/frv/kernel/signal.c |   32 +++++++++++++-------------------
 1 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/arch/frv/kernel/signal.c b/arch/frv/kernel/signal.c
index bab0129..7f624e4 100644
--- a/arch/frv/kernel/signal.c
+++ b/arch/frv/kernel/signal.c
@@ -40,12 +40,13 @@ struct fdpic_func_descriptor {
  */
 asmlinkage int sys_sigsuspend(int history0, int history1, old_sigset_t mask)
 {
-	mask &= _BLOCKABLE;
-	spin_lock_irq(&current->sighand->siglock);
+	sigset_t blocked;
+
 	current->saved_sigmask = current->blocked;
-	siginitset(&current->blocked, mask);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+
+	mask &= _BLOCKABLE;
+	siginitset(&blocked, mask);
+	set_current_blocked(&blocked);
 
 	current->state = TASK_INTERRUPTIBLE;
 	schedule();
@@ -158,10 +159,7 @@ asmlinkage int sys_sigreturn(void)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(&frame->sc, &gr8))
 		goto badframe;
@@ -184,10 +182,7 @@ asmlinkage int sys_rt_sigreturn(void)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(&frame->uc.uc_mcontext, &gr8))
 		goto badframe;
@@ -475,13 +470,12 @@ static int handle_signal(unsigned long sig, siginfo_t *info,
 		ret = setup_frame(sig, ka, oldset);
 
 	if (ret == 0) {
-		spin_lock_irq(&current->sighand->siglock);
-		sigorsets(&current->blocked, &current->blocked,
-			  &ka->sa.sa_mask);
+		sigset_t blocked;
+
+		sigorsets(&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);
+			sigaddset(&blocked, sig);
+		set_current_blocked(&blocked);
 	}
 
 	return ret;
-- 
1.7.4.4

[PATCH 08/41] h8300: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/h8300/kernel/signal.c |   36 +++++++++++++-----------------------
 1 files changed, 13 insertions(+), 23 deletions(-)

diff --git a/arch/h8300/kernel/signal.c b/arch/h8300/kernel/signal.c
index af842c3..4c06f98 100644
--- a/arch/h8300/kernel/signal.c
+++ b/arch/h8300/kernel/signal.c
@@ -57,14 +57,13 @@ asmlinkage int do_signal(struct pt_regs *regs, sigset_t *oldset);
 asmlinkage int do_sigsuspend(struct pt_regs *regs)
 {
 	old_sigset_t mask = regs->er3;
-	sigset_t saveset;
+	sigset_t saveset, blocked;
 
-	mask &= _BLOCKABLE;
-	spin_lock_irq(&current->sighand->siglock);
 	saveset = current->blocked;
-	siginitset(&current->blocked, mask);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+
+	mask &= _BLOCKABLE;
+	siginitset(&blocked, mask);
+	set_current_blocked(&blocked);
 
 	regs->er0 = -EINTR;
 	while (1) {
@@ -90,11 +89,8 @@ do_rt_sigsuspend(struct pt_regs *regs)
 		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);
+	set_current_blocked(&newset);
 
 	regs->er0 = -EINTR;
 	while (1) {
@@ -232,10 +228,7 @@ asmlinkage int do_sigreturn(unsigned long __unused,...)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 	
 	if (restore_sigcontext(regs, &frame->sc, &er0))
 		goto badframe;
@@ -260,10 +253,7 @@ asmlinkage int do_rt_sigreturn(unsigned long __unused,...)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_unlock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_lock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 	
 	if (restore_sigcontext(regs, &frame->uc.uc_mcontext, &er0))
 		goto badframe;
@@ -463,6 +453,8 @@ static void
 handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka,
 	      sigset_t *oldset,	struct pt_regs * regs)
 {
+	sigset_t blocked;
+
 	/* are we from a system call? */
 	if (regs->orig_er0 >= 0) {
 		switch (regs->er0) {
@@ -489,12 +481,10 @@ handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka,
 	else
 		setup_frame(sig, ka, oldset, regs);
 
-	spin_lock_irq(&current->sighand->siglock);
-	sigorsets(&current->blocked,&current->blocked,&ka->sa.sa_mask);
+	sigorsets(&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);
+		sigaddset(&blocked, sig);
+	set_current_blocked(&blocked);
 }
 
 /*
-- 
1.7.4.4

[PATCH 09/41] ia64: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/ia64/kernel/signal.c |   18 ++++++------------
 1 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/arch/ia64/kernel/signal.c b/arch/ia64/kernel/signal.c
index 7bdafc8..1261b2f 100644
--- a/arch/ia64/kernel/signal.c
+++ b/arch/ia64/kernel/signal.c
@@ -201,13 +201,7 @@ ia64_rt_sigreturn (struct sigscratch *scr)
 		goto give_sigsegv;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-
-	spin_lock_irq(&current->sighand->siglock);
-	{
-		current->blocked = set;
-		recalc_sigpending();
-	}
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(sc, scr))
 		goto give_sigsegv;
@@ -424,15 +418,15 @@ static long
 handle_signal (unsigned long sig, struct k_sigaction *ka, siginfo_t *info, sigset_t *oldset,
 	       struct sigscratch *scr)
 {
+	sigset_t blocked;
+
 	if (!setup_frame(sig, ka, info, oldset, scr))
 		return 0;
 
-	spin_lock_irq(&current->sighand->siglock);
-	sigorsets(&current->blocked, &current->blocked, &ka->sa.sa_mask);
+	sigorsets(&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);
+		sigaddset(&blocked, sig);
+	set_current_blocked(&blocked);
 
 	/*
 	 * Let tracing know that we've done the handler setup.
-- 
1.7.4.4

[PATCH 10/41] m32r: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Hirokazu Takata <takata@linux-m32r.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Kyle McMartin <kyle@redhat.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/m32r/kernel/signal.c |   16 +++++++---------
 1 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/arch/m32r/kernel/signal.c b/arch/m32r/kernel/signal.c
index a08697f..98f016a 100644
--- a/arch/m32r/kernel/signal.c
+++ b/arch/m32r/kernel/signal.c
@@ -112,10 +112,7 @@ sys_rt_sigreturn(unsigned long r0, unsigned long r1,
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(regs, &frame->uc.uc_mcontext, &result))
 		goto badframe;
@@ -274,6 +271,8 @@ static int
 handle_signal(unsigned long sig, struct k_sigaction *ka, siginfo_t *info,
 	      sigset_t *oldset, struct pt_regs *regs)
 {
+	sigset_t blocked;
+
 	/* Are we from a system call? */
 	if (regs->syscall_nr >= 0) {
 		/* If so, check system call restarting.. */
@@ -300,12 +299,11 @@ handle_signal(unsigned long sig, struct k_sigaction *ka, siginfo_t *info,
 	if (setup_rt_frame(sig, ka, info, oldset, regs))
 		return -EFAULT;
 
-	spin_lock_irq(&current->sighand->siglock);
-	sigorsets(&current->blocked,&current->blocked,&ka->sa.sa_mask);
+	sigorsets(&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);
+		sigaddset(&blocked, sig);
+	set_current_blocked(&blocked);
+
 	return 0;
 }
 
-- 
1.7.4.4

[PATCH 11/41] m68k: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Greg Ungerer <gerg@uclinux.org>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/m68k/kernel/signal_mm.c |   24 ++++++++++++------------
 arch/m68k/kernel/signal_no.c |   30 ++++++++++++------------------
 2 files changed, 24 insertions(+), 30 deletions(-)

diff --git a/arch/m68k/kernel/signal_mm.c b/arch/m68k/kernel/signal_mm.c
index a0afc23..eeeaf3c 100644
--- a/arch/m68k/kernel/signal_mm.c
+++ b/arch/m68k/kernel/signal_mm.c
@@ -97,12 +97,13 @@ int handle_kernel_fault(struct pt_regs *regs)
 asmlinkage int
 sys_sigsuspend(int unused0, int unused1, old_sigset_t mask)
 {
-	mask &= _BLOCKABLE;
-	spin_lock_irq(&current->sighand->siglock);
+	sigset_t blocked;
+
 	current->saved_sigmask = current->blocked;
-	siginitset(&current->blocked, mask);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+
+	mask &= _BLOCKABLE;
+	siginitset(&blocked, mask);
+	set_current_blocked(&blocked);
 
 	current->state = TASK_INTERRUPTIBLE;
 	schedule();
@@ -465,8 +466,7 @@ asmlinkage int do_sigreturn(unsigned long __unused)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	current->blocked = set;
-	recalc_sigpending();
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(regs, &frame->sc, frame + 1))
 		goto badframe;
@@ -491,8 +491,7 @@ asmlinkage int do_rt_sigreturn(unsigned long __unused)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	current->blocked = set;
-	recalc_sigpending();
+	set_current_blocked(&set);
 
 	if (rt_restore_ucontext(regs, sw, &frame->uc))
 		goto badframe;
@@ -950,6 +949,7 @@ static void
 handle_signal(int sig, struct k_sigaction *ka, siginfo_t *info,
 	      sigset_t *oldset, struct pt_regs *regs)
 {
+	sigset_t blocked;
 	int err;
 	/* are we from a system call? */
 	if (regs->orig_d0 >= 0)
@@ -965,10 +965,10 @@ handle_signal(int sig, struct k_sigaction *ka, siginfo_t *info,
 	if (err)
 		return;
 
-	sigorsets(&current->blocked,&current->blocked,&ka->sa.sa_mask);
+	sigorsets(&blocked, &current->blocked, &ka->sa.sa_mask);
 	if (!(ka->sa.sa_flags & SA_NODEFER))
-		sigaddset(&current->blocked,sig);
-	recalc_sigpending();
+		sigaddset(&blocked, sig);
+	set_current_blocked(&blocked);
 
 	if (test_thread_flag(TIF_DELAYED_TRACE)) {
 		regs->sr &= ~0x8000;
diff --git a/arch/m68k/kernel/signal_no.c b/arch/m68k/kernel/signal_no.c
index 36a81bb..97f3c39 100644
--- a/arch/m68k/kernel/signal_no.c
+++ b/arch/m68k/kernel/signal_no.c
@@ -60,12 +60,13 @@ void ret_from_user_rt_signal(void);
 asmlinkage int
 sys_sigsuspend(int unused0, int unused1, old_sigset_t mask)
 {
-	mask &= _BLOCKABLE;
-	spin_lock_irq(&current->sighand->siglock);
+	sigset_t blocked;
+
 	current->saved_sigmask = current->blocked;
-	siginitset(&current->blocked, mask);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+
+	mask &= _BLOCKABLE;
+	siginitset(&blocked, mask);
+	set_current_blocked(&blocked);
 
 	current->state = TASK_INTERRUPTIBLE;
 	schedule();
@@ -343,10 +344,7 @@ asmlinkage int do_sigreturn(unsigned long __unused)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 	
 	if (restore_sigcontext(regs, &frame->sc, frame + 1, &d0))
 		goto badframe;
@@ -372,10 +370,7 @@ asmlinkage int do_rt_sigreturn(unsigned long __unused)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 	
 	if (rt_restore_ucontext(regs, sw, &frame->uc, &d0))
 		goto badframe;
@@ -693,6 +688,7 @@ static void
 handle_signal(int sig, struct k_sigaction *ka, siginfo_t *info,
 	      sigset_t *oldset, struct pt_regs *regs)
 {
+	sigset_t blocked;
 	int err;
 	/* are we from a system call? */
 	if (regs->orig_d0 >= 0)
@@ -708,12 +704,10 @@ handle_signal(int sig, struct k_sigaction *ka, siginfo_t *info,
 	if (err)
 		return;
 
-	spin_lock_irq(&current->sighand->siglock);
-	sigorsets(&current->blocked,&current->blocked,&ka->sa.sa_mask);
+	sigorsets(&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);
+		sigaddset(&blocked, sig);
+	set_current_blocked(&blocked);
 
 	clear_thread_flag(TIF_RESTORE_SIGMASK);
 }
-- 
1.7.4.4

[PATCH 12/41] microblaze: Don't reimplement force_sigsegv()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

Instead of open coding the sequence from force_sigsegv() just call
it. This also fixes a race because sa_handler was being modified
without holding ->sighand->siglock.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Michal Simek <monstr@monstr.eu>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/microblaze/kernel/signal.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/arch/microblaze/kernel/signal.c b/arch/microblaze/kernel/signal.c
index 5996711..90de06d 100644
--- a/arch/microblaze/kernel/signal.c
+++ b/arch/microblaze/kernel/signal.c
@@ -270,9 +270,7 @@ static void setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
 	return;
 
 give_sigsegv:
-	if (sig == SIGSEGV)
-		ka->sa.sa_handler = SIG_DFL;
-	force_sig(SIGSEGV, current);
+	force_sigsegv(sig, current);
 }
 
 /* Handle restarting system calls */
-- 
1.7.4.4

[PATCH 13/41] microblaze: No need to reset handler if SA_ONESHOT

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

get_signal_to_deliver() already resets the signal handler if
SA_ONESHOT is set in ka->sa.sa_flags, there's no need to do it again
in handle_signal(). Furthermore, because we were modifying
ka->sa.sa_handler without holding ->sighand->siglock there's the
potential we can race with another thread if it modifies the signal
handlers.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Michal Simek <monstr@monstr.eu>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/microblaze/kernel/signal.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/arch/microblaze/kernel/signal.c b/arch/microblaze/kernel/signal.c
index 90de06d..9e749c0 100644
--- a/arch/microblaze/kernel/signal.c
+++ b/arch/microblaze/kernel/signal.c
@@ -320,9 +320,6 @@ handle_signal(unsigned long sig, struct k_sigaction *ka,
 	else
 		setup_rt_frame(sig, ka, NULL, oldset, regs);
 
-	if (ka->sa.sa_flags & SA_ONESHOT)
-		ka->sa.sa_handler = SIG_DFL;
-
 	if (!(ka->sa.sa_flags & SA_NODEFER)) {
 		spin_lock_irq(&current->sighand->siglock);
 		sigorsets(&current->blocked,
-- 
1.7.4.4

[PATCH 14/41] microblaze: Fix signal masking

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

There are a couple of problems with the current signal code,

1. If we failed to setup the signal stack frame then we
should not be masking any signals.

2. ka->sa.sa_mask is only added to the current blocked signals list if
SA_NODEFER is set in ka->sa.sa_flags. If we successfully setup the
signal frame and are going to run the handler then we must honour
sa_mask.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Michal Simek <monstr@monstr.eu>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/microblaze/kernel/signal.c |   31 ++++++++++++++++++-------------
 1 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/arch/microblaze/kernel/signal.c b/arch/microblaze/kernel/signal.c
index 9e749c0..f2c13d5 100644
--- a/arch/microblaze/kernel/signal.c
+++ b/arch/microblaze/kernel/signal.c
@@ -169,7 +169,7 @@ get_sigframe(struct k_sigaction *ka, struct pt_regs *regs, size_t frame_size)
 	return (void __user *)((sp - frame_size) & -8UL);
 }
 
-static void setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
+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;
@@ -267,10 +267,11 @@ static void setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
 		current->comm, current->pid, frame, regs->pc);
 #endif
 
-	return;
+	return 0;
 
 give_sigsegv:
 	force_sigsegv(sig, current);
+	return -EFAULT;
 }
 
 /* Handle restarting system calls */
@@ -314,21 +315,25 @@ static int
 handle_signal(unsigned long sig, struct k_sigaction *ka,
 		siginfo_t *info, sigset_t *oldset, struct pt_regs *regs)
 {
+	int ret;
+
 	/* 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_rt_frame(sig, ka, NULL, oldset, regs);
+		ret = setup_rt_frame(sig, ka, NULL, oldset, regs);
+
+	if (ret)
+		return ret;
 
-	if (!(ka->sa.sa_flags & SA_NODEFER)) {
-		spin_lock_irq(&current->sighand->siglock);
-		sigorsets(&current->blocked,
-				&current->blocked, &ka->sa.sa_mask);
+	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 1;
+	recalc_sigpending();
+	spin_unlock_irq(&current->sighand->siglock);
+
+	return 0;
 }
 
 /*
@@ -369,7 +374,7 @@ int do_signal(struct pt_regs *regs, sigset_t *oldset, int in_syscall)
 		/* Whee! Actually deliver the signal. */
 		if (in_syscall)
 			handle_restart(regs, &ka, 1);
-		if (handle_signal(signr, &ka, &info, oldset, regs)) {
+		if (!handle_signal(signr, &ka, &info, oldset, regs)) {
 			/*
 			 * A signal was successfully delivered; the saved
 			 * sigmask will have been stored in the signal frame,
-- 
1.7.4.4

[PATCH 15/41] microblaze: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Michal Simek <monstr@monstr.eu>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/microblaze/kernel/signal.c |   14 +++++---------
 1 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/arch/microblaze/kernel/signal.c b/arch/microblaze/kernel/signal.c
index f2c13d5..aaf7c27 100644
--- a/arch/microblaze/kernel/signal.c
+++ b/arch/microblaze/kernel/signal.c
@@ -105,10 +105,7 @@ asmlinkage long sys_rt_sigreturn(struct pt_regs *regs)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(regs, &frame->uc.uc_mcontext, &rval))
 		goto badframe;
@@ -315,6 +312,7 @@ static int
 handle_signal(unsigned long sig, struct k_sigaction *ka,
 		siginfo_t *info, sigset_t *oldset, struct pt_regs *regs)
 {
+	sigset_t blocked;
 	int ret;
 
 	/* Set up the stack frame */
@@ -326,12 +324,10 @@ handle_signal(unsigned long sig, struct k_sigaction *ka,
 	if (ret)
 		return ret;
 
-	spin_lock_irq(&current->sighand->siglock);
-	sigorsets(&current->blocked, &current->blocked, &ka->sa.sa_mask);
+	sigorsets(&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);
+		sigaddset(&blocked, sig);
+	set_current_blocked(&blocked);
 
 	return 0;
 }
-- 
1.7.4.4

[PATCH 16/41] MIPS: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: David Daney <ddaney@caviumnetworks.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/mips/kernel/signal.c     |   29 ++++++++---------------------
 arch/mips/kernel/signal32.c   |   20 ++++----------------
 arch/mips/kernel/signal_n32.c |   10 ++--------
 3 files changed, 14 insertions(+), 45 deletions(-)

diff --git a/arch/mips/kernel/signal.c b/arch/mips/kernel/signal.c
index dbbe0ce..bd3f75b 100644
--- a/arch/mips/kernel/signal.c
+++ b/arch/mips/kernel/signal.c
@@ -255,11 +255,8 @@ asmlinkage int sys_sigsuspend(nabi_no_regargs struct pt_regs regs)
 		return -EFAULT;
 	sigdelsetmask(&newset, ~_BLOCKABLE);
 
-	spin_lock_irq(&current->sighand->siglock);
 	current->saved_sigmask = current->blocked;
-	current->blocked = newset;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&newset);
 
 	current->state = TASK_INTERRUPTIBLE;
 	schedule();
@@ -284,11 +281,8 @@ asmlinkage int sys_rt_sigsuspend(nabi_no_regargs struct pt_regs regs)
 		return -EFAULT;
 	sigdelsetmask(&newset, ~_BLOCKABLE);
 
-	spin_lock_irq(&current->sighand->siglock);
 	current->saved_sigmask = current->blocked;
-	current->blocked = newset;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&newset);
 
 	current->state = TASK_INTERRUPTIBLE;
 	schedule();
@@ -360,10 +354,7 @@ asmlinkage void sys_sigreturn(nabi_no_regargs struct pt_regs regs)
 		goto badframe;
 
 	sigdelsetmask(&blocked, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = blocked;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&blocked);
 
 	sig = restore_sigcontext(&regs, &frame->sf_sc);
 	if (sig < 0)
@@ -399,10 +390,7 @@ asmlinkage void sys_rt_sigreturn(nabi_no_regargs struct pt_regs regs)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	sig = restore_sigcontext(&regs, &frame->rs_uc.uc_mcontext);
 	if (sig < 0)
@@ -546,6 +534,7 @@ static int handle_signal(unsigned long sig, siginfo_t *info,
 	int ret;
 	struct mips_abi *abi = current->thread.abi;
 	void *vdso = current->mm->context.vdso;
+	sigset_t blocked;
 
 	if (regs->regs[0]) {
 		switch(regs->regs[2]) {
@@ -578,12 +567,10 @@ static int handle_signal(unsigned long sig, siginfo_t *info,
 	if (ret)
 		return ret;
 
-	spin_lock_irq(&current->sighand->siglock);
-	sigorsets(&current->blocked, &current->blocked, &ka->sa.sa_mask);
+	sigorsets(&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);
+		sigaddset(&blocked, sig);
+	set_current_blocked(&blocked);
 
 	return ret;
 }
diff --git a/arch/mips/kernel/signal32.c b/arch/mips/kernel/signal32.c
index aae9866..902a889 100644
--- a/arch/mips/kernel/signal32.c
+++ b/arch/mips/kernel/signal32.c
@@ -290,11 +290,8 @@ asmlinkage int sys32_sigsuspend(nabi_no_regargs struct pt_regs regs)
 		return -EFAULT;
 	sigdelsetmask(&newset, ~_BLOCKABLE);
 
-	spin_lock_irq(&current->sighand->siglock);
 	current->saved_sigmask = current->blocked;
-	current->blocked = newset;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&newset);
 
 	current->state = TASK_INTERRUPTIBLE;
 	schedule();
@@ -318,11 +315,8 @@ asmlinkage int sys32_rt_sigsuspend(nabi_no_regargs struct pt_regs regs)
 		return -EFAULT;
 	sigdelsetmask(&newset, ~_BLOCKABLE);
 
-	spin_lock_irq(&current->sighand->siglock);
 	current->saved_sigmask = current->blocked;
-	current->blocked = newset;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&newset);
 
 	current->state = TASK_INTERRUPTIBLE;
 	schedule();
@@ -488,10 +482,7 @@ asmlinkage void sys32_sigreturn(nabi_no_regargs struct pt_regs regs)
 		goto badframe;
 
 	sigdelsetmask(&blocked, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = blocked;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&blocked);
 
 	sig = restore_sigcontext32(&regs, &frame->sf_sc);
 	if (sig < 0)
@@ -529,10 +520,7 @@ asmlinkage void sys32_rt_sigreturn(nabi_no_regargs struct pt_regs regs)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	sig = restore_sigcontext32(&regs, &frame->rs_uc.uc_mcontext);
 	if (sig < 0)
diff --git a/arch/mips/kernel/signal_n32.c b/arch/mips/kernel/signal_n32.c
index ee24d81..30fc7ff 100644
--- a/arch/mips/kernel/signal_n32.c
+++ b/arch/mips/kernel/signal_n32.c
@@ -94,11 +94,8 @@ asmlinkage int sysn32_rt_sigsuspend(nabi_no_regargs struct pt_regs regs)
 	sigset_from_compat(&newset, &uset);
 	sigdelsetmask(&newset, ~_BLOCKABLE);
 
-	spin_lock_irq(&current->sighand->siglock);
 	current->saved_sigmask = current->blocked;
-	current->blocked = newset;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&newset);
 
 	current->state = TASK_INTERRUPTIBLE;
 	schedule();
@@ -122,10 +119,7 @@ asmlinkage void sysn32_rt_sigreturn(nabi_no_regargs struct pt_regs regs)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	sig = restore_sigcontext(&regs, &frame->rs_uc.uc_mcontext);
 	if (sig < 0)
-- 
1.7.4.4

[PATCH 17/41] mn10300: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: David Howells <dhowells@redhat.com>
Cc: Koichi Yasutake <yasutake.koichi@jp.panasonic.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/mn10300/kernel/signal.c |   32 +++++++++++++-------------------
 1 files changed, 13 insertions(+), 19 deletions(-)

diff --git a/arch/mn10300/kernel/signal.c b/arch/mn10300/kernel/signal.c
index 690f4e9..107fb4e 100644
--- a/arch/mn10300/kernel/signal.c
+++ b/arch/mn10300/kernel/signal.c
@@ -38,12 +38,13 @@
  */
 asmlinkage long sys_sigsuspend(int history0, int history1, old_sigset_t mask)
 {
-	mask &= _BLOCKABLE;
-	spin_lock_irq(&current->sighand->siglock);
+	sigset_t blocked;
+
 	current->saved_sigmask = current->blocked;
-	siginitset(&current->blocked, mask);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+
+	mask &= _BLOCKABLE;
+	siginitset(&blocked, mask);
+	set_current_blocked(&blocked);
 
 	current->state = TASK_INTERRUPTIBLE;
 	schedule();
@@ -172,10 +173,7 @@ asmlinkage long sys_sigreturn(void)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(current_frame(), &frame->sc, &d0))
 		goto badframe;
@@ -203,10 +201,7 @@ asmlinkage long sys_rt_sigreturn(void)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(current_frame(), &frame->uc.uc_mcontext, &d0))
 		goto badframe;
@@ -477,13 +472,12 @@ static int handle_signal(int sig,
 		ret = setup_frame(sig, ka, oldset, regs);
 
 	if (ret == 0) {
-		spin_lock_irq(&current->sighand->siglock);
-		sigorsets(&current->blocked, &current->blocked,
-			  &ka->sa.sa_mask);
+		sigset_t blocked;
+
+		sigorsets(&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);
+			sigaddset(&blocked, sig);
+		set_current_blocked(&blocked);
 	}
 
 	return ret;
-- 
1.7.4.4

[PATCH 18/41] OpenRISC: Don't reimplement force_sigsegv()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

Instead of open coding the sequence from force_sigsegv() just call
it. This also fixes a race because sa_handler was being modified
without holding ->sighand->siglock.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Jonas Bonn <jonas@southpole.se>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/openrisc/kernel/signal.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/arch/openrisc/kernel/signal.c b/arch/openrisc/kernel/signal.c
index 5f759c7..c023db9 100644
--- a/arch/openrisc/kernel/signal.c
+++ b/arch/openrisc/kernel/signal.c
@@ -257,9 +257,7 @@ static void setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
 	return;
 
 give_sigsegv:
-	if (sig == SIGSEGV)
-		ka->sa.sa_handler = SIG_DFL;
-	force_sig(SIGSEGV, current);
+	force_sigsegv(sig, current);
 }
 
 static inline void
-- 
1.7.4.4

[PATCH 19/41] OpenRISC: No need to reset handler if SA_ONESHOT

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

get_signal_to_deliver() already resets the signal handler if
SA_ONESHOT is set in ka->sa.sa_flags, there's no need to do it again
in handle_signal(). Furthermore, because we were modifying
ka->sa.sa_handler without holding ->sighand->siglock there's the
potential we can race with another thread if it modifies the signal
handlers.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Jonas Bonn <jonas@southpole.se>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/openrisc/kernel/signal.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/arch/openrisc/kernel/signal.c b/arch/openrisc/kernel/signal.c
index c023db9..2f75e74 100644
--- a/arch/openrisc/kernel/signal.c
+++ b/arch/openrisc/kernel/signal.c
@@ -267,9 +267,6 @@ handle_signal(unsigned long sig,
 {
 	setup_rt_frame(sig, ka, info, oldset, regs);
 
-	if (ka->sa.sa_flags & SA_ONESHOT)
-		ka->sa.sa_handler = SIG_DFL;
-
 	spin_lock_irq(&current->sighand->siglock);
 	sigorsets(&current->blocked, &current->blocked, &ka->sa.sa_mask);
 	if (!(ka->sa.sa_flags & SA_NODEFER))
-- 
1.7.4.4

[PATCH 20/41] OpenRISC: Don't mask signals if we fail to setup signal stack

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

setup_rt_frame() needs to return an indication of whether it succeeded
or failed in setting up the signal stack frame. If setup_rt_frame()
fails then we must not modify current->blocked.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Jonas Bonn <jonas@southpole.se>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/openrisc/kernel/signal.c |   29 ++++++++++++++++++-----------
 1 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/arch/openrisc/kernel/signal.c b/arch/openrisc/kernel/signal.c
index 2f75e74..b0bdfda 100644
--- a/arch/openrisc/kernel/signal.c
+++ b/arch/openrisc/kernel/signal.c
@@ -196,8 +196,8 @@ static inline void __user *get_sigframe(struct k_sigaction *ka,
  * trampoline which performs the syscall sigreturn, or a provided
  * user-mode trampoline.
  */
-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 *frame;
 	unsigned long return_ip;
@@ -254,18 +254,23 @@ static void setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
 	/* actually move the usp to reflect the stacked frame */
 	regs->sp = (unsigned long)frame;
 
-	return;
+	return 0;
 
 give_sigsegv:
 	force_sigsegv(sig, current);
+	return -EFAULT;
 }
 
-static inline void
+static inline int
 handle_signal(unsigned long sig,
 	      siginfo_t *info, struct k_sigaction *ka,
 	      sigset_t *oldset, struct pt_regs *regs)
 {
-	setup_rt_frame(sig, ka, info, oldset, regs);
+	int ret;
+
+	ret = setup_rt_frame(sig, ka, info, oldset, regs);
+	if (ret)
+		return ret;
 
 	spin_lock_irq(&current->sighand->siglock);
 	sigorsets(&current->blocked, &current->blocked, &ka->sa.sa_mask);
@@ -274,6 +279,8 @@ handle_signal(unsigned long sig,
 	recalc_sigpending();
 
 	spin_unlock_irq(&current->sighand->siglock);
+
+	return 0;
 }
 
 /*
@@ -362,13 +369,13 @@ void do_signal(struct pt_regs *regs)
 			oldset = &current->blocked;
 
 		/* Whee!  Actually deliver the signal.  */
-		handle_signal(signr, &info, &ka, oldset, regs);
-		/* 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))
+		if (!handle_signal(signr, &info, &ka, oldset, regs)) {
+			/* 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 */
 			clear_thread_flag(TIF_RESTORE_SIGMASK);
+		}
 
 		tracehook_signal_handler(signr, &info, &ka, regs,
 					 test_thread_flag(TIF_SINGLESTEP));
-- 
1.7.4.4

[PATCH 21/41] OpenRISC: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Jonas Bonn <jonas@southpole.se>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/openrisc/kernel/signal.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/arch/openrisc/kernel/signal.c b/arch/openrisc/kernel/signal.c
index b0bdfda..50a5156 100644
--- a/arch/openrisc/kernel/signal.c
+++ b/arch/openrisc/kernel/signal.c
@@ -108,10 +108,7 @@ asmlinkage long _sys_rt_sigreturn(struct pt_regs *regs)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(regs, &frame->uc.uc_mcontext))
 		goto badframe;
@@ -266,19 +263,17 @@ handle_signal(unsigned long sig,
 	      siginfo_t *info, struct k_sigaction *ka,
 	      sigset_t *oldset, struct pt_regs *regs)
 {
+	sigset_t blocked;
 	int ret;
 
 	ret = setup_rt_frame(sig, ka, info, oldset, regs);
 	if (ret)
 		return ret;
 
-	spin_lock_irq(&current->sighand->siglock);
-	sigorsets(&current->blocked, &current->blocked, &ka->sa.sa_mask);
+	sigorsets(&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);
+		sigaddset(&blocked, sig);
+	set_current_blocked(&blocked);
 
 	return 0;
 }
-- 
1.7.4.4

[PATCH 22/41] parisc: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Kyle McMartin <kyle@mcmartin.ca>
Cc: Helge Deller <deller@gmx.de>
Cc: "James E.J. Bottomley" <jejb@parisc-linux.org>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/parisc/kernel/signal.c |   15 ++++++---------
 1 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/arch/parisc/kernel/signal.c b/arch/parisc/kernel/signal.c
index 12c1ed3..4d64f72 100644
--- a/arch/parisc/kernel/signal.c
+++ b/arch/parisc/kernel/signal.c
@@ -131,10 +131,7 @@ sys_rt_sigreturn(struct pt_regs *regs, int in_syscall)
 	}
 		
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	/* Good thing we saved the old gr[30], eh? */
 #ifdef CONFIG_64BIT
@@ -447,6 +444,8 @@ static long
 handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka,
 		sigset_t *oldset, struct pt_regs *regs, int in_syscall)
 {
+	sigset_t blocked;
+
 	DBG(1,"handle_signal: sig=%ld, ka=%p, info=%p, oldset=%p, regs=%p\n",
 	       sig, ka, info, oldset, regs);
 	
@@ -454,12 +453,10 @@ handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka,
 	if (!setup_rt_frame(sig, ka, info, oldset, regs, in_syscall))
 		return 0;
 
-	spin_lock_irq(&current->sighand->siglock);
-	sigorsets(&current->blocked,&current->blocked,&ka->sa.sa_mask);
+	sigorsets(&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);
+		sigaddset(&blocked, sig);
+	set_current_blocked(&blocked);
 
 	tracehook_signal_handler(sig, info, ka, regs, 
 		test_thread_flag(TIF_SINGLESTEP) ||
-- 
1.7.4.4

[PATCH 23/41] powerpc: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/powerpc/kernel/signal.c    |   16 ++++++----------
 arch/powerpc/kernel/signal_32.c |   11 ++++++-----
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index 2300426..8f6731f 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -56,10 +56,7 @@ void __user * get_sigframe(struct k_sigaction *ka, struct pt_regs *regs,
 void restore_sigmask(sigset_t *set)
 {
 	sigdelsetmask(set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = *set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(set);
 }
 
 static void check_syscall_restart(struct pt_regs *regs, struct k_sigaction *ka,
@@ -167,13 +164,12 @@ static int do_signal_pending(sigset_t *oldset, struct pt_regs *regs)
 
 	regs->trap = 0;
 	if (ret) {
-		spin_lock_irq(&current->sighand->siglock);
-		sigorsets(&current->blocked, &current->blocked,
-			  &ka.sa.sa_mask);
+		sigset_t blocked;
+
+		sigorsets(&blocked, &current->blocked, &ka.sa.sa_mask);
 		if (!(ka.sa.sa_flags & SA_NODEFER))
-			sigaddset(&current->blocked, signr);
-		recalc_sigpending();
-		spin_unlock_irq(&current->sighand->siglock);
+			sigaddset(&blocked, signr);
+		set_current_blocked(&blocked);
 
 		/*
 		 * A signal was successfully delivered; the saved sigmask is in
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 78b76dc..5fc9d19 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -242,12 +242,13 @@ static inline int restore_general_regs(struct pt_regs *regs,
  */
 long sys_sigsuspend(old_sigset_t mask)
 {
-	mask &= _BLOCKABLE;
-	spin_lock_irq(&current->sighand->siglock);
+	sigset_t blocked;
+
 	current->saved_sigmask = current->blocked;
-	siginitset(&current->blocked, mask);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+
+	mask &= _BLOCKABLE;
+	siginitset(&blocked, mask);
+	set_current_blocked(&blocked);
 
  	current->state = TASK_INTERRUPTIBLE;
  	schedule();
-- 
1.7.4.4

[PATCH 24/41] score: Don't mask signals if we fail to setup signal stack

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

If setup_rt_frame() returns -EFAULT then we must not block any signals
in the current process.

Cc: Chen Liqin <liqin.chen@sunplusct.com>
Cc: Lennox Wu <lennox.wu@gmail.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/score/kernel/signal.c |   14 ++++++++------
 1 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/score/kernel/signal.c b/arch/score/kernel/signal.c
index aa57440..bf9e33e 100644
--- a/arch/score/kernel/signal.c
+++ b/arch/score/kernel/signal.c
@@ -272,12 +272,14 @@ static int handle_signal(unsigned long sig, siginfo_t *info,
 	 */
 	ret = setup_rt_frame(ka, regs, sig, oldset, info);
 
-	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);
+	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))
+			sigaddset(&current->blocked, sig);
+		recalc_sigpending();
+		spin_unlock_irq(&current->sighand->siglock);
+	}
 
 	return ret;
 }
-- 
1.7.4.4

[PATCH 25/41] score: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Chen Liqin <liqin.chen@sunplusct.com>
Cc: Lennox Wu <lennox.wu@gmail.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/score/kernel/signal.c |   15 ++++++---------
 1 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/arch/score/kernel/signal.c b/arch/score/kernel/signal.c
index bf9e33e..3ba0298 100644
--- a/arch/score/kernel/signal.c
+++ b/arch/score/kernel/signal.c
@@ -159,10 +159,7 @@ score_rt_sigreturn(struct pt_regs *regs)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	sig = restore_sigcontext(regs, &frame->rs_uc.uc_mcontext);
 	if (sig < 0)
@@ -273,12 +270,12 @@ static int handle_signal(unsigned long sig, siginfo_t *info,
 	ret = setup_rt_frame(ka, regs, sig, oldset, info);
 
 	if (ret == 0) {
-		spin_lock_irq(&current->sighand->siglock);
-		sigorsets(&current->blocked, &current->blocked, &ka->sa.sa_mask);
+		sigset_t blocked;
+
+		sigorsets(&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);
+			sigaddset(&blocked, sig);
+		set_current_blocked(&blocked);
 	}
 
 	return ret;
-- 
1.7.4.4

[PATCH 26/41] sh: No need to reset handler if SA_ONESHOT

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

get_signal_to_deliver() already resets the signal handler if
SA_ONESHOT is set in ka->sa.sa_flags, there's no need to do it again
in handle_signal(). Furthermore, because we were modifying
ka->sa.sa_handler without holding ->sighand->siglock there's the
potential we can race with another thread if it modifies the signal
handlers.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/sh/kernel/signal_32.c |    3 ---
 arch/sh/kernel/signal_64.c |    3 ---
 2 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/arch/sh/kernel/signal_32.c b/arch/sh/kernel/signal_32.c
index 579cd2c..7a00304 100644
--- a/arch/sh/kernel/signal_32.c
+++ b/arch/sh/kernel/signal_32.c
@@ -548,9 +548,6 @@ handle_signal(unsigned long sig, struct k_sigaction *ka, siginfo_t *info,
 	else
 		ret = setup_frame(sig, ka, oldset, regs);
 
-	if (ka->sa.sa_flags & SA_ONESHOT)
-		ka->sa.sa_handler = SIG_DFL;
-
 	if (ret == 0) {
 		spin_lock_irq(&current->sighand->siglock);
 		sigorsets(&current->blocked,&current->blocked,&ka->sa.sa_mask);
diff --git a/arch/sh/kernel/signal_64.c b/arch/sh/kernel/signal_64.c
index 5a9f1f1..c0e7d10 100644
--- a/arch/sh/kernel/signal_64.c
+++ b/arch/sh/kernel/signal_64.c
@@ -738,9 +738,6 @@ handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka,
 	else
 		ret = setup_frame(sig, ka, oldset, regs);
 
-	if (ka->sa.sa_flags & SA_ONESHOT)
-		ka->sa.sa_handler = SIG_DFL;
-
 	if (ret == 0) {
 		spin_lock_irq(&current->sighand->siglock);
 		sigorsets(&current->blocked,&current->blocked,&ka->sa.sa_mask);
-- 
1.7.4.4

[PATCH 27/41] sh: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Paul Mundt <lethal@linux-sh.org>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/sh/kernel/signal_32.c |   32 +++++++++++++-------------------
 arch/sh/kernel/signal_64.c |   37 +++++++++++++------------------------
 2 files changed, 26 insertions(+), 43 deletions(-)

diff --git a/arch/sh/kernel/signal_32.c b/arch/sh/kernel/signal_32.c
index 7a00304..c97705f 100644
--- a/arch/sh/kernel/signal_32.c
+++ b/arch/sh/kernel/signal_32.c
@@ -58,12 +58,13 @@ sys_sigsuspend(old_sigset_t mask,
 	       unsigned long r5, unsigned long r6, unsigned long r7,
 	       struct pt_regs __regs)
 {
-	mask &= _BLOCKABLE;
-	spin_lock_irq(&current->sighand->siglock);
+	sigset_t blocked;
+
 	current->saved_sigmask = current->blocked;
-	siginitset(&current->blocked, mask);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+
+	mask &= _BLOCKABLE;
+	siginitset(&blocked, mask);
+	set_current_blocked(&blocked);
 
 	current->state = TASK_INTERRUPTIBLE;
 	schedule();
@@ -240,11 +241,7 @@ asmlinkage int sys_sigreturn(unsigned long r4, unsigned long r5,
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(regs, &frame->sc, &r0))
 		goto badframe;
@@ -274,10 +271,7 @@ asmlinkage int sys_rt_sigreturn(unsigned long r4, unsigned long r5,
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(regs, &frame->uc.uc_mcontext, &r0))
 		goto badframe;
@@ -549,12 +543,12 @@ handle_signal(unsigned long sig, struct k_sigaction *ka, siginfo_t *info,
 		ret = setup_frame(sig, ka, oldset, regs);
 
 	if (ret == 0) {
-		spin_lock_irq(&current->sighand->siglock);
-		sigorsets(&current->blocked,&current->blocked,&ka->sa.sa_mask);
+		sigset_t blocked;
+
+		sigorsets(&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);
+			sigaddset(&blocked, sig);
+		set_current_blocked(&blocked);
 	}
 
 	return ret;
diff --git a/arch/sh/kernel/signal_64.c b/arch/sh/kernel/signal_64.c
index c0e7d10..14d5ed4 100644
--- a/arch/sh/kernel/signal_64.c
+++ b/arch/sh/kernel/signal_64.c
@@ -163,14 +163,13 @@ sys_sigsuspend(old_sigset_t mask,
 	       unsigned long r6, unsigned long r7,
 	       struct pt_regs * regs)
 {
-	sigset_t saveset;
+	sigset_t saveset, blocked;
 
-	mask &= _BLOCKABLE;
-	spin_lock_irq(&current->sighand->siglock);
 	saveset = current->blocked;
-	siginitset(&current->blocked, mask);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+
+	mask &= _BLOCKABLE;
+	siginitset(&blocked, mask);
+	set_current_blocked(&blocked);
 
 	REF_REG_RET = -EINTR;
 	while (1) {
@@ -202,11 +201,8 @@ sys_rt_sigsuspend(sigset_t *unewset, size_t sigsetsize,
 	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);
+	set_current_blocked(&newset);
 
 	REF_REG_RET = -EINTR;
 	while (1) {
@@ -412,11 +408,7 @@ asmlinkage int sys_sigreturn(unsigned long r2, unsigned long r3,
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(regs, &frame->sc, &ret))
 		goto badframe;
@@ -449,10 +441,7 @@ asmlinkage int sys_rt_sigreturn(unsigned long r2, unsigned long r3,
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(regs, &frame->uc.uc_mcontext, &ret))
 		goto badframe;
@@ -739,12 +728,12 @@ handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka,
 		ret = setup_frame(sig, ka, oldset, regs);
 
 	if (ret == 0) {
-		spin_lock_irq(&current->sighand->siglock);
-		sigorsets(&current->blocked,&current->blocked,&ka->sa.sa_mask);
+		sigset_t blocked;
+
+		sigorsets(&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);
+			sigaddset(&blocked, sig);
+		set_current_blocked(&blocked);
 	}
 
 	return ret;
-- 
1.7.4.4

[PATCH 28/41] sparc: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/sparc/kernel/signal32.c  |   19 ++++++-------------
 arch/sparc/kernel/signal_32.c |   30 ++++++++++++------------------
 arch/sparc/kernel/signal_64.c |   30 ++++++++++++------------------
 3 files changed, 30 insertions(+), 49 deletions(-)

diff --git a/arch/sparc/kernel/signal32.c b/arch/sparc/kernel/signal32.c
index 75fad42..4f381ce 100644
--- a/arch/sparc/kernel/signal32.c
+++ b/arch/sparc/kernel/signal32.c
@@ -287,10 +287,7 @@ void do_sigreturn32(struct pt_regs *regs)
 		case 1: set.sig[0] = seta[0] + (((long)seta[1]) << 32);
 	}
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 	return;
 
 segv:
@@ -383,10 +380,7 @@ asmlinkage void do_rt_sigreturn32(struct pt_regs *regs)
 		case 1: set.sig[0] = seta.sig[0] + (((long)seta.sig[1]) << 32);
 	}
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 	return;
 segv:
 	force_sig(SIGSEGV, current);
@@ -756,6 +750,7 @@ static inline int handle_signal32(unsigned long signr, struct k_sigaction *ka,
 				  siginfo_t *info,
 				  sigset_t *oldset, struct pt_regs *regs)
 {
+	sigset_t blocked;
 	int err;
 
 	if (ka->sa.sa_flags & SA_SIGINFO)
@@ -766,12 +761,10 @@ static inline int handle_signal32(unsigned long signr, struct k_sigaction *ka,
 	if (err)
 		return err;
 
-	spin_lock_irq(&current->sighand->siglock);
-	sigorsets(&current->blocked,&current->blocked,&ka->sa.sa_mask);
+	sigorsets(&blocked, &current->blocked, &ka->sa.sa_mask);
 	if (!(ka->sa.sa_flags & SA_NOMASK))
-		sigaddset(&current->blocked,signr);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+		sigaddset(&blocked, signr);
+	set_current_blocked(&blocked);
 
 	tracehook_signal_handler(signr, info, ka, regs, 0);
 
diff --git a/arch/sparc/kernel/signal_32.c b/arch/sparc/kernel/signal_32.c
index 5e5c5fd..d65c31d 100644
--- a/arch/sparc/kernel/signal_32.c
+++ b/arch/sparc/kernel/signal_32.c
@@ -60,12 +60,13 @@ struct rt_signal_frame {
 
 static int _sigpause_common(old_sigset_t set)
 {
-	set &= _BLOCKABLE;
-	spin_lock_irq(&current->sighand->siglock);
+	sigset_t blocked;
+
 	current->saved_sigmask = current->blocked;
-	siginitset(&current->blocked, set);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+
+	set &= _BLOCKABLE;
+	siginitset(&blocked, set);
+	set_current_blocked(&blocked);
 
 	current->state = TASK_INTERRUPTIBLE;
 	schedule();
@@ -165,10 +166,7 @@ asmlinkage void do_sigreturn(struct pt_regs *regs)
 		goto segv_and_exit;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 	return;
 
 segv_and_exit:
@@ -229,10 +227,7 @@ asmlinkage void do_rt_sigreturn(struct pt_regs *regs)
 	set_fs(old_fs);
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 	return;
 segv:
 	force_sig(SIGSEGV, current);
@@ -484,6 +479,7 @@ static inline int
 handle_signal(unsigned long signr, struct k_sigaction *ka,
 	      siginfo_t *info, sigset_t *oldset, struct pt_regs *regs)
 {
+	sigset_t blocked;
 	int err;
 
 	if (ka->sa.sa_flags & SA_SIGINFO)
@@ -494,12 +490,10 @@ handle_signal(unsigned long signr, struct k_sigaction *ka,
 	if (err)
 		return err;
 
-	spin_lock_irq(&current->sighand->siglock);
-	sigorsets(&current->blocked,&current->blocked,&ka->sa.sa_mask);
+	sigorsets(&blocked, &current->blocked, &ka->sa.sa_mask);
 	if (!(ka->sa.sa_flags & SA_NOMASK))
-		sigaddset(&current->blocked, signr);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+		sigaddset(&blocked, signr);
+	set_current_blocked(&blocked);
 
 	tracehook_signal_handler(signr, info, ka, regs, 0);
 
diff --git a/arch/sparc/kernel/signal_64.c b/arch/sparc/kernel/signal_64.c
index 006fe45..bb153d1 100644
--- a/arch/sparc/kernel/signal_64.c
+++ b/arch/sparc/kernel/signal_64.c
@@ -69,10 +69,7 @@ asmlinkage void sparc64_set_context(struct pt_regs *regs)
 				goto do_sigsegv;
 		}
 		sigdelsetmask(&set, ~_BLOCKABLE);
-		spin_lock_irq(&current->sighand->siglock);
-		current->blocked = set;
-		recalc_sigpending();
-		spin_unlock_irq(&current->sighand->siglock);
+		set_current_blocked(&set);
 	}
 	if (test_thread_flag(TIF_32BIT)) {
 		pc &= 0xffffffff;
@@ -241,12 +238,13 @@ struct rt_signal_frame {
 
 static long _sigpause_common(old_sigset_t set)
 {
-	set &= _BLOCKABLE;
-	spin_lock_irq(&current->sighand->siglock);
+	sigset_t blocked;
+
 	current->saved_sigmask = current->blocked;
-	siginitset(&current->blocked, set);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+
+	set &= _BLOCKABLE;
+	siginitset(&blocked, set);
+	set_current_blocked(&set);
 
 	current->state = TASK_INTERRUPTIBLE;
 	schedule();
@@ -341,10 +339,7 @@ void do_rt_sigreturn(struct pt_regs *regs)
 	pt_regs_clear_syscall(regs);
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 	return;
 segv:
 	force_sig(SIGSEGV, current);
@@ -498,18 +493,17 @@ static inline int handle_signal(unsigned long signr, struct k_sigaction *ka,
 				siginfo_t *info,
 				sigset_t *oldset, struct pt_regs *regs)
 {
+	sigset_t blocked;
 	int err;
 
 	err = setup_rt_frame(ka, regs, signr, oldset,
 			     (ka->sa.sa_flags & SA_SIGINFO) ? info : NULL);
 	if (err)
 		return err;
-	spin_lock_irq(&current->sighand->siglock);
-	sigorsets(&current->blocked,&current->blocked,&ka->sa.sa_mask);
+	sigorsets(&blocked, &current->blocked, &ka->sa.sa_mask);
 	if (!(ka->sa.sa_flags & SA_NOMASK))
-		sigaddset(&current->blocked,signr);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+		sigaddset(&blocked, signr);
+	set_current_blocked(&blocked);
 
 	tracehook_signal_handler(signr, info, ka, regs, 0);
 
-- 
1.7.4.4

[PATCH 29/41] tile: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Chris Metcalf <cmetcalf@tilera.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/tile/kernel/compat_signal.c |    5 +----
 arch/tile/kernel/signal.c        |   16 ++++++----------
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/arch/tile/kernel/compat_signal.c b/arch/tile/kernel/compat_signal.c
index a7869ad..77763cc 100644
--- a/arch/tile/kernel/compat_signal.c
+++ b/arch/tile/kernel/compat_signal.c
@@ -303,10 +303,7 @@ long compat_sys_rt_sigreturn(struct pt_regs *regs)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(regs, &frame->uc.uc_mcontext))
 		goto badframe;
diff --git a/arch/tile/kernel/signal.c b/arch/tile/kernel/signal.c
index bedaf4e..6ddceec 100644
--- a/arch/tile/kernel/signal.c
+++ b/arch/tile/kernel/signal.c
@@ -97,10 +97,7 @@ SYSCALL_DEFINE1(rt_sigreturn, struct pt_regs *, regs)
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(regs, &frame->uc.uc_mcontext))
 		goto badframe;
@@ -286,13 +283,12 @@ static int handle_signal(unsigned long sig, siginfo_t *info,
 		 * the work_pending path in the return-to-user code, and
 		 * either way we can re-enable interrupts unconditionally.
 		 */
-		spin_lock_irq(&current->sighand->siglock);
-		sigorsets(&current->blocked,
-			  &current->blocked, &ka->sa.sa_mask);
+		sigset_t blocked;
+
+		sigorsets(&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);
+			sigaddset(&blocked, sig);
+		set_current_blocked(&blocked);
 	}
 
 	return ret;
-- 
1.7.4.4

[PATCH 30/41] um: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Richard Weinberger <richard@nod.at>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/um/kernel/signal.c     |   22 +++++++++++-----------
 arch/um/sys-i386/signal.c   |   12 ++----------
 arch/um/sys-x86_64/signal.c |    6 +-----
 3 files changed, 14 insertions(+), 26 deletions(-)

diff --git a/arch/um/kernel/signal.c b/arch/um/kernel/signal.c
index b5c094c..1812524 100644
--- a/arch/um/kernel/signal.c
+++ b/arch/um/kernel/signal.c
@@ -73,13 +73,12 @@ static int handle_signal(struct pt_regs *regs, unsigned long signr,
 		spin_unlock_irq(&current->sighand->siglock);
 		force_sigsegv(signr, current);
 	} else {
-		spin_lock_irq(&current->sighand->siglock);
-		sigorsets(&current->blocked, &current->blocked,
-			  &ka->sa.sa_mask);
+		sigset_t blocked:
+
+		sigorsets(&blocked, &current->blocked, &ka->sa.sa_mask);
 		if (!(ka->sa.sa_flags & SA_NODEFER))
-			sigaddset(&current->blocked, signr);
-		recalc_sigpending();
-		spin_unlock_irq(&current->sighand->siglock);
+			sigaddset(&blocked, signr);
+		set_current_blocked(&blocked);
 	}
 
 	return err;
@@ -163,12 +162,13 @@ int do_signal(void)
  */
 long sys_sigsuspend(int history0, int history1, old_sigset_t mask)
 {
-	mask &= _BLOCKABLE;
-	spin_lock_irq(&current->sighand->siglock);
+	sigset_t blocked;
+
 	current->saved_sigmask = current->blocked;
-	siginitset(&current->blocked, mask);
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+
+	mask &= _BLOCKABLE;
+	siginitset(&blocked, mask);
+	set_current_blocked(&blocked);
 
 	current->state = TASK_INTERRUPTIBLE;
 	schedule();
diff --git a/arch/um/sys-i386/signal.c b/arch/um/sys-i386/signal.c
index 89a4662..4403b48 100644
--- a/arch/um/sys-i386/signal.c
+++ b/arch/um/sys-i386/signal.c
@@ -458,11 +458,7 @@ long sys_sigreturn(struct pt_regs regs)
 		goto segfault;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (copy_sc_from_user(&current->thread.regs, sc))
 		goto segfault;
@@ -489,11 +485,7 @@ long sys_rt_sigreturn(struct pt_regs regs)
 		goto segfault;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (copy_sc_from_user(&current->thread.regs, &uc->uc_mcontext))
 		goto segfault;
diff --git a/arch/um/sys-x86_64/signal.c b/arch/um/sys-x86_64/signal.c
index b6b65c7..581b64d 100644
--- a/arch/um/sys-x86_64/signal.c
+++ b/arch/um/sys-x86_64/signal.c
@@ -270,11 +270,7 @@ long sys_rt_sigreturn(struct pt_regs *regs)
 		goto segfault;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (copy_sc_from_user(&current->thread.regs, &uc->uc_mcontext,
 			      &frame->fpstate))
-- 
1.7.4.4

[PATCH 31/41] um: Don't restore current->blocked on error

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

If we fail to setup the signal stack frame then we don't need to
restore current->blocked because it is not modified by
setup_signal_stack_*.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Richard Weinberger <richard@nod.at>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/um/kernel/signal.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/arch/um/kernel/signal.c b/arch/um/kernel/signal.c
index 1812524..d177685 100644
--- a/arch/um/kernel/signal.c
+++ b/arch/um/kernel/signal.c
@@ -66,13 +66,9 @@ static int handle_signal(struct pt_regs *regs, unsigned long signr,
 #endif
 		err = setup_signal_stack_si(sp, signr, ka, regs, info, oldset);
 
-	if (err) {
-		spin_lock_irq(&current->sighand->siglock);
-		current->blocked = *oldset;
-		recalc_sigpending();
-		spin_unlock_irq(&current->sighand->siglock);
+	if (err)
 		force_sigsegv(signr, current);
-	} else {
+	else {
 		sigset_t blocked:
 
 		sigorsets(&blocked, &current->blocked, &ka->sa.sa_mask);
-- 
1.7.4.4

[PATCH 32/41] unicore32: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/unicore32/kernel/signal.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/arch/unicore32/kernel/signal.c b/arch/unicore32/kernel/signal.c
index b163fca..911b549 100644
--- a/arch/unicore32/kernel/signal.c
+++ b/arch/unicore32/kernel/signal.c
@@ -63,10 +63,7 @@ static int restore_sigframe(struct pt_regs *regs, struct sigframe __user *sf)
 	err = __copy_from_user(&set, &sf->uc.uc_sigmask, sizeof(set));
 	if (err == 0) {
 		sigdelsetmask(&set, ~_BLOCKABLE);
-		spin_lock_irq(&current->sighand->siglock);
-		current->blocked = set;
-		recalc_sigpending();
-		spin_unlock_irq(&current->sighand->siglock);
+		set_current_blocked(&set);
 	}
 
 	err |= __get_user(regs->UCreg_00, &sf->uc.uc_mcontext.regs.UCreg_00);
@@ -321,6 +318,7 @@ static int handle_signal(unsigned long sig, struct k_sigaction *ka,
 {
 	struct thread_info *thread = current_thread_info();
 	struct task_struct *tsk = current;
+	sigset_t blocked;
 	int usig = sig;
 	int ret;
 
@@ -372,13 +370,10 @@ static int handle_signal(unsigned long sig, struct k_sigaction *ka,
 	/*
 	 * Block the signal if we were successful.
 	 */
-	spin_lock_irq(&tsk->sighand->siglock);
-	sigorsets(&tsk->blocked, &tsk->blocked,
-		  &ka->sa.sa_mask);
+	sigorsets(&blocked, &tsk->blocked, &ka->sa.sa_mask);
 	if (!(ka->sa.sa_flags & SA_NODEFER))
-		sigaddset(&tsk->blocked, sig);
-	recalc_sigpending();
-	spin_unlock_irq(&tsk->sighand->siglock);
+		sigaddset(&blocked, sig);
+	set_current_blocked(&blocked);
 
 	return 0;
 }
-- 
1.7.4.4

[PATCH 33/41] xtensa: Don't reimplement force_sigsegv()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

Instead of open coding the sequence from force_sigsegv() just call
it. This also fixes a race because sa_handler was being modified
without holding ->sighand->siglock.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Chris Zankel <chris@zankel.net>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/xtensa/kernel/signal.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/arch/xtensa/kernel/signal.c b/arch/xtensa/kernel/signal.c
index f2220b5..4f53770 100644
--- a/arch/xtensa/kernel/signal.c
+++ b/arch/xtensa/kernel/signal.c
@@ -425,9 +425,7 @@ static void setup_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
 	return;
 
 give_sigsegv:
-	if (sig == SIGSEGV)
-		ka->sa.sa_handler = SIG_DFL;
-	force_sig(SIGSEGV, current);
+	force_sigsegv(sig, current);
 }
 
 /*
-- 
1.7.4.4

[PATCH 34/41] xtensa: No need to reset handler if SA_ONESHOT

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

get_signal_to_deliver() already resets the signal handler if
SA_ONESHOT is set in ka->sa.sa_flags, there's no need to do it again
in handle_signal(). Furthermore, because we were modifying
ka->sa.sa_handler without holding ->sighand->siglock there's the
potential we can race with another thread if it modifies the signal
handlers.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Chris Zankel <chris@zankel.net>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/xtensa/kernel/signal.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/arch/xtensa/kernel/signal.c b/arch/xtensa/kernel/signal.c
index 4f53770..24655e3 100644
--- a/arch/xtensa/kernel/signal.c
+++ b/arch/xtensa/kernel/signal.c
@@ -536,9 +536,6 @@ int do_signal(struct pt_regs *regs, sigset_t *oldset)
 		/* Set up the stack frame */
 		setup_frame(signr, &ka, &info, oldset, regs);
 
-		if (ka.sa.sa_flags & SA_ONESHOT)
-			ka.sa.sa_handler = SIG_DFL;
-
 		spin_lock_irq(&current->sighand->siglock);
 		sigorsets(&current->blocked, &current->blocked, &ka.sa.sa_mask);
 		if (!(ka.sa.sa_flags & SA_NODEFER))
-- 
1.7.4.4

[PATCH 35/41] xtensa: Don't mask signals if we fail to setup signal stack

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

setup_frame() needs to return an indication of whether it succeeded or
failed in setting up the signal stack frame. If setup_frame() fails
then we must not modify current->blocked.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Chris Zankel <chris@zankel.net>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/xtensa/kernel/signal.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/arch/xtensa/kernel/signal.c b/arch/xtensa/kernel/signal.c
index 24655e3..17ceab8 100644
--- a/arch/xtensa/kernel/signal.c
+++ b/arch/xtensa/kernel/signal.c
@@ -336,8 +336,8 @@ gen_return_code(unsigned char *codemem)
 }
 
 
-static void setup_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
-			sigset_t *set, struct pt_regs *regs)
+static int setup_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
+		       sigset_t *set, struct pt_regs *regs)
 {
 	struct rt_sigframe *frame;
 	int err = 0;
@@ -422,10 +422,11 @@ static void setup_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
 		current->comm, current->pid, signal, frame, regs->pc);
 #endif
 
-	return;
+	return 0;
 
 give_sigsegv:
 	force_sigsegv(sig, current);
+	return -EFAULT;
 }
 
 /*
@@ -534,7 +535,9 @@ int do_signal(struct pt_regs *regs, sigset_t *oldset)
 
 		/* Whee!  Actually deliver the signal.  */
 		/* Set up the stack frame */
-		setup_frame(signr, &ka, &info, oldset, regs);
+		ret = setup_frame(signr, &ka, &info, oldset, regs);
+		if (ret)
+			return ret;
 
 		spin_lock_irq(&current->sighand->siglock);
 		sigorsets(&current->blocked, &current->blocked, &ka.sa.sa_mask);
-- 
1.7.4.4

[PATCH 36/41] xtensa: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Chris Zankel <chris@zankel.net>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 arch/xtensa/kernel/signal.c |   19 ++++++-------------
 1 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/arch/xtensa/kernel/signal.c b/arch/xtensa/kernel/signal.c
index 17ceab8..4df5da5 100644
--- a/arch/xtensa/kernel/signal.c
+++ b/arch/xtensa/kernel/signal.c
@@ -260,10 +260,7 @@ asmlinkage long xtensa_rt_sigreturn(long a0, long a1, long a2, long a3,
 		goto badframe;
 
 	sigdelsetmask(&set, ~_BLOCKABLE);
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = set;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&set);
 
 	if (restore_sigcontext(regs, frame))
 		goto badframe;
@@ -448,11 +445,8 @@ asmlinkage long xtensa_rt_sigsuspend(sigset_t __user *unewset,
 		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);
+	set_current_blocked(&newset);
 
 	regs->areg[2] = -EINTR;
 	while (1) {
@@ -487,6 +481,7 @@ int do_signal(struct pt_regs *regs, sigset_t *oldset)
 	siginfo_t info;
 	int signr;
 	struct k_sigaction ka;
+	sigset_t blocked;
 
 	if (!user_mode(regs))
 		return 0;
@@ -539,12 +534,10 @@ int do_signal(struct pt_regs *regs, sigset_t *oldset)
 		if (ret)
 			return ret;
 
-		spin_lock_irq(&current->sighand->siglock);
-		sigorsets(&current->blocked, &current->blocked, &ka.sa.sa_mask);
+		sigorsets(&blocked, &current->blocked, &ka.sa.sa_mask);
 		if (!(ka.sa.sa_flags & SA_NODEFER))
-			sigaddset(&current->blocked, signr);
-		recalc_sigpending();
-		spin_unlock_irq(&current->sighand->siglock);
+			sigaddset(&blocked, signr);
+		set_current_blocked(&blocked);
 		if (current->ptrace & PT_SINGLESTEP)
 			task_pt_regs(current)->icountlevel = 1;
 
-- 
1.7.4.4

[PATCH 37/41] autofs4: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Ian Kent <raven@themaw.net>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 fs/autofs4/waitq.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/fs/autofs4/waitq.c b/fs/autofs4/waitq.c
index e1fbdee..64ff15a 100644
--- a/fs/autofs4/waitq.c
+++ b/fs/autofs4/waitq.c
@@ -458,21 +458,16 @@ int autofs4_wait(struct autofs_sb_info *sbi, struct dentry *dentry,
 	 */
 	if (wq->name.name) {
 		/* Block all but "shutdown" signals while waiting */
-		sigset_t oldset;
+		sigset_t oldset, blocked;
 		unsigned long irqflags;
 
-		spin_lock_irqsave(&current->sighand->siglock, irqflags);
 		oldset = current->blocked;
-		siginitsetinv(&current->blocked, SHUTDOWN_SIGS & ~oldset.sig[0]);
-		recalc_sigpending();
-		spin_unlock_irqrestore(&current->sighand->siglock, irqflags);
+		siginitsetinv(&blocked, SHUTDOWN_SIGS & ~oldset.sig[0]);
+		set_current_blocked(&blocked);
 
 		wait_event_interruptible(wq->queue, wq->name.name == NULL);
-
-		spin_lock_irqsave(&current->sighand->siglock, irqflags);
-		current->blocked = oldset;
-		recalc_sigpending();
-		spin_unlock_irqrestore(&current->sighand->siglock, irqflags);
+		
+		set_current_blocked(&oldset);
 	} else {
 		DPRINTK("skipped sleeping");
 	}
-- 
1.7.4.4

[PATCH 38/41] coda: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Jan Harkes <jaharkes@cs.cmu.edu>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 fs/coda/upcall.c |   19 ++++++++-----------
 1 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/fs/coda/upcall.c b/fs/coda/upcall.c
index 9727e0c..ce5925d 100644
--- a/fs/coda/upcall.c
+++ b/fs/coda/upcall.c
@@ -575,24 +575,21 @@ int venus_statfs(struct dentry *dentry, struct kstatfs *sfs)
  */
 static void coda_block_signals(sigset_t *old)
 {
-	spin_lock_irq(&current->sighand->siglock);
+	sigset_t blocked;
+
 	*old = current->blocked;
 
-	sigfillset(&current->blocked);
-	sigdelset(&current->blocked, SIGKILL);
-	sigdelset(&current->blocked, SIGSTOP);
-	sigdelset(&current->blocked, SIGINT);
+	sigfillset(&blocked);
+	sigdelset(&blocked, SIGKILL);
+	sigdelset(&blocked, SIGSTOP);
+	sigdelset(&blocked, SIGINT);
 
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(&blocked);
 }
 
 static void coda_unblock_signals(sigset_t *old)
 {
-	spin_lock_irq(&current->sighand->siglock);
-	current->blocked = *old;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
+	set_current_blocked(old);
 }
 
 /* Don't allow signals to interrupt the following upcalls before venus
-- 
1.7.4.4

[PATCH 39/41] dlm: Remove another superfluous call to recalc_sigpending()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

recalc_sigpending() is called within sigprocmask(), so there is no
need call it again after sigprocmask() has returned. I must have
missed this call when removing the other recalc_sigpending() in commit
4bcad6c1ef53 ("dlm: Remove superfluous call to recalc_sigpending())".

Cc: Christine Caulfield <ccaulfie@redhat.com>
Cc: David Teigland <teigland@redhat.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 fs/dlm/user.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/fs/dlm/user.c b/fs/dlm/user.c
index d8ea607..b38b122 100644
--- a/fs/dlm/user.c
+++ b/fs/dlm/user.c
@@ -678,7 +678,6 @@ static int device_close(struct inode *inode, struct file *file)
 	   device_remove_lockspace() */
 
 	sigprocmask(SIG_SETMASK, &tmpsig, NULL);
-	recalc_sigpending();
 
 	return 0;
 }
-- 
1.7.4.4

[PATCH 40/41] ncpfs: Use set_current_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Petr Vandrovec <petr@vandrovec.name>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 fs/ncpfs/sock.c |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/fs/ncpfs/sock.c b/fs/ncpfs/sock.c
index 3a15872..883fdc3 100644
--- a/fs/ncpfs/sock.c
+++ b/fs/ncpfs/sock.c
@@ -749,7 +749,7 @@ static int ncp_do_request(struct ncp_server *server, int size,
 		return -EIO;
 	}
 	{
-		sigset_t old_set;
+		sigset_t old_set, blocked;
 		unsigned long mask, flags;
 
 		spin_lock_irqsave(&current->sighand->siglock, flags);
@@ -769,16 +769,14 @@ static int ncp_do_request(struct ncp_server *server, int size,
 			if (current->sighand->action[SIGQUIT - 1].sa.sa_handler == SIG_DFL)
 				mask |= sigmask(SIGQUIT);
 		}
-		siginitsetinv(&current->blocked, mask);
-		recalc_sigpending();
+
+		siginitsetinv(&blocked, mask);
+		__set_task_blocked(current, &blocked);
 		spin_unlock_irqrestore(&current->sighand->siglock, flags);
 		
 		result = do_ncp_rpc_call(server, size, reply, max_reply_size);
 
-		spin_lock_irqsave(&current->sighand->siglock, flags);
-		current->blocked = old_set;
-		recalc_sigpending();
-		spin_unlock_irqrestore(&current->sighand->siglock, flags);
+		set_current_blocked(&old_set);
 	}
 
 	DDPRINTK("do_ncp_rpc_call returned %d\n", result);
-- 
1.7.4.4

[PATCH 41/41] exit: Use __set_task_blocked()

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

As described in e6fa16ab ("signal: sigprocmask() should do
retarget_shared_pending()") the modification of current->blocked is
incorrect as we need to check whether the signal we're about to block
is pending in the shared queue.

Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Tejun Heo <tj@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 kernel/exit.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index 2913b35..a47c0f3 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -369,19 +369,22 @@ static void set_special_pids(struct pid *pid)
  */
 int allow_signal(int sig)
 {
+	sigset_t blocked;
+
 	if (!valid_signal(sig) || sig < 1)
 		return -EINVAL;
 
 	spin_lock_irq(&current->sighand->siglock);
 	/* This is only needed for daemonize()'ed kthreads */
-	sigdelset(&current->blocked, sig);
+	siginitset(&blocked, current->blocked);
+	sigdelset(&blocked, sig);
 	/*
 	 * Kernel threads handle their own signals. Let the signal code
 	 * know it'll be handled, so that they don't get converted to
 	 * SIGKILL or just silently dropped.
 	 */
 	current->sighand->action[(sig)-1].sa.sa_handler = (void __user *)2;
-	recalc_sigpending();
+	__set_task_blocked(current, &blocked);
 	spin_unlock_irq(&current->sighand->siglock);
 	return 0;
 }
-- 
1.7.4.4

Re: [PATCH 39/41] dlm: Remove another superfluous call to recalc_sigpending()

[David Teigland]

On Thu, Aug 11, 2011 at 02:57:13PM +0100, Matt Fleming wrote:
> From: Matt Fleming <matt.fleming@intel.com>
> 
> recalc_sigpending() is called within sigprocmask(), so there is no
> need call it again after sigprocmask() has returned. I must have
> missed this call when removing the other recalc_sigpending() in commit
> 4bcad6c1ef53 ("dlm: Remove superfluous call to recalc_sigpending())".

ack

Re: [PATCH 00/41] signal: Use set_current_blocked()

[Oleg Nesterov]

Hi Matt,

On 08/11, Matt Fleming wrote:
>
> I did a tree wide scan of all the code that modifies current->blocked
> without using the set_current_blocked() accessor. This patch series is
> the result of the conversion to the new accessor functions.

Yes, this is what I was going to do, but didn't find time ;)

Thanks, everything looks "obviously fine" at first glance. except
ncpfs does something mysterious (and afaics wrong) with or without
the patch.

I'll try to read this carefully on weekend though. I do not expect I'll
find any problem, just may be a couple of nits about the changelogs.
Say, 41/41... doesn't hurt, but in fact the current code is fine as is.

Oleg.

Re: [PATCH 02/41] arm: Use set_current_blocked()

[Will Deacon]

Hi Matt,

On Thu, Aug 11, 2011 at 02:56:36PM +0100, Matt Fleming wrote:
> From: Matt Fleming <matt.fleming@linux.intel.com>
> 
> As described in e6fa16ab ("signal: sigprocmask() should do
> retarget_shared_pending()") the modification of current->blocked is
> incorrect as we need to check for shared signals we're about to block.
> 
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Arnd Bergmann <arnd.bergmann@linaro.org>
> Cc: Dave Martin <dave.martin@linaro.org>
> Cc: Nicolas Pitre <nicolas.pitre@linaro.org>
> CC: Will Deacon <will.deacon@arm.com>
> Signed-off-by: Matt Fleming <matt.fleming@linux.intel.com>
> ---
>  arch/arm/kernel/signal.c |   25 +++++++++++--------------
>  1 files changed, 11 insertions(+), 14 deletions(-)

Looks good to me.

Acked-by: Will Deacon <will.deacon@arm.com>

Cheers,

Will

Re: [PATCH 29/41] tile: Use set_current_blocked()

[Chris Metcalf]

On 8/11/2011 9:57 AM, Matt Fleming wrote:
> From: Matt Fleming <matt.fleming@intel.com>
>
> As described in e6fa16ab ("signal: sigprocmask() should do
> retarget_shared_pending()") the modification of current->blocked is
> incorrect as we need to check whether the signal we're about to block
> is pending in the shared queue.
>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Chris Metcalf <cmetcalf@tilera.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Matt Fleming <matt.fleming@intel.com>
> ---
>  arch/tile/kernel/compat_signal.c |    5 +----
>  arch/tile/kernel/signal.c        |   16 ++++++----------
>  2 files changed, 7 insertions(+), 14 deletions(-)

Acked-by: Chris Metcalf <cmetcalf@tilera.com>

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

Re: [PATCH 18/41] OpenRISC: Don't reimplement force_sigsegv()

[Oleg Nesterov]

On 08/11, Matt Fleming wrote:
>
> Instead of open coding the sequence from force_sigsegv() just call
> it. This also fixes a race because sa_handler was being modified
> without holding ->sighand->siglock.
>
> --- a/arch/openrisc/kernel/signal.c
> +++ b/arch/openrisc/kernel/signal.c
> @@ -257,9 +257,7 @@ static void setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
>  	return;
>
>  give_sigsegv:
> -	if (sig == SIGSEGV)
> -		ka->sa.sa_handler = SIG_DFL;
> -	force_sig(SIGSEGV, current);
> +	force_sigsegv(sig, current);
>  }

Agreed, but...

I don't really understand the changelog, which race this patch fix?

Yes, we shouldn't change sa_handler lockless, this "breaks the rules"
but I do not see any immediate problem. And since force_sigsegv() drops
the lock after setting SIG_DFL we can "race" with the sub-thread anyway.


Hmm. Looking more, I think that this patch is not the cleanup, but the
bugfix. The current code is simply wrong, it plays with ka, and it points
to the _copy_ of sighand->action[], so this code is simply pointless.


Unless I missed something, could you fix the changelog and resend?

Oleg.

Re: [PATCH 19/41] OpenRISC: No need to reset handler if SA_ONESHOT

[Oleg Nesterov]

On 08/11, Matt Fleming wrote:
>
> get_signal_to_deliver() already resets the signal handler if
> SA_ONESHOT is set in ka->sa.sa_flags,

Yes.

> there's no need to do it again
> in handle_signal().

Again, this code in fact does nothing, it plays with the copy.

> Furthermore, because we were modifying
> ka->sa.sa_handler without holding ->sighand->siglock there's the
> potential we can race with another thread if it modifies the signal
> handlers.

and thus this is not true.



Afaics, the changelog should be fixed but the patch is fine ;)

Oleg.

Re: [PATCH 12/41] microblaze: Don't reimplement force_sigsegv()

[Oleg Nesterov]

On 08/11, Matt Fleming wrote:
>
> Instead of open coding the sequence from force_sigsegv() just call
> it. This also fixes a race because sa_handler was being modified
> without holding ->sighand->siglock.
> 
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Michal Simek <monstr@monstr.eu>
> Signed-off-by: Matt Fleming <matt.fleming@intel.com>
> ---
>  arch/microblaze/kernel/signal.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/microblaze/kernel/signal.c b/arch/microblaze/kernel/signal.c
> index 5996711..90de06d 100644
> --- a/arch/microblaze/kernel/signal.c
> +++ b/arch/microblaze/kernel/signal.c
> @@ -270,9 +270,7 @@ static void setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
>  	return;
>  
>  give_sigsegv:
> -	if (sig == SIGSEGV)
> -		ka->sa.sa_handler = SIG_DFL;
> -	force_sig(SIGSEGV, current);
> +	force_sigsegv(sig, current);
>  }

This and the next patch seem to have the same problems with the
changelogs as 18 and 19.

13-14 looks correct to me...

Oleg.

Re: [PATCH 12/41] microblaze: Don't reimplement force_sigsegv()

[Oleg Nesterov]

On 08/16, Oleg Nesterov wrote:
>
> This and the next patch seem to have the same problems with the
> changelogs as 18 and 19.
>
> 13-14 looks correct to me...

I meant 14-15, sorry for noise

Oleg.

Re: [PATCH 26/41] sh: No need to reset handler if SA_ONESHOT

[Oleg Nesterov]

On 08/11, Matt Fleming wrote:
>
> get_signal_to_deliver() already resets the signal handler if
> SA_ONESHOT is set in ka->sa.sa_flags, there's no need to do it again
> in handle_signal(). Furthermore, because we were modifying
> ka->sa.sa_handler without holding ->sighand->siglock there's the
> potential we can race with another thread if it modifies the signal
> handlers.

Again ;) the patch is fine, the changelog is not right.

Oleg.

Re: [PATCH 31/41] um: Don't restore current->blocked on error

[Oleg Nesterov]

On 08/11, Matt Fleming wrote:
>
> If we fail to setup the signal stack frame then we don't need to
> restore current->blocked because it is not modified by
> setup_signal_stack_*.

Yes. And this is simply wrong if TIF_RESTORE_SIGMASK. We can block
SIGSEGV which we need to dequeue.

ACK

> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Richard Weinberger <richard@nod.at>
> Signed-off-by: Matt Fleming <matt.fleming@intel.com>
> ---
>  arch/um/kernel/signal.c |    8 ++------
>  1 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/um/kernel/signal.c b/arch/um/kernel/signal.c
> index 1812524..d177685 100644
> --- a/arch/um/kernel/signal.c
> +++ b/arch/um/kernel/signal.c
> @@ -66,13 +66,9 @@ static int handle_signal(struct pt_regs *regs, unsigned long signr,
>  #endif
>  		err = setup_signal_stack_si(sp, signr, ka, regs, info, oldset);
>  
> -	if (err) {
> -		spin_lock_irq(&current->sighand->siglock);
> -		current->blocked = *oldset;
> -		recalc_sigpending();
> -		spin_unlock_irq(&current->sighand->siglock);
> +	if (err)
>  		force_sigsegv(signr, current);
> -	} else {
> +	else {
>  		sigset_t blocked:
>  
>  		sigorsets(&blocked, &current->blocked, &ka->sa.sa_mask);
> -- 
> 1.7.4.4
> 

Re: [PATCH 33/41] xtensa: Don't reimplement force_sigsegv()

[Oleg Nesterov]

On 08/11, Matt Fleming wrote:
>
> Instead of open coding the sequence from force_sigsegv() just call
> it. This also fixes a race because sa_handler was being modified
> without holding ->sighand->siglock.

The same problem with the changelog in this and the next patch.

The patch itself looks nice.

> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Chris Zankel <chris@zankel.net>
> Signed-off-by: Matt Fleming <matt.fleming@intel.com>
> ---
>  arch/xtensa/kernel/signal.c |    4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/xtensa/kernel/signal.c b/arch/xtensa/kernel/signal.c
> index f2220b5..4f53770 100644
> --- a/arch/xtensa/kernel/signal.c
> +++ b/arch/xtensa/kernel/signal.c
> @@ -425,9 +425,7 @@ static void setup_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
>  	return;
>  
>  give_sigsegv:
> -	if (sig == SIGSEGV)
> -		ka->sa.sa_handler = SIG_DFL;
> -	force_sig(SIGSEGV, current);
> +	force_sigsegv(sig, current);
>  }
>  
>  /*
> -- 
> 1.7.4.4
> 

Re: [PATCH 37/41] autofs4: Use set_current_blocked()

[Oleg Nesterov]

On 08/11, Matt Fleming wrote:
>
> As described in e6fa16ab ("signal: sigprocmask() should do
> retarget_shared_pending()") the modification of current->blocked is
> incorrect as we need to check whether the signal we're about to block
> is pending in the shared queue.

ACK.


cough, can't resist...

> @@ -458,21 +458,16 @@ int autofs4_wait(struct autofs_sb_info *sbi, struct dentry *dentry,
>  	 */
>  	if (wq->name.name) {
>  		/* Block all but "shutdown" signals while waiting */
> -		sigset_t oldset;
> +		sigset_t oldset, blocked;
>  		unsigned long irqflags;
>  
> -		spin_lock_irqsave(&current->sighand->siglock, irqflags);
>  		oldset = current->blocked;
> -		siginitsetinv(&current->blocked, SHUTDOWN_SIGS & ~oldset.sig[0]);
> -		recalc_sigpending();
> -		spin_unlock_irqrestore(&current->sighand->siglock, irqflags);
> +		siginitsetinv(&blocked, SHUTDOWN_SIGS & ~oldset.sig[0]);
> +		set_current_blocked(&blocked);
>  
>  		wait_event_interruptible(wq->queue, wq->name.name == NULL);
> -
> -		spin_lock_irqsave(&current->sighand->siglock, irqflags);
> -		current->blocked = oldset;
> -		recalc_sigpending();
> -		spin_unlock_irqrestore(&current->sighand->siglock, irqflags);
> +		

this adds the trailing whitespaces ;)

Oleg.

Re: [PATCH 40/41] ncpfs: Use set_current_blocked()

[Oleg Nesterov]

On 08/11, Matt Fleming wrote:
>
> As described in e6fa16ab ("signal: sigprocmask() should do
> retarget_shared_pending()") the modification of current->blocked is
> incorrect as we need to check whether the signal we're about to block
> is pending in the shared queue.

I'd wish I could understand this code but this seems impossible ;)
IOW, "This doesn't seem right at all." looks reasonable, and the
PF_EXITING adds even more confusion.

As for this patch, it looks (almost) fine anyway. But,

> @@ -749,7 +749,7 @@ static int ncp_do_request(struct ncp_server *server, int size,
>  		return -EIO;
>  	}
>  	{
> -		sigset_t old_set;
> +		sigset_t old_set, blocked;
>  		unsigned long mask, flags;
>  
>  		spin_lock_irqsave(&current->sighand->siglock, flags);
> @@ -769,16 +769,14 @@ static int ncp_do_request(struct ncp_server *server, int size,
>  			if (current->sighand->action[SIGQUIT - 1].sa.sa_handler == SIG_DFL)
>  				mask |= sigmask(SIGQUIT);
>  		}
> -		siginitsetinv(&current->blocked, mask);
> -		recalc_sigpending();
> +
> +		siginitsetinv(&blocked, mask);
> +		__set_task_blocked(current, &blocked);
>  		spin_unlock_irqrestore(&current->sighand->siglock, flags);

Why do we take ->siglock in the first place?

I think it is not needed. We can calculate mask/blocked lockless and
use set_task_blocked(). This also makes sense because __set_task_blocked
is not exported ;)

the sighand->action[] checks are racy anyway in the mt case, siglock
can't help.

Oleg.

Re: [PATCH 41/41] exit: Use __set_task_blocked()

[Oleg Nesterov]

On 08/11, Matt Fleming wrote:
>
> As described in e6fa16ab ("signal: sigprocmask() should do
> retarget_shared_pending()") the modification of current->blocked is
> incorrect as we need to check whether the signal we're about to block
> is pending in the shared queue.

Yes. but in this case the code is correct. First of all, unblocking
is always fine. Also, this should be used by kthreads only, and they
are single-threaded.

But there are other reasons why I don't like this change, even if we
ignore the fact the patched exit.c can't be compiled (__set_task_blocked
is not exported ;).

>  int allow_signal(int sig)
>  {
> +	sigset_t blocked;
> +
>  	if (!valid_signal(sig) || sig < 1)
>  		return -EINVAL;
>
>  	spin_lock_irq(&current->sighand->siglock);
>  	/* This is only needed for daemonize()'ed kthreads */

Exactly. And nowadays the daemonize()'ed kthreads should not play
with allow_signal().

And more, I think it is the time to kill daemonize(). Contrary to
what /bin/grep thinks, it has only one user.

In short: this code in allow_signal() should die. I'll try to send
the patches soon...

Oleg.

Re: [PATCH 18/41] OpenRISC: Don't reimplement force_sigsegv()

[Matt Fleming]

On Tue, 2011-08-16 at 18:49 +0200, Oleg Nesterov wrote:
> On 08/11, Matt Fleming wrote:
> >
> > Instead of open coding the sequence from force_sigsegv() just call
> > it. This also fixes a race because sa_handler was being modified
> > without holding ->sighand->siglock.
> >
> > --- a/arch/openrisc/kernel/signal.c
> > +++ b/arch/openrisc/kernel/signal.c
> > @@ -257,9 +257,7 @@ static void setup_rt_frame(int sig, struct k_sigaction *ka, siginfo_t *info,
> >  	return;
> >
> >  give_sigsegv:
> > -	if (sig == SIGSEGV)
> > -		ka->sa.sa_handler = SIG_DFL;
> > -	force_sig(SIGSEGV, current);
> > +	force_sigsegv(sig, current);
> >  }
> 
> Agreed, but...
> 
> I don't really understand the changelog, which race this patch fix?
> 
> Yes, we shouldn't change sa_handler lockless, this "breaks the rules"
> but I do not see any immediate problem. And since force_sigsegv() drops
> the lock after setting SIG_DFL we can "race" with the sub-thread anyway.

Argh, yeah this and the rest of patches that try to fixup 'ka' are
wrong, because like you say, we're operating on a copy on the stack, so
there's no race. I missed that :-(

I did notice that race in force_sigsegv() too, is it a real problem? It
certainly looked to me like it could be a real problem.

> Hmm. Looking more, I think that this patch is not the cleanup, but the
> bugfix. The current code is simply wrong, it plays with ka, and it points
> to the _copy_ of sighand->action[], so this code is simply pointless.

Yeah, I will make that more explicit in the changelog.

> Unless I missed something, could you fix the changelog and resend?

Nope you haven't missed anything, you're spot on. I will resend.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 39/41] dlm: Remove another superfluous call to recalc_sigpending()

[Oleg Nesterov]

On 08/11, Matt Fleming wrote:
>
> recalc_sigpending() is called within sigprocmask(), so there is no
> need call it again after sigprocmask() has returned.

And more, this is simply wrong. This can race with signal_wake_up().

ACK. but may me you can update the changelog although I won't insist.

Oleg.

Re: [PATCH 00/41] signal: Use set_current_blocked()

[Matt Fleming]

On Thu, 2011-08-11 at 18:03 +0200, Oleg Nesterov wrote:
> 
> I'll try to read this carefully on weekend though. I do not expect I'll
> find any problem, just may be a couple of nits about the changelogs.
> Say, 41/41... doesn't hurt, but in fact the current code is fine as is.

Thanks for the review Oleg! Did you want me to send just the updated
versions of patches or the entire series again? It would seem over the
top to send the entire series.

Also, are you going to pull these patches into your tree and send them
to Linus or would it be better to route them through -mm?

-- 
Matt Fleming, Intel Open Source Technology Center

[PATCH 0/1] kthreads: allow_signal: don't play with ->blocked

[Oleg Nesterov]

On 08/16, Oleg Nesterov wrote:
>
> >  int allow_signal(int sig)
> >  {
> > +	sigset_t blocked;
> > +
> >  	if (!valid_signal(sig) || sig < 1)
> >  		return -EINVAL;
> >
> >  	spin_lock_irq(&current->sighand->siglock);
> >  	/* This is only needed for daemonize()'ed kthreads */
>
> Exactly. And nowadays the daemonize()'ed kthreads should not play
> with allow_signal().
>
> And more, I think it is the time to kill daemonize(). Contrary to
> what /bin/grep thinks, it has only one user.

No, daemonize() has no users which actually need it.

Imho we should deprecate it, it has no users after the patches I sent.

> In short: this code in allow_signal() should die.

Yes.

To clarify: this change breaks drivers/staging/rtl8712/, but I belive
it is wrong and should be fixed. I have already sent the patch.

Oleg.

[PATCH 1/1] kthreads: allow_signal: don't play with ->blocked

[Oleg Nesterov]

allow_signal(sig) unblocks the signal. This was only needed because
we had the daemonize()'ed kthreads playing with signals. And daemonize()
can't use ignore_signals() but does sigprocmask(SIG_BLOCK) because it
was used after kernel_thread(CLONE_SIGHAND).

Nobody does this any longer, we can remove this hack. And hopefully
we can deprecate daemonize() soon, all current users do not actually
need it.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---

 kernel/exit.c |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

--- 3.1/kernel/exit.c~4_allow_signal_dont_unblock	2011-08-16 20:14:20.000000000 +0200
+++ 3.1/kernel/exit.c	2011-08-16 21:20:11.000000000 +0200
@@ -365,7 +365,8 @@ static void set_special_pids(struct pid 
 
 /*
  * Let kernel threads use this to say that they allow a certain signal.
- * Must not be used if kthread was cloned with CLONE_SIGHAND.
+ * Must not be used if kthread was cloned with CLONE_SIGHAND or daemonize()
+ * was called.
  */
 int allow_signal(int sig)
 {
@@ -373,8 +374,6 @@ int allow_signal(int sig)
 		return -EINVAL;
 
 	spin_lock_irq(&current->sighand->siglock);
-	/* This is only needed for daemonize()'ed kthreads */
-	sigdelset(&current->blocked, sig);
 	/*
 	 * Kernel threads handle their own signals. Let the signal code
 	 * know it'll be handled, so that they don't get converted to

Re: [PATCH 1/1] kthreads: allow_signal: don't play with ->blocked

[Tejun Heo]

Hello, Oleg.

On Tue, Aug 16, 2011 at 09:44:50PM +0200, Oleg Nesterov wrote:
> allow_signal(sig) unblocks the signal. This was only needed because
> we had the daemonize()'ed kthreads playing with signals. And daemonize()
> can't use ignore_signals() but does sigprocmask(SIG_BLOCK) because it
> was used after kernel_thread(CLONE_SIGHAND).
> 
> Nobody does this any longer, we can remove this hack. And hopefully
> we can deprecate daemonize() soon, all current users do not actually
> need it.
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>

I agree with the patchset but given that daemonize() isn't all that
popular and you already posted most (or was it all?) conversions,
wouldn't it be better to do this in a single patchset?  ie. Convert
all daemonize() users, kill daemonize(), and drop the hack from
allow_signal().

Thanks.

-- 
tejun

Re: [PATCH 00/41] signal: Use set_current_blocked()

[Oleg Nesterov]

On 08/11, Matt Fleming wrote:
>
> I did a tree wide scan of all the code that modifies current->blocked
> without using the set_current_blocked() accessor. This patch series is
> the result of the conversion to the new accessor functions.

Thanks Matt.

The whole series looks fine, except the nits I sent. Still I hope the
maintainers can recheck.

One more nit... Every handle_signal() in arch/ does
current->blocked |= ka->sa.sa_mask plus SA_NODEFER code. May be we
should start with the patch below?

The only problem is, I can't invent the good name for this helper.
Also, in this case you need to rediff a lot of patches in this
series, so I won't insist.

Oleg.


--- x/arch/x86/kernel/signal.c
+++ x/arch/x86/kernel/signal.c
@@ -678,11 +678,20 @@ setup_rt_frame(int sig, struct k_sigacti
 	return ret;
 }
 
+void xxx(struct k_sigaction *ka)
+{
+	sigset_t blocked;
+
+	sigorsets(&blocked, &current->blocked, &ka->sa.sa_mask);
+	if (!(ka->sa.sa_flags & SA_NODEFER))
+		sigaddset(&blocked, sig);
+	set_current_blocked(&blocked);
+}
+
 static int
 handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka,
 		struct pt_regs *regs)
 {
-	sigset_t blocked;
 	int ret;
 
 	/* Are we from a system call? */
@@ -733,10 +742,7 @@ handle_signal(unsigned long sig, siginfo
 	 */
 	regs->flags &= ~X86_EFLAGS_TF;
 
-	sigorsets(&blocked, &current->blocked, &ka->sa.sa_mask);
-	if (!(ka->sa.sa_flags & SA_NODEFER))
-		sigaddset(&blocked, sig);
-	set_current_blocked(&blocked);
+	xxx(ka);
 
 	tracehook_signal_handler(sig, info, ka, regs,
 				 test_thread_flag(TIF_SINGLESTEP));

Re: [PATCH 1/1] kthreads: allow_signal: don't play with ->blocked

[Oleg Nesterov]

On 08/16, Tejun Heo wrote:
>
> Hello, Oleg.
>
> On Tue, Aug 16, 2011 at 09:44:50PM +0200, Oleg Nesterov wrote:
> > allow_signal(sig) unblocks the signal. This was only needed because
> > we had the daemonize()'ed kthreads playing with signals. And daemonize()
> > can't use ignore_signals() but does sigprocmask(SIG_BLOCK) because it
> > was used after kernel_thread(CLONE_SIGHAND).
> >
> > Nobody does this any longer, we can remove this hack. And hopefully
> > we can deprecate daemonize() soon, all current users do not actually
> > need it.
> >
> > Signed-off-by: Oleg Nesterov <oleg@redhat.com>
>
> I agree with the patchset but given that daemonize() isn't all that
> popular and you already posted most (or was it all?) conversions,
> wouldn't it be better to do this in a single patchset?  ie. Convert
> all daemonize() users, kill daemonize(), and drop the hack from
> allow_signal().

May be... But please note that there are too different things.

daemonize() should be deprecated (imho), but this is a bit off-topic.
I think that a daemonize()'ed kthread should not play with allow_signal()
anyway. And nobody does, except rtl8712 which should be fixed afaics.

So this code is already unneeded, but looks confusing.



Tejun, Matt, I am sorry. I have to run away, I'll reply to other
emails tomorrow.

Thanks for review!

Oleg.

Re: [PATCH 37/41] autofs4: Use set_current_blocked()

[Matt Fleming]

On Tue, 2011-08-16 at 19:47 +0200, Oleg Nesterov wrote:
> On 08/11, Matt Fleming wrote:
> >
> > As described in e6fa16ab ("signal: sigprocmask() should do
> > retarget_shared_pending()") the modification of current->blocked is
> > incorrect as we need to check whether the signal we're about to block
> > is pending in the shared queue.
> 
> ACK.
> 
> 
> cough, can't resist...
> 
> > @@ -458,21 +458,16 @@ int autofs4_wait(struct autofs_sb_info *sbi, struct dentry *dentry,
> >  	 */
> >  	if (wq->name.name) {
> >  		/* Block all but "shutdown" signals while waiting */
> > -		sigset_t oldset;
> > +		sigset_t oldset, blocked;
> >  		unsigned long irqflags;
> >  
> > -		spin_lock_irqsave(&current->sighand->siglock, irqflags);
> >  		oldset = current->blocked;
> > -		siginitsetinv(&current->blocked, SHUTDOWN_SIGS & ~oldset.sig[0]);
> > -		recalc_sigpending();
> > -		spin_unlock_irqrestore(&current->sighand->siglock, irqflags);
> > +		siginitsetinv(&blocked, SHUTDOWN_SIGS & ~oldset.sig[0]);
> > +		set_current_blocked(&blocked);
> >  
> >  		wait_event_interruptible(wq->queue, wq->name.name == NULL);
> > -
> > -		spin_lock_irqsave(&current->sighand->siglock, irqflags);
> > -		current->blocked = oldset;
> > -		recalc_sigpending();
> > -		spin_unlock_irqrestore(&current->sighand->siglock, irqflags);
> > +		
> 
> this adds the trailing whitespaces ;)

Whoops! I fixed that and added your Acked-by. Thanks!

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 40/41] ncpfs: Use set_current_blocked()

[Matt Fleming]

On Tue, 2011-08-16 at 19:56 +0200, Oleg Nesterov wrote:
> On 08/11, Matt Fleming wrote:
> >
> > As described in e6fa16ab ("signal: sigprocmask() should do
> > retarget_shared_pending()") the modification of current->blocked is
> > incorrect as we need to check whether the signal we're about to block
> > is pending in the shared queue.
> 
> I'd wish I could understand this code but this seems impossible ;)

Yeah, I gave up after staring at it for about twenty minutes. I couldn't
fathom the logic behind it.

> IOW, "This doesn't seem right at all." looks reasonable, and the
> PF_EXITING adds even more confusion.

Definitely. If I was more confident in this area of the kernel I would
have just deleted it ;-)

In fact, the more I stare at it, the more I think it needs removing.
Because the thread doesn't hold ->siglock over do_ncp_rpc_call() another
thread could change the signal handler for SIGINT or SIGQUIT mid-call.
Which makes the code under "if (server->m.flags & NCP_MOUNT_INTR)"
pointless.

Petr, Al, Arnd? Could one of you hit me with a clue bat?

> As for this patch, it looks (almost) fine anyway. But,
> 
> > @@ -749,7 +749,7 @@ static int ncp_do_request(struct ncp_server *server, int size,
> >  		return -EIO;
> >  	}
> >  	{
> > -		sigset_t old_set;
> > +		sigset_t old_set, blocked;
> >  		unsigned long mask, flags;
> >  
> >  		spin_lock_irqsave(&current->sighand->siglock, flags);
> > @@ -769,16 +769,14 @@ static int ncp_do_request(struct ncp_server *server, int size,
> >  			if (current->sighand->action[SIGQUIT - 1].sa.sa_handler == SIG_DFL)
> >  				mask |= sigmask(SIGQUIT);
> >  		}
> > -		siginitsetinv(&current->blocked, mask);
> > -		recalc_sigpending();
> > +
> > +		siginitsetinv(&blocked, mask);
> > +		__set_task_blocked(current, &blocked);
> >  		spin_unlock_irqrestore(&current->sighand->siglock, flags);
> 
> Why do we take ->siglock in the first place?
> 
> I think it is not needed. We can calculate mask/blocked lockless and
> use set_task_blocked(). This also makes sense because __set_task_blocked
> is not exported ;)

Eek! Sorry, I didn't realise this didn't compile.

> the sighand->action[] checks are racy anyway in the mt case, siglock
> can't help.

Hmm.. really? I thought that ->siglock serialised modifications to
sighand->action[] even in the mt case, no? This was the only reason that
I left the sighand locking around in this function.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 1/1] kthreads: allow_signal: don't play with ->blocked

[Matt Fleming]

On Tue, 2011-08-16 at 21:51 +0200, Tejun Heo wrote:
> 
> I agree with the patchset but given that daemonize() isn't all that
> popular and you already posted most (or was it all?) conversions,
> wouldn't it be better to do this in a single patchset?  ie. Convert
> all daemonize() users, kill daemonize(), and drop the hack from
> allow_signal().

But because daemonize() is exported by the kernel should it go through
the Documentation/feature-removal-schedule.txt procedure? And if so, can
the allow_signal() patch still go in before daemonize() is removed?

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 41/41] exit: Use __set_task_blocked()

[Matt Fleming]

On Tue, 2011-08-16 at 20:06 +0200, Oleg Nesterov wrote:
> On 08/11, Matt Fleming wrote:
> >
> > As described in e6fa16ab ("signal: sigprocmask() should do
> > retarget_shared_pending()") the modification of current->blocked is
> > incorrect as we need to check whether the signal we're about to block
> > is pending in the shared queue.
> 
> Yes. but in this case the code is correct. First of all, unblocking
> is always fine. Also, this should be used by kthreads only, and they
> are single-threaded.
> 
> But there are other reasons why I don't like this change, even if we
> ignore the fact the patched exit.c can't be compiled (__set_task_blocked
> is not exported ;).

OK, I've dropped this patch from the series.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 1/1] kthreads: allow_signal: don't play with ->blocked

[Tejun Heo]

Hello,

On Tue, Aug 16, 2011 at 10:50:22PM +0100, Matt Fleming wrote:
> On Tue, 2011-08-16 at 21:51 +0200, Tejun Heo wrote:
> > I agree with the patchset but given that daemonize() isn't all that
> > popular and you already posted most (or was it all?) conversions,
> > wouldn't it be better to do this in a single patchset?  ie. Convert
> > all daemonize() users, kill daemonize(), and drop the hack from
> > allow_signal().
> 
> But because daemonize() is exported by the kernel should it go through
> the Documentation/feature-removal-schedule.txt procedure? And if so, can
> the allow_signal() patch still go in before daemonize() is removed?

IMHO, not really.  APIs get modified and dropped all the time and only
small fraction goes through feature-removal-schedule.  For APIs which
are widely used and/or difficult to migrate from, it sure makes sense
to do the staged removal but in this case it's an interface which is
quite unpopular and with relatively easy workaround (just use
kthread).

The worst thing we can do regarding API change is silently changing
semantics while not changing the interface.  For this patchset I don't
think it would matter all that much but is going that route.
ie. allow_signal() behavior is proposed to be changed because
in-kernel daemonize() users don't depend on it while leaving
daemonize() alone.  This is much worse than simply removing
daemonize() with sufficient explanation in the commit message.
Out-of-kernel user which depended on the combination working would now
be left with code which compiles fine but behaves differently, which
sucks big time.

These changes _are_ related and interdependent, and routing these
small changes through different trees often end up delaying things
unnecessarily.  One subsystem maintainer forgets to apply a patch or
send pull request and it can get easily drawn out half a year and
people forget what the original change was about after a while often
leading to half done conversions.  So, let's please collect all the
related patches into one series, drop all in-kernel daemonize() users,
kill daemonize() and then change allow_signal() behavior.

Thanks.

-- 
tejun

Re: [PATCH 1/1] kthreads: allow_signal: don't play with ->blocked

[Matt Fleming]

On Wed, 2011-08-17 at 09:27 +0200, Tejun Heo wrote:
> Hello,
> 
> On Tue, Aug 16, 2011 at 10:50:22PM +0100, Matt Fleming wrote:
> > 
> > But because daemonize() is exported by the kernel should it go through
> > the Documentation/feature-removal-schedule.txt procedure? And if so, can
> > the allow_signal() patch still go in before daemonize() is removed?
> 
> IMHO, not really.  APIs get modified and dropped all the time and only
> small fraction goes through feature-removal-schedule.  For APIs which
> are widely used and/or difficult to migrate from, it sure makes sense
> to do the staged removal but in this case it's an interface which is
> quite unpopular and with relatively easy workaround (just use
> kthread).
> 
> The worst thing we can do regarding API change is silently changing
> semantics while not changing the interface.  For this patchset I don't
> think it would matter all that much but is going that route.
> ie. allow_signal() behavior is proposed to be changed because
> in-kernel daemonize() users don't depend on it while leaving
> daemonize() alone.  This is much worse than simply removing
> daemonize() with sufficient explanation in the commit message.
> Out-of-kernel user which depended on the combination working would now
> be left with code which compiles fine but behaves differently, which
> sucks big time.
> 
> These changes _are_ related and interdependent, and routing these
> small changes through different trees often end up delaying things
> unnecessarily.  One subsystem maintainer forgets to apply a patch or
> send pull request and it can get easily drawn out half a year and
> people forget what the original change was about after a while often
> leading to half done conversions.  So, let's please collect all the
> related patches into one series, drop all in-kernel daemonize() users,
> kill daemonize() and then change allow_signal() behavior.

OK, that makes a lot of sense to me. Thanks for the rationale.

Oleg, feel to carry over my Acked-by's for the patches you've already
posted if you decide to go ahead with Tejun's plan.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 40/41] ncpfs: Use set_current_blocked()

[Oleg Nesterov]

On 08/16, Matt Fleming wrote:
>
> On Tue, 2011-08-16 at 19:56 +0200, Oleg Nesterov wrote:
> > On 08/11, Matt Fleming wrote:
> > >
> > > As described in e6fa16ab ("signal: sigprocmask() should do
> > > retarget_shared_pending()") the modification of current->blocked is
> > > incorrect as we need to check whether the signal we're about to block
> > > is pending in the shared queue.
> >
> > I'd wish I could understand this code but this seems impossible ;)
>
> Yeah, I gave up after staring at it for about twenty minutes. I couldn't
> fathom the logic behind it.
>
> > IOW, "This doesn't seem right at all." looks reasonable, and the
> > PF_EXITING adds even more confusion.
>
> Definitely. If I was more confident in this area of the kernel I would
> have just deleted it ;-)

Same here ;)

> Because the thread doesn't hold ->siglock over do_ncp_rpc_call() another
> thread could change the signal handler for SIGINT or SIGQUIT mid-call.
> Which makes the code under "if (server->m.flags & NCP_MOUNT_INTR)"
> pointless.

(indeed, and see below)

> > Why do we take ->siglock in the first place?
> >
> > I think it is not needed. We can calculate mask/blocked lockless and
> > use set_task_blocked(). This also makes sense because __set_task_blocked
> > is not exported ;)
>
> Eek! Sorry, I didn't realise this didn't compile.
>
> > the sighand->action[] checks are racy anyway in the mt case, siglock
> > can't help.
>
> Hmm.. really? I thought that ->siglock serialised modifications to
> sighand->action[] even in the mt case, no?

Sure. But another thread can change sighand->action[] right after we
drop ->siglock. So how can this lock help? We simply read the word,
this is atomic and doesn't need the locking.

Oleg.

Re: [PATCH 12/41] microblaze: Don't reimplement force_sigsegv()

[Michal Simek]

Oleg Nesterov wrote:
> On 08/16, Oleg Nesterov wrote:
>> This and the next patch seem to have the same problems with the
>> changelogs as 18 and 19.
>>
>> 13-14 looks correct to me...
> 
> I meant 14-15, sorry for noise

Do you mean that there is
From: Matt Fleming <matt.fleming@intel.com> ?

Anyway: I have looked at patches 12-15 and they look good to me.

Fix that "From:" line and you can add my
Acked-by: Michal Simek <monstr@monstr.eu>

If you want to propose 12-15 through my Microblaze tree, please let me know.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: [PATCH 12/41] microblaze: Don't reimplement force_sigsegv()

[Oleg Nesterov]

On 08/17, Michal Simek wrote:
>
> Oleg Nesterov wrote:
>> On 08/16, Oleg Nesterov wrote:
>>> This and the next patch seem to have the same problems with the
>>> changelogs as 18 and 19.
>>>
>>> 13-14 looks correct to me...
>>
>> I meant 14-15, sorry for noise
>
> Do you mean that there is
> From: Matt Fleming <matt.fleming@intel.com> ?

Confused... "From" looks correct to me.

The changelog is wrong and should be updated (Matt seems to agree).
This patch fixes the bug, this should be explained. The problem is,
ka points to the copy of sighand->action, the current code is pointless.

> Anyway: I have looked at patches 12-15 and they look good to me.
>
> Acked-by: Michal Simek <monstr@monstr.eu>

Assuming the changelog is fixed,

	Acked-by: Oleg Nesterov <oleg@redhat.com>

> If you want to propose 12-15 through my Microblaze tree, please let me know.

Probably this is the best option. Matt, what do you think?

Oleg.

Re: [PATCH 12/41] microblaze: Don't reimplement force_sigsegv()

[Matt Fleming]

On Wed, 2011-08-17 at 15:01 +0200, Michal Simek wrote:
> Oleg Nesterov wrote:
> > On 08/16, Oleg Nesterov wrote:
> >> This and the next patch seem to have the same problems with the
> >> changelogs as 18 and 19.
> >>
> >> 13-14 looks correct to me...
> > 
> > I meant 14-15, sorry for noise
> 
> Do you mean that there is
> From: Matt Fleming <matt.fleming@intel.com> ?
> 
> Anyway: I have looked at patches 12-15 and they look good to me.
> 
> Fix that "From:" line and you can add my

What's wrong with the From: line? That should be fine. Oleg was saying
that the description in the changelog needed updating.

> Acked-by: Michal Simek <monstr@monstr.eu>

Thanks.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 12/41] microblaze: Don't reimplement force_sigsegv()

[Michal Simek]

Matt Fleming wrote:
> On Wed, 2011-08-17 at 15:01 +0200, Michal Simek wrote:
>> Oleg Nesterov wrote:
>>> On 08/16, Oleg Nesterov wrote:
>>>> This and the next patch seem to have the same problems with the
>>>> changelogs as 18 and 19.
>>>>
>>>> 13-14 looks correct to me...
>>> I meant 14-15, sorry for noise
>> Do you mean that there is
>> From: Matt Fleming <matt.fleming@intel.com> ?
>>
>> Anyway: I have looked at patches 12-15 and they look good to me.
>>
>> Fix that "From:" line and you can add my
> 
> What's wrong with the From: line? That should be fine. Oleg was saying
> that the description in the changelog needed updating.

It is just additional information but I see when you apply it with git-am it is not there.
That's fine.

Fix what Oleg described and let me know if you want to add to Microblaze tree.

Thanks,
Michal


-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian

Re: [PATCH 12/41] microblaze: Don't reimplement force_sigsegv()

[Matt Fleming]

On Wed, 2011-08-17 at 15:14 +0200, Oleg Nesterov wrote:
> 
> Assuming the changelog is fixed,
> 
> 	Acked-by: Oleg Nesterov <oleg@redhat.com>

Thanks!

> > If you want to propose 12-15 through my Microblaze tree, please let me know.
> 
> Probably this is the best option. Matt, what do you think?

Yeah, that seems like the best solution. All of the patches are
independent so it's easy for the respective maintainers to apply them
when they're ready.

I'll be sending out updated versions today.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 40/41] ncpfs: Use set_current_blocked()

[Matt Fleming]

On Wed, 2011-08-17 at 14:04 +0200, Oleg Nesterov wrote:
> On 08/16, Matt Fleming wrote:
> >
> > > the sighand->action[] checks are racy anyway in the mt case, siglock
> > > can't help.
> >
> > Hmm.. really? I thought that ->siglock serialised modifications to
> > sighand->action[] even in the mt case, no?
> 
> Sure. But another thread can change sighand->action[] right after we
> drop ->siglock. So how can this lock help? We simply read the word,
> this is atomic and doesn't need the locking.

Oh right, in the scenario in ncp_do_request(), sure I understand that. I
thought you were saying that in the general case ->siglock doesn't
protect sighand->action[]! That's why I was confused ;-)

OK, how about this patch (instead of 40/41) which gets rid of all the
nasties? I've Cc'd linux-fsdevel so people can hopefully OK this from a
file system perspective.

Some Tested-by's would be good too.

--------8<--------

>From bb1650295054bdfa96f8f4ff61507d314be8296a Mon Sep 17 00:00:00 2001
From: Matt Fleming <matt.fleming@intel.com>
Date: Wed, 17 Aug 2011 13:59:12 +0100
Subject: [PATCH] ncpfs: Don't attempt to mask signals during
 do_ncp_rpc_call()

Delete the code in ncp_do_request() that attempts to mask signals
across the call to do_ncp_rpc_call(). This code was racy because it
dropped ->siglock across do_ncp_rpc_call() so it was possible for
another thread to modify the signal handlers, which made the code
pointless.

Instead of fixing the code to hold the lock across the call let's just
delete it because, as the FIXME comment (which has been around since
the beginning of git history) says, trying to block signals doesn't
seem right at all.

Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 fs/ncpfs/sock.c |   32 +-------------------------------
 1 files changed, 1 insertions(+), 31 deletions(-)

diff --git a/fs/ncpfs/sock.c b/fs/ncpfs/sock.c
index 3a15872..6618402 100644
--- a/fs/ncpfs/sock.c
+++ b/fs/ncpfs/sock.c
@@ -748,38 +748,8 @@ static int ncp_do_request(struct ncp_server *server, int size,
 	if (!ncp_conn_valid(server)) {
 		return -EIO;
 	}
-	{
-		sigset_t old_set;
-		unsigned long mask, flags;
-
-		spin_lock_irqsave(&current->sighand->siglock, flags);
-		old_set = current->blocked;
-		if (current->flags & PF_EXITING)
-			mask = 0;
-		else
-			mask = sigmask(SIGKILL);
-		if (server->m.flags & NCP_MOUNT_INTR) {
-			/* FIXME: This doesn't seem right at all.  So, like,
-			   we can't handle SIGINT and get whatever to stop?
-			   What if we've blocked it ourselves?  What about
-			   alarms?  Why, in fact, are we mucking with the
-			   sigmask at all? -- r~ */
-			if (current->sighand->action[SIGINT - 1].sa.sa_handler == SIG_DFL)
-				mask |= sigmask(SIGINT);
-			if (current->sighand->action[SIGQUIT - 1].sa.sa_handler == SIG_DFL)
-				mask |= sigmask(SIGQUIT);
-		}
-		siginitsetinv(&current->blocked, mask);
-		recalc_sigpending();
-		spin_unlock_irqrestore(&current->sighand->siglock, flags);
-		
-		result = do_ncp_rpc_call(server, size, reply, max_reply_size);
 
-		spin_lock_irqsave(&current->sighand->siglock, flags);
-		current->blocked = old_set;
-		recalc_sigpending();
-		spin_unlock_irqrestore(&current->sighand->siglock, flags);
-	}
+	result = do_ncp_rpc_call(server, size, reply, max_reply_size);
 
 	DDPRINTK("do_ncp_rpc_call returned %d\n", result);
 
-- 
1.7.4.4

Re: [PATCH 40/41] ncpfs: Use set_current_blocked()

[Oleg Nesterov]

On 08/17, Matt Fleming wrote:
>
> On Wed, 2011-08-17 at 14:04 +0200, Oleg Nesterov wrote:
> > On 08/16, Matt Fleming wrote:
> > >
> > > > the sighand->action[] checks are racy anyway in the mt case, siglock
> > > > can't help.
> > >
> > > Hmm.. really? I thought that ->siglock serialised modifications to
> > > sighand->action[] even in the mt case, no?
> >
> > Sure. But another thread can change sighand->action[] right after we
> > drop ->siglock. So how can this lock help? We simply read the word,
> > this is atomic and doesn't need the locking.
>
> Oh right, in the scenario in ncp_do_request(), sure I understand that. I
> thought you were saying that in the general case ->siglock doesn't
> protect sighand->action[]! That's why I was confused ;-)
>
> OK, how about this patch (instead of 40/41) which gets rid of all the
> nasties? I've Cc'd linux-fsdevel so people can hopefully OK this from a
> file system perspective.

Well, of course I am in no position to ack this change ;)

But obviously I like the idea to kill the obviously wrong code.
In particular, the PF_EXITING/SIGKILL logic looks as "must die
in any case" to me.

If maintainers object, you can remove ->siglock and convert the code
to use set_current_blocked(). IOW, simplify your original patch.

Oleg.

> From bb1650295054bdfa96f8f4ff61507d314be8296a Mon Sep 17 00:00:00 2001
> From: Matt Fleming <matt.fleming@intel.com>
> Date: Wed, 17 Aug 2011 13:59:12 +0100
> Subject: [PATCH] ncpfs: Don't attempt to mask signals during
>  do_ncp_rpc_call()
> 
> Delete the code in ncp_do_request() that attempts to mask signals
> across the call to do_ncp_rpc_call(). This code was racy because it
> dropped ->siglock across do_ncp_rpc_call() so it was possible for
> another thread to modify the signal handlers, which made the code
> pointless.
> 
> Instead of fixing the code to hold the lock across the call let's just
> delete it because, as the FIXME comment (which has been around since
> the beginning of git history) says, trying to block signals doesn't
> seem right at all.
> 
> Signed-off-by: Matt Fleming <matt.fleming@intel.com>
> ---
>  fs/ncpfs/sock.c |   32 +-------------------------------
>  1 files changed, 1 insertions(+), 31 deletions(-)
> 
> diff --git a/fs/ncpfs/sock.c b/fs/ncpfs/sock.c
> index 3a15872..6618402 100644
> --- a/fs/ncpfs/sock.c
> +++ b/fs/ncpfs/sock.c
> @@ -748,38 +748,8 @@ static int ncp_do_request(struct ncp_server *server, int size,
>  	if (!ncp_conn_valid(server)) {
>  		return -EIO;
>  	}
> -	{
> -		sigset_t old_set;
> -		unsigned long mask, flags;
> -
> -		spin_lock_irqsave(&current->sighand->siglock, flags);
> -		old_set = current->blocked;
> -		if (current->flags & PF_EXITING)
> -			mask = 0;
> -		else
> -			mask = sigmask(SIGKILL);
> -		if (server->m.flags & NCP_MOUNT_INTR) {
> -			/* FIXME: This doesn't seem right at all.  So, like,
> -			   we can't handle SIGINT and get whatever to stop?
> -			   What if we've blocked it ourselves?  What about
> -			   alarms?  Why, in fact, are we mucking with the
> -			   sigmask at all? -- r~ */
> -			if (current->sighand->action[SIGINT - 1].sa.sa_handler == SIG_DFL)
> -				mask |= sigmask(SIGINT);
> -			if (current->sighand->action[SIGQUIT - 1].sa.sa_handler == SIG_DFL)
> -				mask |= sigmask(SIGQUIT);
> -		}
> -		siginitsetinv(&current->blocked, mask);
> -		recalc_sigpending();
> -		spin_unlock_irqrestore(&current->sighand->siglock, flags);
> -		
> -		result = do_ncp_rpc_call(server, size, reply, max_reply_size);
>  
> -		spin_lock_irqsave(&current->sighand->siglock, flags);
> -		current->blocked = old_set;
> -		recalc_sigpending();
> -		spin_unlock_irqrestore(&current->sighand->siglock, flags);
> -	}
> +	result = do_ncp_rpc_call(server, size, reply, max_reply_size);
>  
>  	DDPRINTK("do_ncp_rpc_call returned %d\n", result);
>  
> -- 
> 1.7.4.4
> 
> 

Re: [PATCH 00/41] signal: Use set_current_blocked()

[Oleg Nesterov]

On 08/16, Matt Fleming wrote:
>
> Did you want me to send just the updated
> versions of patches or the entire series again? It would seem over the
> top to send the entire series.

This is up to you. Just in case, feel free to add my acked-by or
reviewed-by to any patch I didn't comment explicitly. Or may be

> Also, are you going to pull these patches into your tree and send them
> to Linus or would it be better to route them through -mm?

This is the question ;)

I simply do not know what should we do. I think this should be
routed via maintainers.

Or I can push this into the poor ptrace branch... but I am
thinking with horror about the possible conflicts in arch/ code...

What do you think?



Damn. And I can't relax. This is very minor, but I still think
it would be nice to have a common helper which plays sa_mask /
SA_NODEFER to set ->blocked. But in this case, you probably need
to resend the whole series ;)

I mean something like the patch below. OK, nevermind. Probably
this factorization doesn't buy too much, but it will complicate
the merging.

Oleg.

--- x/include/linux/signal.h
+++ x/include/linux/signal.h
@@ -254,6 +254,7 @@ extern void set_current_blocked(const si
 extern int show_unhandled_signals;
 
 extern int get_signal_to_deliver(siginfo_t *info, struct k_sigaction *return_ka, struct pt_regs *regs, void *cookie);
+extern void xxx(struct k_sigaction *ka);
 extern void exit_signals(struct task_struct *tsk);
 
 extern struct kmem_cache *sighand_cachep;
--- x/kernel/signal.c
+++ x/kernel/signal.c
@@ -2314,6 +2314,16 @@ relock:
 	return signr;
 }
 
+void xxx(struct k_sigaction *ka)
+{
+	sigset_t blocked;
+
+	sigorsets(&blocked, &current->blocked, &ka->sa.sa_mask);
+	if (!(ka->sa.sa_flags & SA_NODEFER))
+		sigaddset(&blocked, sig);
+	set_current_blocked(&blocked);
+}
+
 /*
  * It could be that complete_signal() picked us to notify about the
  * group-wide signal. Other threads should be notified now to take
--- x/arch/x86/kernel/signal.c
+++ x/arch/x86/kernel/signal.c
@@ -682,7 +682,6 @@ static int
 handle_signal(unsigned long sig, siginfo_t *info, struct k_sigaction *ka,
 		struct pt_regs *regs)
 {
-	sigset_t blocked;
 	int ret;
 
 	/* Are we from a system call? */
@@ -733,14 +732,10 @@ handle_signal(unsigned long sig, siginfo
 	 */
 	regs->flags &= ~X86_EFLAGS_TF;
 
-	sigorsets(&blocked, &current->blocked, &ka->sa.sa_mask);
-	if (!(ka->sa.sa_flags & SA_NODEFER))
-		sigaddset(&blocked, sig);
-	set_current_blocked(&blocked);
+	xxx(ka);
 
 	tracehook_signal_handler(sig, info, ka, regs,
 				 test_thread_flag(TIF_SINGLESTEP));
-
 	return 0;
 }
 

Re: [PATCH 1/1] kthreads: allow_signal: don't play with ->blocked

[Oleg Nesterov]

On 08/17, Tejun Heo wrote:
>
> On Tue, Aug 16, 2011 at 10:50:22PM +0100, Matt Fleming wrote:
> > On Tue, 2011-08-16 at 21:51 +0200, Tejun Heo wrote:
> > > I agree with the patchset but given that daemonize() isn't all that
> > > popular and you already posted most (or was it all?) conversions,

Yes, with the patches I sent daemonize() has no callers.

> but in this case it's an interface which is
> quite unpopular and with relatively easy workaround (just use
> kthread).

Agreed.

> The worst thing we can do regarding API change is silently changing
> semantics while not changing the interface.
> ...
> Out-of-kernel user which depended on the combination working would now
> be left with code which compiles fine but behaves differently, which
> sucks big time.

Yes, this is of course possible.

> So, let's please collect all the
> related patches into one series,

This is what I can't understand ;) This connects to your
"How do you wanna route these" question in another thread.

> drop all in-kernel daemonize() users,
> kill daemonize() and then change allow_signal() behavior.

OK, if we kill daemonize() without the deprecation stage, this is fine.

Initially I assumed it won't go away soon, and this sigdelset()
is really nasty although minor.

Oleg.

Re: [PATCH 00/41] signal: Use set_current_blocked()

[Matt Fleming]

On Wed, 2011-08-17 at 19:01 +0200, Oleg Nesterov wrote:
> On 08/16, Matt Fleming wrote:
> >
> > Did you want me to send just the updated
> > versions of patches or the entire series again? It would seem over the
> > top to send the entire series.
> 
> This is up to you. Just in case, feel free to add my acked-by or
> reviewed-by to any patch I didn't comment explicitly. Or may be
> 
> > Also, are you going to pull these patches into your tree and send them
> > to Linus or would it be better to route them through -mm?
> 
> This is the question ;)
> 
> I simply do not know what should we do. I think this should be
> routed via maintainers.

Yeah, that seems like the best solution.

> Or I can push this into the poor ptrace branch... but I am
> thinking with horror about the possible conflicts in arch/ code...
> 
> What do you think?

I'll let all the maintainers take it through their trees.

> Damn. And I can't relax. This is very minor, but I still think
> it would be nice to have a common helper which plays sa_mask /
> SA_NODEFER to set ->blocked. But in this case, you probably need
> to resend the whole series ;)
> 
> I mean something like the patch below. OK, nevermind. Probably
> this factorization doesn't buy too much, but it will complicate
> the merging.

This does look like a decent clean up, especially because so many
architectures got this sequence wrong in the past. But yeah, it will
require me to rewrite half my series ;-)

Is it worth rewriting the series? Dunno. I'm not convinced that the
wrapper buys us enough for that, plus it'd increase the number of
patches.

Maybe it'd be better to do it as a separate set of patches?

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 32/41] unicore32: Use set_current_blocked()

[Guan Xuetao]

It looks good to me.

And already tested by compilation.

Cc: Arnd Bergmann <arnd@arndb.de>

Acked-by: Guan Xuetao <gxt@mprc.pku.edu.cn>

Thanks & Regards.

Guan Xuetao


On Thu, 2011-08-11 at 14:57 +0100, Matt Fleming wrote:
> From: Matt Fleming <matt.fleming@intel.com>
> 
> As described in e6fa16ab ("signal: sigprocmask() should do
> retarget_shared_pending()") the modification of current->blocked is
> incorrect as we need to check whether the signal we're about to block
> is pending in the shared queue.
> 
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Guan Xuetao <gxt@mprc.pku.edu.cn>
> Signed-off-by: Matt Fleming <matt.fleming@intel.com>
> ---
>  arch/unicore32/kernel/signal.c |   15 +++++----------
>  1 files changed, 5 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/unicore32/kernel/signal.c b/arch/unicore32/kernel/signal.c
> index b163fca..911b549 100644
> --- a/arch/unicore32/kernel/signal.c
> +++ b/arch/unicore32/kernel/signal.c
> @@ -63,10 +63,7 @@ static int restore_sigframe(struct pt_regs *regs, struct sigframe __user *sf)
>  	err = __copy_from_user(&set, &sf->uc.uc_sigmask, sizeof(set));
>  	if (err == 0) {
>  		sigdelsetmask(&set, ~_BLOCKABLE);
> -		spin_lock_irq(&current->sighand->siglock);
> -		current->blocked = set;
> -		recalc_sigpending();
> -		spin_unlock_irq(&current->sighand->siglock);
> +		set_current_blocked(&set);
>  	}
>  
>  	err |= __get_user(regs->UCreg_00, &sf->uc.uc_mcontext.regs.UCreg_00);
> @@ -321,6 +318,7 @@ static int handle_signal(unsigned long sig, struct k_sigaction *ka,
>  {
>  	struct thread_info *thread = current_thread_info();
>  	struct task_struct *tsk = current;
> +	sigset_t blocked;
>  	int usig = sig;
>  	int ret;
>  
> @@ -372,13 +370,10 @@ static int handle_signal(unsigned long sig, struct k_sigaction *ka,
>  	/*
>  	 * Block the signal if we were successful.
>  	 */
> -	spin_lock_irq(&tsk->sighand->siglock);
> -	sigorsets(&tsk->blocked, &tsk->blocked,
> -		  &ka->sa.sa_mask);
> +	sigorsets(&blocked, &tsk->blocked, &ka->sa.sa_mask);
>  	if (!(ka->sa.sa_flags & SA_NODEFER))
> -		sigaddset(&tsk->blocked, sig);
> -	recalc_sigpending();
> -	spin_unlock_irq(&tsk->sighand->siglock);
> +		sigaddset(&blocked, sig);
> +	set_current_blocked(&blocked);
>  
>  	return 0;
>  }

Re: [PATCH 00/41] signal: Use set_current_blocked()

[Matt Fleming]

On Wed, 2011-08-17 at 23:17 +0100, Matt Fleming wrote:
> 
> This does look like a decent clean up, especially because so many
> architectures got this sequence wrong in the past. But yeah, it will
> require me to rewrite half my series ;-)
> 
> Is it worth rewriting the series? Dunno. I'm not convinced that the
> wrapper buys us enough for that, plus it'd increase the number of
> patches.
> 
> Maybe it'd be better to do it as a separate set of patches?

OK so, I went ahead and rewrote the series using your clean up. It's in
the 'oleg/set-current-blocked-v2' branch at, 

git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/linux-2.6.git

Would you mind taking a quick look and see which series you prefer? I'll
wait for your reply before I resend any patches.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 40/41] ncpfs: Use set_current_blocked()

[Oleg Nesterov]

On 08/17, Petr Vandrovec wrote:
>
> I do not care about 'intr' support, but regular code path blocking all
> signals has to stay.

I do not care about SIGINT/SIGQUIT check, just I simply can't understand
the logic.

But PF_EXITING/SIGKILL code looks absolutely strange.

> PF_EXITING code is there because in the past something was BUG_ON-ing that
> nobody should touch signals while exiting,

Not sure I understand... do you mean we have a kernel bug? it should be
fixed then.

> and same code made sure you
> cannot send signals to exiting process.

Again, can't understand.

A PF_EXITING doesn't need to block the signals at all. See wants_siganl().

But, otoh, even sigblock(SIGKILL) can't protect from SIGKILL or another
fatal signal if this thread has a live subthread.

However, blocking SIGKILL makes the difference if it exits with the pending
SIGKILL. In this case recalc_sigpending() clears TIF_SIGPENDING, this "helps"
to actually sleep in TASK_INTERRUPTIBLE.

At the same time, ncp_do_request() can unblock, say, SIGINT. And this
means that if this task exits because it was killed by SIGINT it won't
block anyway.

And I can't understand why it if fine to kill the live task, but not fine
to interrupt the exiting task. If this was intended. And if you want to
block all signals, why can't you do wait_event_uninterruptible() instead?
Or interruptible/uninterruptible depending on PF_EXITING.


Now suppose it is not PF_EXITING. In this case it is pointless to block
any fatal signal unless this task is single-threaded.


So far I think this code does not understand what it does ;)


OK. I guess nobody cares. Matt, could you redo your original patch?
just remove ->siglock and convert it to use set_current_blocked()
leaving this logic alone.

Oleg.

Re: [PATCH 18/41] OpenRISC: Don't reimplement force_sigsegv()

[Oleg Nesterov]

On 08/16, Matt Fleming wrote:
>
> On Tue, 2011-08-16 at 18:49 +0200, Oleg Nesterov wrote:
> >
> > And since force_sigsegv() drops
> > the lock after setting SIG_DFL we can "race" with the sub-thread anyway.
>
> I did notice that race in force_sigsegv() too, is it a real problem?

Oh, I don't really know. I mean, I do not know if this really needs
the fix.

OK, suppose that another thread does signal(SIGSEGV, SIG_IGN) in
between. This probably means it asks for the problems anyway. and
we can pretend this was done before this SIGSEGV was dequeued.

If it does signal(SIGSEGV, my_handler), then most probably
force_sigsegv() will be called again soon, after dequeueing SIGSEGV.

Oleg.

Re: [PATCH 00/41] signal: Use set_current_blocked()

[Oleg Nesterov]

On 08/18, Matt Fleming wrote:
>
> On Wed, 2011-08-17 at 23:17 +0100, Matt Fleming wrote:
> >
> > This does look like a decent clean up, especially because so many
> > architectures got this sequence wrong in the past. But yeah, it will
> > require me to rewrite half my series ;-)
> >
> > Is it worth rewriting the series? Dunno. I'm not convinced that the
> > wrapper buys us enough for that,

Oh, I am not sure too. I almost regret I mentioned this cleanup. Just
I was a bit suprised by how many arches do this wrong. See also below.

> plus it'd increase the number of
> patches.

By 1. A single patch can convert the code to use set_current_blocked()
and block_sigmask().

And probably the patch which adds block_sigmask() should change arch/x86
as well, this makes its purpose immediately clear. IOW, I'd prefer the
patch I sent (with rename). But once again, I won't persist.

> > Maybe it'd be better to do it as a separate set of patches?

I don't think so... this means another series touching arch/* to
make the very minor cleanup.

> OK so, I went ahead and rewrote the series using your clean up. It's in
> the 'oleg/set-current-blocked-v2' branch at,
>
> git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/linux-2.6.git
>
> Would you mind taking a quick look and see which series you prefer?

No, no, no. Matt, please choose the series which _you_ prefer. You
are the author. I am fine either way.

> I resend any patches.

Yes. I think you should resend the whole series in any case. Because
I think our discussion could confuse the maintainers.


Can I ask you to CC them all in 00/XX ? In this case I can reply
to 00 saying that I am going to take the whole series, unless the
maintainer want to do this. Otherwise I should reply per-arch, this
will certainly cause the confusion.

And, Matt, if I take the patch and then we have any sort of conflict,
you will have to help me with the git problems ;) My understanding
of git magic is quite limited.

Oleg.

Re: [PATCH 40/41] ncpfs: Use set_current_blocked()

[Matt Fleming]

On Thu, 2011-08-18 at 19:05 +0200, Oleg Nesterov wrote:
> 
> OK. I guess nobody cares. Matt, could you redo your original patch?
> just remove ->siglock and convert it to use set_current_blocked()
> leaving this logic alone.

Sure, will do.

-- 
Matt Fleming, Intel Open Source Technology Center