* [PATCH RESEND2 00/11] cleanup/fix allow_signal/disallow_signal
@ 2014-04-15 18:36 Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 01/11] signals: kill sigfindinword() Oleg Nesterov
` (10 more replies)
0 siblings, 11 replies; 12+ messages in thread
From: Oleg Nesterov @ 2014-04-15 18:36 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra
Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel
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(-)
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RESEND2 01/11] signals: kill sigfindinword()
2014-04-15 18:36 [PATCH RESEND2 00/11] cleanup/fix allow_signal/disallow_signal Oleg Nesterov
@ 2014-04-15 18:36 ` Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 02/11] signals: s/siginitset/sigemptyset/ in do_sigtimedwait() Oleg Nesterov
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Oleg Nesterov @ 2014-04-15 18:36 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra
Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel
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 <oleg@redhat.com>
Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
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(-)
diff --git a/arch/m68k/include/asm/signal.h b/arch/m68k/include/asm/signal.h
index 214320b..8c8ce5e 100644
--- a/arch/m68k/include/asm/signal.h
+++ b/arch/m68k/include/asm/signal.h
@@ -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__
diff --git a/arch/x86/include/asm/signal.h b/arch/x86/include/asm/signal.h
index 35e67a4..31eab86 100644
--- a/arch/x86/include/asm/signal.h
+++ b/arch/x86/include/asm/signal.h
@@ -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__ */
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 2ac423b..ae744c3 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -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)
--
1.5.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH RESEND2 02/11] signals: s/siginitset/sigemptyset/ in do_sigtimedwait()
2014-04-15 18:36 [PATCH RESEND2 00/11] cleanup/fix allow_signal/disallow_signal Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 01/11] signals: kill sigfindinword() Oleg Nesterov
@ 2014-04-15 18:36 ` Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 03/11] signals: kill rm_from_queue(), change prepare_signal() to use for_each_thread() Oleg Nesterov
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Oleg Nesterov @ 2014-04-15 18:36 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra
Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel
Cosmetic, but siginitset(0) looks a bit strange, sigemptyset() is what
do_sigtimedwait() needs.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
kernel/signal.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index 6ea13c0..a717749 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -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);
--
1.5.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH RESEND2 03/11] signals: kill rm_from_queue(), change prepare_signal() to use for_each_thread()
2014-04-15 18:36 [PATCH RESEND2 00/11] cleanup/fix allow_signal/disallow_signal Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 01/11] signals: kill sigfindinword() Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 02/11] signals: s/siginitset/sigemptyset/ in do_sigtimedwait() Oleg Nesterov
@ 2014-04-15 18:36 ` Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 04/11] signals: rename rm_from_queue_full() to flush_sigqueue_mask() Oleg Nesterov
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Oleg Nesterov @ 2014-04-15 18:36 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra
Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel
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 <oleg@redhat.com>
---
kernel/signal.c | 43 ++++++++++---------------------------------
1 files changed, 10 insertions(+), 33 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index a717749..6072efa 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -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.
--
1.5.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH RESEND2 04/11] signals: rename rm_from_queue_full() to flush_sigqueue_mask()
2014-04-15 18:36 [PATCH RESEND2 00/11] cleanup/fix allow_signal/disallow_signal Oleg Nesterov
` (2 preceding siblings ...)
2014-04-15 18:36 ` [PATCH RESEND2 03/11] signals: kill rm_from_queue(), change prepare_signal() to use for_each_thread() Oleg Nesterov
@ 2014-04-15 18:36 ` Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 05/11] signals: cleanup the usage of t/current in do_sigaction() Oleg Nesterov
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Oleg Nesterov @ 2014-04-15 18:36 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra
Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel
"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 <oleg@redhat.com>
---
kernel/signal.c | 19 ++++++++-----------
1 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index 6072efa..c0b7d0c 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -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;
--
1.5.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH RESEND2 05/11] signals: cleanup the usage of t/current in do_sigaction()
2014-04-15 18:36 [PATCH RESEND2 00/11] cleanup/fix allow_signal/disallow_signal Oleg Nesterov
` (3 preceding siblings ...)
2014-04-15 18:36 ` [PATCH RESEND2 04/11] signals: rename rm_from_queue_full() to flush_sigqueue_mask() Oleg Nesterov
@ 2014-04-15 18:36 ` Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 06/11] signals: mv {dis,}allow_signal() from sched.h/exit.c to signal.[ch] Oleg Nesterov
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Oleg Nesterov @ 2014-04-15 18:36 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra
Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel
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 <oleg@redhat.com>
---
kernel/signal.c | 15 +++++++--------
1 files changed, 7 insertions(+), 8 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index c0b7d0c..a58b0db 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -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(¤t->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(¤t->sighand->siglock);
+ spin_unlock_irq(&p->sighand->siglock);
return 0;
}
--
1.5.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH RESEND2 06/11] signals: mv {dis,}allow_signal() from sched.h/exit.c to signal.[ch]
2014-04-15 18:36 [PATCH RESEND2 00/11] cleanup/fix allow_signal/disallow_signal Oleg Nesterov
` (4 preceding siblings ...)
2014-04-15 18:36 ` [PATCH RESEND2 05/11] signals: cleanup the usage of t/current in do_sigaction() Oleg Nesterov
@ 2014-04-15 18:36 ` Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 07/11] signals: jffs2: fix the wrong usage of disallow_signal() Oleg Nesterov
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Oleg Nesterov @ 2014-04-15 18:36 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra
Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel
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 <oleg@redhat.com>
---
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(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index a04214f..82fe3da 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -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 *);
diff --git a/include/linux/signal.h b/include/linux/signal.h
index ae744c3..ac83c59 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -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,
diff --git a/kernel/exit.c b/kernel/exit.c
index 6ed6a1d..ad7183a 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -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(¤t->sighand->siglock);
- /* This is only needed for daemonize()'ed kthreads */
- sigdelset(¤t->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(¤t->sighand->siglock);
- return 0;
-}
-
-EXPORT_SYMBOL(allow_signal);
-
-int disallow_signal(int sig)
-{
- if (!valid_signal(sig) || sig < 1)
- return -EINVAL;
-
- spin_lock_irq(¤t->sighand->siglock);
- current->sighand->action[(sig)-1].sa.sa_handler = SIG_IGN;
- recalc_sigpending();
- spin_unlock_irq(¤t->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.
diff --git a/kernel/signal.c b/kernel/signal.c
index a58b0db..8a7ee44 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -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(¤t->sighand->siglock);
+ /* This is only needed for daemonize()'ed kthreads */
+ sigdelset(¤t->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(¤t->sighand->siglock);
+}
+EXPORT_SYMBOL(allow_signal);
+
+void disallow_signal(int sig)
+{
+ spin_lock_irq(¤t->sighand->siglock);
+ current->sighand->action[(sig)-1].sa.sa_handler = SIG_IGN;
+ recalc_sigpending();
+ spin_unlock_irq(¤t->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;
--
1.5.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH RESEND2 07/11] signals: jffs2: fix the wrong usage of disallow_signal()
2014-04-15 18:36 [PATCH RESEND2 00/11] cleanup/fix allow_signal/disallow_signal Oleg Nesterov
` (5 preceding siblings ...)
2014-04-15 18:36 ` [PATCH RESEND2 06/11] signals: mv {dis,}allow_signal() from sched.h/exit.c to signal.[ch] Oleg Nesterov
@ 2014-04-15 18:36 ` Oleg Nesterov
2014-04-15 18:37 ` [PATCH RESEND2 08/11] signals: kill the obsolete sigdelset() and recalc_sigpending() in allow_signal() Oleg Nesterov
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Oleg Nesterov @ 2014-04-15 18:36 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra
Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel
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 <oleg@redhat.com>
---
fs/jffs2/background.c | 12 +++++++-----
1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/fs/jffs2/background.c b/fs/jffs2/background.c
index 2b60ce1..bb9cebc 100644
--- a/fs/jffs2/background.c
+++ b/fs/jffs2/background.c
@@ -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) {
--
1.5.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH RESEND2 08/11] signals: kill the obsolete sigdelset() and recalc_sigpending() in allow_signal()
2014-04-15 18:36 [PATCH RESEND2 00/11] cleanup/fix allow_signal/disallow_signal Oleg Nesterov
` (6 preceding siblings ...)
2014-04-15 18:36 ` [PATCH RESEND2 07/11] signals: jffs2: fix the wrong usage of disallow_signal() Oleg Nesterov
@ 2014-04-15 18:37 ` Oleg Nesterov
2014-04-15 18:37 ` [PATCH RESEND2 09/11] signals: disallow_signal() should flush the potentially pending signal Oleg Nesterov
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Oleg Nesterov @ 2014-04-15 18:37 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra
Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel
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 <oleg@redhat.com>
---
kernel/signal.c | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index 8a7ee44..3eec27b 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3071,16 +3071,13 @@ COMPAT_SYSCALL_DEFINE4(rt_tgsigqueueinfo,
*/
void allow_signal(int sig)
{
- spin_lock_irq(¤t->sighand->siglock);
- /* This is only needed for daemonize()'ed kthreads */
- sigdelset(¤t->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(¤t->sighand->siglock);
current->sighand->action[(sig)-1].sa.sa_handler = (void __user *)2;
- recalc_sigpending();
spin_unlock_irq(¤t->sighand->siglock);
}
EXPORT_SYMBOL(allow_signal);
--
1.5.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH RESEND2 09/11] signals: disallow_signal() should flush the potentially pending signal
2014-04-15 18:36 [PATCH RESEND2 00/11] cleanup/fix allow_signal/disallow_signal Oleg Nesterov
` (7 preceding siblings ...)
2014-04-15 18:37 ` [PATCH RESEND2 08/11] signals: kill the obsolete sigdelset() and recalc_sigpending() in allow_signal() Oleg Nesterov
@ 2014-04-15 18:37 ` Oleg Nesterov
2014-04-15 18:37 ` [PATCH RESEND2 10/11] signals: introduce kernel_sigaction() Oleg Nesterov
2014-04-15 18:37 ` [PATCH RESEND2 11/11] signals: change wait_for_helper() to use kernel_sigaction() Oleg Nesterov
10 siblings, 0 replies; 12+ messages in thread
From: Oleg Nesterov @ 2014-04-15 18:37 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra
Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel
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 <oleg@redhat.com>
---
kernel/signal.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index 3eec27b..4bab1b7 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3084,8 +3084,15 @@ EXPORT_SYMBOL(allow_signal);
void disallow_signal(int sig)
{
+ sigset_t mask;
+
+ sigemptyset(&mask);
+ sigaddset(&mask, sig);
+
spin_lock_irq(¤t->sighand->siglock);
current->sighand->action[(sig)-1].sa.sa_handler = SIG_IGN;
+ flush_sigqueue_mask(&mask, ¤t->signal->shared_pending);
+ flush_sigqueue_mask(&mask, ¤t->pending);
recalc_sigpending();
spin_unlock_irq(¤t->sighand->siglock);
}
--
1.5.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH RESEND2 10/11] signals: introduce kernel_sigaction()
2014-04-15 18:36 [PATCH RESEND2 00/11] cleanup/fix allow_signal/disallow_signal Oleg Nesterov
` (8 preceding siblings ...)
2014-04-15 18:37 ` [PATCH RESEND2 09/11] signals: disallow_signal() should flush the potentially pending signal Oleg Nesterov
@ 2014-04-15 18:37 ` Oleg Nesterov
2014-04-15 18:37 ` [PATCH RESEND2 11/11] signals: change wait_for_helper() to use kernel_sigaction() Oleg Nesterov
10 siblings, 0 replies; 12+ messages in thread
From: Oleg Nesterov @ 2014-04-15 18:37 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra
Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel
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 <oleg@redhat.com>
---
include/linux/signal.h | 18 ++++++++++++++++--
kernel/signal.c | 36 ++++++++++++------------------------
2 files changed, 28 insertions(+), 26 deletions(-)
diff --git a/include/linux/signal.h b/include/linux/signal.h
index ac83c59..c9e6536 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -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,
diff --git a/kernel/signal.c b/kernel/signal.c
index 4bab1b7..45dba61 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -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(¤t->sighand->siglock);
- current->sighand->action[(sig)-1].sa.sa_handler = (void __user *)2;
- spin_unlock_irq(¤t->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(¤t->sighand->siglock);
- current->sighand->action[(sig)-1].sa.sa_handler = SIG_IGN;
- flush_sigqueue_mask(&mask, ¤t->signal->shared_pending);
- flush_sigqueue_mask(&mask, ¤t->pending);
- recalc_sigpending();
+ flush_sigqueue_mask(&mask, ¤t->signal->shared_pending);
+ flush_sigqueue_mask(&mask, ¤t->pending);
+ recalc_sigpending();
+ }
spin_unlock_irq(¤t->sighand->siglock);
}
-EXPORT_SYMBOL(disallow_signal);
+EXPORT_SYMBOL(kernel_sigaction);
int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact)
{
--
1.5.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH RESEND2 11/11] signals: change wait_for_helper() to use kernel_sigaction()
2014-04-15 18:36 [PATCH RESEND2 00/11] cleanup/fix allow_signal/disallow_signal Oleg Nesterov
` (9 preceding siblings ...)
2014-04-15 18:37 ` [PATCH RESEND2 10/11] signals: introduce kernel_sigaction() Oleg Nesterov
@ 2014-04-15 18:37 ` Oleg Nesterov
10 siblings, 0 replies; 12+ messages in thread
From: Oleg Nesterov @ 2014-04-15 18:37 UTC (permalink / raw)
To: Andrew Morton, Peter Zijlstra
Cc: Al Viro, David Woodhouse, Frederic Weisbecker,
Geert Uytterhoeven, Ingo Molnar, Mathieu Desnoyers,
Richard Weinberger, Steven Rostedt, Tejun Heo, linux-kernel
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 <oleg@redhat.com>
---
kernel/kmod.c | 5 +----
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/kernel/kmod.c b/kernel/kmod.c
index 6b375af..c18c891 100644
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -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(¤t->sighand->siglock);
- current->sighand->action[SIGCHLD-1].sa.sa_handler = SIG_DFL;
- spin_unlock_irq(¤t->sighand->siglock);
-
+ kernel_sigaction(SIGCHLD, SIG_DFL);
pid = kernel_thread(____call_usermodehelper, sub_info, SIGCHLD);
if (pid < 0) {
sub_info->retval = pid;
--
1.5.5.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-04-15 18:49 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-04-15 18:36 [PATCH RESEND2 00/11] cleanup/fix allow_signal/disallow_signal Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 01/11] signals: kill sigfindinword() Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 02/11] signals: s/siginitset/sigemptyset/ in do_sigtimedwait() Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 03/11] signals: kill rm_from_queue(), change prepare_signal() to use for_each_thread() Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 04/11] signals: rename rm_from_queue_full() to flush_sigqueue_mask() Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 05/11] signals: cleanup the usage of t/current in do_sigaction() Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 06/11] signals: mv {dis,}allow_signal() from sched.h/exit.c to signal.[ch] Oleg Nesterov
2014-04-15 18:36 ` [PATCH RESEND2 07/11] signals: jffs2: fix the wrong usage of disallow_signal() Oleg Nesterov
2014-04-15 18:37 ` [PATCH RESEND2 08/11] signals: kill the obsolete sigdelset() and recalc_sigpending() in allow_signal() Oleg Nesterov
2014-04-15 18:37 ` [PATCH RESEND2 09/11] signals: disallow_signal() should flush the potentially pending signal Oleg Nesterov
2014-04-15 18:37 ` [PATCH RESEND2 10/11] signals: introduce kernel_sigaction() Oleg Nesterov
2014-04-15 18:37 ` [PATCH RESEND2 11/11] signals: change wait_for_helper() to use kernel_sigaction() Oleg Nesterov
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.