cleanup/fix allow_signal/disallow_signal

PATCH New Oleg Nesterov 1 participant 12 messages Apr 15, 2014

Subsystems (4)

x86 Scheduler m68k architecture Journalling flash file system V2 (JFFS2)

lore

Share & tools

lore

Oleg Nesterov

author

2014-04-15 18:36 UTC #0 lore raw

cover cleanup/fix allow_signal/disallow_signal

I added RESEND2 to avoid the confusion, sorry to all for spam.

To remind, this doesn't depend on (or conflict with) the previous patch "kill CLONE_KERNEL, change kernel_thread(kernel_init) to avoid CLONE_SIGHAND" I sent.

Oleg.

 arch/m68k/include/asm/signal.h |    9 ----
 arch/x86/include/asm/signal.h  |    6 ---
 fs/jffs2/background.c          |   12 +++--
 include/linux/sched.h          |    3 -
 include/linux/signal.h         |   21 +++++++--
 kernel/exit.c                  |   39 -----------------
 kernel/kmod.c                  |    5 +--
 kernel/signal.c                |   90 ++++++++++++++++++---------------------
 8 files changed, 66 insertions(+), 119 deletions(-)

Oleg Nesterov

author

2014-04-15 18:37 UTC #1 lore raw

PATCH 1/11 signals: kill sigfindinword()

It has no users and it doesn't look useful. I do not know why/when it was introduced, I can't even find any user in the git history.

Signed-off-by Oleg Nesterov <[email protected]>

Acked-by Geert Uytterhoeven <[email protected]>

---
 arch/m68k/include/asm/signal.h |    9 ---------
 arch/x86/include/asm/signal.h  |    6 ------
 include/linux/signal.h         |    5 -----
 3 files changed, 0 insertions(+), 20 deletions(-)

arch/m68k/include/asm/signal.h +0 -9

index 214320b..8c8ce5e 100644
@@ -60,15 +60,6 @@ static inline int __gen_sigismember(sigset_t *set, int _sig)
 	 __const_sigismember(set,sig) :		\
 	 __gen_sigismember(set,sig))
 
-static inline int sigfindinword(unsigned long word)
-{
-	asm ("bfffo %1{#0,#0},%0"
-		: "=d" (word)
-		: "d" (word & -word)
-		: "cc");
-	return word ^ 31;
-}
-
 #endif /* !CONFIG_CPU_HAS_NO_BITFIELDS */
 
 #ifndef __uClinux__

arch/x86/include/asm/signal.h +0 -6

index 35e67a4..31eab86 100644
@@ -92,12 +92,6 @@ static inline int __gen_sigismember(sigset_t *set, int _sig)
 	 ? __const_sigismember((set), (sig))	\
 	 : __gen_sigismember((set), (sig)))
 
-static inline int sigfindinword(unsigned long word)
-{
-	asm("bsfl %1,%0" : "=r"(word) : "rm"(word) : "cc");
-	return word;
-}
-
 struct pt_regs;
 
 #else /* __i386__ */

include/linux/signal.h +0 -5

index 2ac423b..ae744c3 100644
@@ -63,11 +63,6 @@ static inline int sigismember(sigset_t *set, int _sig)
 		return 1 & (set->sig[sig / _NSIG_BPW] >> (sig % _NSIG_BPW));
 }
 
-static inline int sigfindinword(unsigned long word)
-{
-	return ffz(~word);
-}
-
 #endif /* __HAVE_ARCH_SIG_BITOPS */
 
 static inline int sigisemptyset(sigset_t *set)

Signature · 1 line

1.5.5.1

Oleg Nesterov

author

2014-04-15 18:37 UTC #2 lore raw

PATCH 2/11 signals: s/siginitset/sigemptyset/ in do_sigtimedwait()

In reply to Oleg Nesterov

Cosmetic, but siginitset(0) looks a bit strange, sigemptyset() is what do_sigtimedwait() needs.

Signed-off-by Oleg Nesterov <[email protected]>

---
 kernel/signal.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

kernel/signal.c +1 -1

index 6ea13c0..a717749 100644
@@ -2854,7 +2854,7 @@ int do_sigtimedwait(const sigset_t *which, siginfo_t *info,
 
 		spin_lock_irq(&tsk->sighand->siglock);
 		__set_task_blocked(tsk, &tsk->real_blocked);
-		siginitset(&tsk->real_blocked, 0);
+		sigemptyset(&tsk->real_blocked);
 		sig = dequeue_signal(tsk, &mask, info);
 	}
 	spin_unlock_irq(&tsk->sighand->siglock);

Signature · 1 line

1.5.5.1

Oleg Nesterov

author

2014-04-15 18:37 UTC #3 lore raw

PATCH 3/11 signals: kill rm_from_queue(), change prepare_signal() to use for_each_thread()

In reply to Oleg Nesterov

rm_from_queue() doesn't make sense. The only caller, prepare_signal(), can use rm_from_queue_full() with the same effect.

While at it, change prepare_signal() to use for_each_thread() instead of do/while_each_thread.

Signed-off-by Oleg Nesterov <[email protected]>

---
 kernel/signal.c |   43 ++++++++++---------------------------------
 1 files changed, 10 insertions(+), 33 deletions(-)

kernel/signal.c +10 ~~-33~~

index a717749..6072efa 100644
@@ -727,29 +727,6 @@ static int rm_from_queue_full(sigset_t *mask, struct sigpending *s)
 	}
 	return 1;
 }
-/*
- * Remove signals in mask from the pending set and queue.
- * Returns 1 if any signals were found.
- *
- * All callers must be holding the siglock.
- */
-static int rm_from_queue(unsigned long mask, struct sigpending *s)
-{
-	struct sigqueue *q, *n;
-
-	if (!sigtestsetmask(&s->signal, mask))
-		return 0;
-
-	sigdelsetmask(&s->signal, mask);
-	list_for_each_entry_safe(q, n, &s->list, list) {
-		if (q->info.si_signo < SIGRTMIN &&
-		    (mask & sigmask(q->info.si_signo))) {
-			list_del_init(&q->list);
-			__sigqueue_free(q);
-		}
-	}
-	return 1;
-}
 
 static inline int is_si_special(const struct siginfo *info)
 {

@@ -861,6 +838,7 @@ static bool prepare_signal(int sig, struct task_struct *p, bool force)
 {
 	struct signal_struct *signal = p->signal;
 	struct task_struct *t;
+	sigset_t flush;
 
 	if (signal->flags & (SIGNAL_GROUP_EXIT | SIGNAL_GROUP_COREDUMP)) {
 		if (signal->flags & SIGNAL_GROUP_COREDUMP)

@@ -872,26 +850,25 @@ static bool prepare_signal(int sig, struct task_struct *p, bool force)
 		/*
 		 * This is a stop signal.  Remove SIGCONT from all queues.
 		 */
-		rm_from_queue(sigmask(SIGCONT), &signal->shared_pending);
-		t = p;
-		do {
-			rm_from_queue(sigmask(SIGCONT), &t->pending);
-		} while_each_thread(p, t);
+		siginitset(&flush, sigmask(SIGCONT));
+		rm_from_queue_full(&flush, &signal->shared_pending);
+		for_each_thread(p, t)
+			rm_from_queue_full(&flush, &t->pending);
 	} else if (sig == SIGCONT) {
 		unsigned int why;
 		/*
 		 * Remove all stop signals from all queues, wake all threads.
 		 */
-		rm_from_queue(SIG_KERNEL_STOP_MASK, &signal->shared_pending);
-		t = p;
-		do {
+		siginitset(&flush, SIG_KERNEL_STOP_MASK);
+		rm_from_queue_full(&flush, &signal->shared_pending);
+		for_each_thread(p, t) {
+			rm_from_queue_full(&flush, &t->pending);
 			task_clear_jobctl_pending(t, JOBCTL_STOP_PENDING);
-			rm_from_queue(SIG_KERNEL_STOP_MASK, &t->pending);
 			if (likely(!(t->ptrace & PT_SEIZED)))
 				wake_up_state(t, __TASK_STOPPED);
 			else
 				ptrace_trap_notify(t);
-		} while_each_thread(p, t);
+		}
 
 		/*
 		 * Notify the parent with CLD_CONTINUED if we were stopped.

Signature · 1 line

1.5.5.1

Oleg Nesterov

author

2014-04-15 18:37 UTC #4 lore raw

PATCH 4/11 signals: rename rm_from_queue_full() to flush_sigqueue_mask()

In reply to Oleg Nesterov

"rm_from_queue_full" looks ugly and misleading, especially now that rm_from_queue() has gone away. Rename it to flush_sigqueue_mask(), this matches flush_sigqueue() we already have.

Also remove the obsolete comment which explains the difference with rm_from_queue() we already killed.

Signed-off-by Oleg Nesterov <[email protected]>

---
 kernel/signal.c |   19 ++++++++-----------
 1 files changed, 8 insertions(+), 11 deletions(-)

kernel/signal.c +8 ~~-11~~

index 6072efa..c0b7d0c 100644
@@ -705,11 +705,8 @@ void signal_wake_up_state(struct task_struct *t, unsigned int state)
  * Returns 1 if any signals were found.
  *
  * All callers must be holding the siglock.
- *
- * This version takes a sigset mask and looks at all signals,
- * not just those in the first mask word.
  */
-static int rm_from_queue_full(sigset_t *mask, struct sigpending *s)
+static int flush_sigqueue_mask(sigset_t *mask, struct sigpending *s)
 {
 	struct sigqueue *q, *n;
 	sigset_t m;

@@ -851,18 +848,18 @@ static bool prepare_signal(int sig, struct task_struct *p, bool force)
 		 * This is a stop signal.  Remove SIGCONT from all queues.
 		 */
 		siginitset(&flush, sigmask(SIGCONT));
-		rm_from_queue_full(&flush, &signal->shared_pending);
+		flush_sigqueue_mask(&flush, &signal->shared_pending);
 		for_each_thread(p, t)
-			rm_from_queue_full(&flush, &t->pending);
+			flush_sigqueue_mask(&flush, &t->pending);
 	} else if (sig == SIGCONT) {
 		unsigned int why;
 		/*
 		 * Remove all stop signals from all queues, wake all threads.
 		 */
 		siginitset(&flush, SIG_KERNEL_STOP_MASK);
-		rm_from_queue_full(&flush, &signal->shared_pending);
+		flush_sigqueue_mask(&flush, &signal->shared_pending);
 		for_each_thread(p, t) {
-			rm_from_queue_full(&flush, &t->pending);
+			flush_sigqueue_mask(&flush, &t->pending);
 			task_clear_jobctl_pending(t, JOBCTL_STOP_PENDING);
 			if (likely(!(t->ptrace & PT_SEIZED)))
 				wake_up_state(t, __TASK_STOPPED);

@@ -3101,9 +3098,9 @@ int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact)
 		if (sig_handler_ignored(sig_handler(t, sig), sig)) {
 			sigemptyset(&mask);
 			sigaddset(&mask, sig);
-			rm_from_queue_full(&mask, &t->signal->shared_pending);
+			flush_sigqueue_mask(&mask, &t->signal->shared_pending);
 			do {
-				rm_from_queue_full(&mask, &t->pending);
+				flush_sigqueue_mask(&mask, &t->pending);
 			} while_each_thread(current, t);
 		}
 	}

@@ -3112,7 +3109,7 @@ int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact)
 	return 0;
 }
 
-static int 
+static int
 do_sigaltstack (const stack_t __user *uss, stack_t __user *uoss, unsigned long sp)
 {
 	stack_t oss;

Signature · 1 line

1.5.5.1

Oleg Nesterov

author

2014-04-15 18:49 UTC #5 lore raw

PATCH 5/11 signals: cleanup the usage of t/current in do_sigaction()

In reply to Oleg Nesterov

The usage of "task_struct *t" and "current" in do_sigaction() looks really annoying and chaotic. Initially "t" is used as a cached value of current but not consistently, then it is reused as a loop variable and we have to use "current" again.

Cleanup this mess and also convert the code to use for_each_thread().

Signed-off-by Oleg Nesterov <[email protected]>

---
 kernel/signal.c |   15 +++++++--------
 1 files changed, 7 insertions(+), 8 deletions(-)

kernel/signal.c +7 -8

index c0b7d0c..a58b0db 100644
@@ -3067,16 +3067,16 @@ COMPAT_SYSCALL_DEFINE4(rt_tgsigqueueinfo,
 
 int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact)
 {
-	struct task_struct *t = current;
+	struct task_struct *p = current, *t;
 	struct k_sigaction *k;
 	sigset_t mask;
 
 	if (!valid_signal(sig) || sig < 1 || (act && sig_kernel_only(sig)))
 		return -EINVAL;
 
-	k = &t->sighand->action[sig-1];
+	k = &p->sighand->action[sig-1];
 
-	spin_lock_irq(&current->sighand->siglock);
+	spin_lock_irq(&p->sighand->siglock);
 	if (oact)
 		*oact = *k;

@@ -3095,17 +3095,16 @@ int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact)
 		 *   (for example, SIGCHLD), shall cause the pending signal to
 		 *   be discarded, whether or not it is blocked"
 		 */
-		if (sig_handler_ignored(sig_handler(t, sig), sig)) {
+		if (sig_handler_ignored(sig_handler(p, sig), sig)) {
 			sigemptyset(&mask);
 			sigaddset(&mask, sig);
-			flush_sigqueue_mask(&mask, &t->signal->shared_pending);
-			do {
+			flush_sigqueue_mask(&mask, &p->signal->shared_pending);
+			for_each_thread(p, t)
 				flush_sigqueue_mask(&mask, &t->pending);
-			} while_each_thread(current, t);
 		}
 	}
 
-	spin_unlock_irq(&current->sighand->siglock);
+	spin_unlock_irq(&p->sighand->siglock);
 	return 0;
 }

Signature · 1 line

1.5.5.1

Oleg Nesterov

author

2014-04-15 18:48 UTC #6 lore raw

PATCH 6/11 signals: mv {dis,}allow_signal() from sched.h/exit.c to signal.[ch]

In reply to Oleg Nesterov

Move the declaration/definition of allow_signal/disallow_signal to signal.h/signal.c. The new place is more logical and allows to use the static helpers in signal.c (see the next changes).

While at it, make them return void and remove the valid_signal() check. Nobody checks the returned value, and in-kernel users must not pass the wrong signal number.

Signed-off-by Oleg Nesterov <[email protected]>

---
 include/linux/sched.h  |    3 ---
 include/linux/signal.h |    2 ++
 kernel/exit.c          |   39 ---------------------------------------
 kernel/signal.c        |   29 +++++++++++++++++++++++++++++
 4 files changed, 31 insertions(+), 42 deletions(-)

include/linux/sched.h +0 -3

index a04214f..82fe3da 100644
@@ -2360,9 +2360,6 @@ extern void flush_itimer_signals(void);
 
 extern void do_group_exit(int);
 
-extern int allow_signal(int);
-extern int disallow_signal(int);
-
 extern int do_execve(struct filename *,
 		     const char __user * const __user *,
 		     const char __user * const __user *);

include/linux/signal.h +2 -0

index ae744c3..ac83c59 100644
@@ -284,6 +284,8 @@ extern int get_signal_to_deliver(siginfo_t *info, struct k_sigaction *return_ka,
 extern void signal_setup_done(int failed, struct ksignal *ksig, int stepping);
 extern void signal_delivered(int sig, siginfo_t *info, struct k_sigaction *ka, struct pt_regs *regs, int stepping);
 extern void exit_signals(struct task_struct *tsk);
+extern void allow_signal(int);
+extern void disallow_signal(int);
 
 /*
  * Eventually that'll replace get_signal_to_deliver(); macro for now,

kernel/exit.c +0 ~~-39~~

index 6ed6a1d..ad7183a 100644
@@ -313,45 +313,6 @@ kill_orphaned_pgrp(struct task_struct *tsk, struct task_struct *parent)
 	}
 }
 
-/*
- * Let kernel threads use this to say that they allow a certain signal.
- * Must not be used if kthread was cloned with CLONE_SIGHAND.
- */
-int allow_signal(int sig)
-{
-	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);
-	/*
-	 * 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();
-	spin_unlock_irq(&current->sighand->siglock);
-	return 0;
-}
-
-EXPORT_SYMBOL(allow_signal);
-
-int disallow_signal(int sig)
-{
-	if (!valid_signal(sig) || sig < 1)
-		return -EINVAL;
-
-	spin_lock_irq(&current->sighand->siglock);
-	current->sighand->action[(sig)-1].sa.sa_handler = SIG_IGN;
-	recalc_sigpending();
-	spin_unlock_irq(&current->sighand->siglock);
-	return 0;
-}
-
-EXPORT_SYMBOL(disallow_signal);
-
 #ifdef CONFIG_MM_OWNER
 /*
  * A task is exiting.   If it owned this mm, find a new owner for the mm.

kernel/signal.c +29 -0

index a58b0db..8a7ee44 100644
@@ -3065,6 +3065,35 @@ COMPAT_SYSCALL_DEFINE4(rt_tgsigqueueinfo,
 }
 #endif
 
+/*
+ * Let kernel threads use this to say that they allow a certain signal.
+ * Must not be used if kthread was cloned with CLONE_SIGHAND.
+ */
+void allow_signal(int sig)
+{
+	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
+	 * SIGKILL or just silently dropped.
+	 */
+	current->sighand->action[(sig)-1].sa.sa_handler = (void __user *)2;
+	recalc_sigpending();
+	spin_unlock_irq(&current->sighand->siglock);
+}
+EXPORT_SYMBOL(allow_signal);
+
+void disallow_signal(int sig)
+{
+	spin_lock_irq(&current->sighand->siglock);
+	current->sighand->action[(sig)-1].sa.sa_handler = SIG_IGN;
+	recalc_sigpending();
+	spin_unlock_irq(&current->sighand->siglock);
+}
+EXPORT_SYMBOL(disallow_signal);
+
 int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact)
 {
 	struct task_struct *p = current, *t;

Signature · 1 line

1.5.5.1

Oleg Nesterov

author

2014-04-15 18:37 UTC #7 lore raw

PATCH 7/11 signals: jffs2: fix the wrong usage of disallow_signal()

In reply to Oleg Nesterov

jffs2_garbage_collect_thread() does disallow_signal(SIGHUP) around jffs2_garbage_collect_pass() and the comment says "We don't want SIGHUP to interrupt us".

But disallow_signal() can't ensure that jffs2_garbage_collect_pass() won't be interrupted by SIGHUP, the problem is that SIGHUP can be already pending when disallow_signal() is called, and in this case any interruptible sleep won't block.

Note: this is in fact because disallow_signal() is buggy and should be fixed, see the next changes.

But there is another reason why disallow_signal() is wrong: SIG_IGN set by disallow_signal() silently discards any SIGHUP which can be sent before the next allow_signal(SIGHUP).

Change this code to use sigprocmask(SIG_UNBLOCK/SIG_BLOCK, SIGHUP). This even matches the old (and wrong) semantics allow/disallow had when this logic was written.

Signed-off-by Oleg Nesterov <[email protected]>

---
 fs/jffs2/background.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

fs/jffs2/background.c +7 -5

index 2b60ce1..bb9cebc 100644
@@ -75,10 +75,13 @@ void jffs2_stop_garbage_collect_thread(struct jffs2_sb_info *c)
 static int jffs2_garbage_collect_thread(void *_c)
 {
 	struct jffs2_sb_info *c = _c;
+	sigset_t hupmask;
 
+	siginitset(&hupmask, sigmask(SIGHUP));
 	allow_signal(SIGKILL);
 	allow_signal(SIGSTOP);
 	allow_signal(SIGCONT);
+	allow_signal(SIGHUP);
 
 	c->gc_task = current;
 	complete(&c->gc_thread_start);

@@ -87,7 +90,7 @@ static int jffs2_garbage_collect_thread(void *_c)
 
 	set_freezable();
 	for (;;) {
-		allow_signal(SIGHUP);
+		sigprocmask(SIG_UNBLOCK, &hupmask, NULL);
 	again:
 		spin_lock(&c->erase_completion_lock);
 		if (!jffs2_thread_should_wake(c)) {

@@ -95,10 +98,9 @@ static int jffs2_garbage_collect_thread(void *_c)
 			spin_unlock(&c->erase_completion_lock);
 			jffs2_dbg(1, "%s(): sleeping...\n", __func__);
 			schedule();
-		} else
+		} else {
 			spin_unlock(&c->erase_completion_lock);
-			
-
+		}
 		/* Problem - immediately after bootup, the GCD spends a lot
 		 * of time in places like jffs2_kill_fragtree(); so much so
 		 * that userspace processes (like gdm and X) are starved

@@ -150,7 +152,7 @@ static int jffs2_garbage_collect_thread(void *_c)
 			}
 		}
 		/* We don't want SIGHUP to interrupt us. STOP and KILL are OK though. */
-		disallow_signal(SIGHUP);
+		sigprocmask(SIG_BLOCK, &hupmask, NULL);
 
 		jffs2_dbg(1, "%s(): pass\n", __func__);
 		if (jffs2_garbage_collect_pass(c) == -ENOSPC) {

Signature · 1 line

1.5.5.1

Oleg Nesterov

author

2014-04-15 18:38 UTC #8 lore raw

PATCH 8/11 signals: kill the obsolete sigdelset() and recalc_sigpending() in allow_signal()

In reply to Oleg Nesterov

allow_signal() does sigdelset(current->blocked) due to historic reason, previously it could be called by a daemonize()'ed kthread, and daemonize() played with current->blocked.

Now that daemonize() has gone away we can remove sigdelset() and recalc_sigpending(). If a user really wants to unblock a signal, it must use sigprocmask() or set_current_block() explicitely.

Signed-off-by Oleg Nesterov <[email protected]>

---
 kernel/signal.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

kernel/signal.c +1 -4

index 8a7ee44..3eec27b 100644
@@ -3071,16 +3071,13 @@ COMPAT_SYSCALL_DEFINE4(rt_tgsigqueueinfo,
  */
 void allow_signal(int sig)
 {
-	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
 	 * SIGKILL or just silently dropped.
 	 */
+	spin_lock_irq(&current->sighand->siglock);
 	current->sighand->action[(sig)-1].sa.sa_handler = (void __user *)2;
-	recalc_sigpending();
 	spin_unlock_irq(&current->sighand->siglock);
 }
 EXPORT_SYMBOL(allow_signal);

Signature · 1 line

1.5.5.1

Oleg Nesterov

author

2014-04-15 18:38 UTC #9 lore raw

PATCH 9/11 signals: disallow_signal() should flush the potentially pending signal

In reply to Oleg Nesterov

disallow_signal() simply sets SIG_IGN, this is not enough and recalc_sigpending() is simply pointless because in can never change the state of TIF_SIGPENDING.

If we ignore a signal, we also need to do flush_sigqueue_mask() for the case when this signal is pending, this way recalc_sigpending() can actually clear TIF_SIGPENDING and we do not "leak" the allocated siginfo's.

Signed-off-by Oleg Nesterov <[email protected]>

---
 kernel/signal.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

kernel/signal.c +7 -0

index 3eec27b..4bab1b7 100644
@@ -3084,8 +3084,15 @@ EXPORT_SYMBOL(allow_signal);
 
 void disallow_signal(int sig)
 {
+	sigset_t mask;
+
+	sigemptyset(&mask);
+	sigaddset(&mask, sig);
+
 	spin_lock_irq(&current->sighand->siglock);
 	current->sighand->action[(sig)-1].sa.sa_handler = SIG_IGN;
+	flush_sigqueue_mask(&mask, &current->signal->shared_pending);
+	flush_sigqueue_mask(&mask, &current->pending);
 	recalc_sigpending();
 	spin_unlock_irq(&current->sighand->siglock);
 }

Signature · 1 line

1.5.5.1

Oleg Nesterov

author

2014-04-15 18:37 UTC #10 lore raw

PATCH 10/11 signals: introduce kernel_sigaction()

In reply to Oleg Nesterov

Now that allow_signal() is really trivial we can unify it with disallow_signal(). Add the new helper, kernel_sigaction(), and reimplement allow_signal/disallow_signal as a trivial wrappers.

This saves one EXPORT_SYMBOL() and the new helper can have more users.

Signed-off-by Oleg Nesterov <[email protected]>

---
 include/linux/signal.h |   18 ++++++++++++++++--
 kernel/signal.c        |   36 ++++++++++++------------------------
 2 files changed, 28 insertions(+), 26 deletions(-)

include/linux/signal.h +16 -2

index ac83c59..c9e6536 100644
@@ -284,8 +284,22 @@ extern int get_signal_to_deliver(siginfo_t *info, struct k_sigaction *return_ka,
 extern void signal_setup_done(int failed, struct ksignal *ksig, int stepping);
 extern void signal_delivered(int sig, siginfo_t *info, struct k_sigaction *ka, struct pt_regs *regs, int stepping);
 extern void exit_signals(struct task_struct *tsk);
-extern void allow_signal(int);
-extern void disallow_signal(int);
+extern void kernel_sigaction(int, __sighandler_t);
+
+static inline void allow_signal(int 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.
+	 */
+	kernel_sigaction(sig, (__force __sighandler_t)2);
+}
+
+static inline void disallow_signal(int sig)
+{
+	kernel_sigaction(sig, SIG_IGN);
+}
 
 /*
  * Eventually that'll replace get_signal_to_deliver(); macro for now,

kernel/signal.c +12 ~~-24~~

index 4bab1b7..45dba61 100644
@@ -3066,37 +3066,25 @@ COMPAT_SYSCALL_DEFINE4(rt_tgsigqueueinfo,
 #endif
 
 /*
- * Let kernel threads use this to say that they allow a certain signal.
- * Must not be used if kthread was cloned with CLONE_SIGHAND.
+ * For kthreads only, must not be used if cloned with CLONE_SIGHAND
  */
-void allow_signal(int sig)
+void kernel_sigaction(int sig, __sighandler_t action)
 {
-	/*
-	 * 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.
-	 */
 	spin_lock_irq(&current->sighand->siglock);
-	current->sighand->action[(sig)-1].sa.sa_handler = (void __user *)2;
-	spin_unlock_irq(&current->sighand->siglock);
-}
-EXPORT_SYMBOL(allow_signal);
+	current->sighand->action[sig - 1].sa.sa_handler = action;
+	if (action == SIG_IGN) {
+		sigset_t mask;
 
-void disallow_signal(int sig)
-{
-	sigset_t mask;
+		sigemptyset(&mask);
+		sigaddset(&mask, sig);
 
-	sigemptyset(&mask);
-	sigaddset(&mask, sig);
-
-	spin_lock_irq(&current->sighand->siglock);
-	current->sighand->action[(sig)-1].sa.sa_handler = SIG_IGN;
-	flush_sigqueue_mask(&mask, &current->signal->shared_pending);
-	flush_sigqueue_mask(&mask, &current->pending);
-	recalc_sigpending();
+		flush_sigqueue_mask(&mask, &current->signal->shared_pending);
+		flush_sigqueue_mask(&mask, &current->pending);
+		recalc_sigpending();
+	}
 	spin_unlock_irq(&current->sighand->siglock);
 }
-EXPORT_SYMBOL(disallow_signal);
+EXPORT_SYMBOL(kernel_sigaction);
 
 int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact)
 {

Signature · 1 line

1.5.5.1

Oleg Nesterov

author

2014-04-15 18:37 UTC #11 lore raw

PATCH 11/11 signals: change wait_for_helper() to use kernel_sigaction()

In reply to Oleg Nesterov

Now that we have kernel_sigaction() we can change wait_for_helper() to use it and cleanups the code a bit.

Signed-off-by Oleg Nesterov <[email protected]>

---
 kernel/kmod.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

kernel/kmod.c +1 -4

index 6b375af..c18c891 100644
@@ -285,10 +285,7 @@ static int wait_for_helper(void *data)
 	pid_t pid;
 
 	/* If SIGCLD is ignored sys_wait4 won't populate the status. */
-	spin_lock_irq(&current->sighand->siglock);
-	current->sighand->action[SIGCHLD-1].sa.sa_handler = SIG_DFL;
-	spin_unlock_irq(&current->sighand->siglock);
-
+	kernel_sigaction(SIGCHLD, SIG_DFL);
 	pid = kernel_thread(____call_usermodehelper, sub_info, SIGCHLD);
 	if (pid < 0) {
 		sub_info->retval = pid;

Signature · 1 line

1.5.5.1