From: Oleg Nesterov <oleg@redhat.com>
To: Tejun Heo <tj@kernel.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
Andrew Morton <akpm@linux-foundation.org>
Cc: "Nikita V. Youshchenko" <nyoushchenko@mvista.com>,
Matt Fleming <matt@console-pimps.org>,
linux-kernel@vger.kernel.org
Subject: [PATCH v2 5/7] signal: sigprocmask() should do retarget_shared_pending()
Date: Mon, 18 Apr 2011 15:45:57 +0200 [thread overview]
Message-ID: <20110418134557.GF15951@redhat.com> (raw)
In-Reply-To: <20110418134421.GA15951@redhat.com>
In short, almost every changing of current->blocked is wrong, or at least
can lead to the unexpected results.
For example. Two threads T1 and T2, T1 sleeps in sigtimedwait/pause/etc.
kill(tgid, SIG) can pick T2 for TIF_SIGPENDING. If T2 calls sigprocmask()
and blocks SIG before it notices the pending signal, nobody else can handle
this pending shared signal.
I am not sure this is bug, but at least this looks strange imho. T1 should
not sleep forever, there is a signal which should wake it up.
This patch moves the code which actually changes ->blocked into the new
helper, set_current_blocked() and changes this code to call
retarget_shared_pending() as exit_signals() does. We should only care about
the signals we just blocked, we use "newset & ~current->blocked" as a mask.
We do not check !sigisemptyset(newblocked), retarget_shared_pending() is
cheap unless mask & shared_pending.
Note: for this particular case we could simply change sigprocmask() to
return -EINTR if signal_pending(), but then we should change other callers
and, more importantly, if we need this fix then set_current_blocked() will
have more callers and some of them can't restart. See the next patch as a
random example.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
include/linux/signal.h | 1 +
kernel/signal.c | 21 ++++++++++++++++-----
2 files changed, 17 insertions(+), 5 deletions(-)
--- sigprocmask/include/linux/signal.h~5_sigprocmask_retarget 2011-04-17 22:23:29.000000000 +0200
+++ sigprocmask/include/linux/signal.h 2011-04-17 22:36:41.000000000 +0200
@@ -243,6 +243,7 @@ extern long do_rt_tgsigqueueinfo(pid_t t
siginfo_t *info);
extern long do_sigpending(void __user *, unsigned long);
extern int sigprocmask(int, sigset_t *, sigset_t *);
+extern void set_current_blocked(const sigset_t *);
extern int show_unhandled_signals;
struct pt_regs;
--- sigprocmask/kernel/signal.c~5_sigprocmask_retarget 2011-04-17 22:16:44.000000000 +0200
+++ sigprocmask/kernel/signal.c 2011-04-17 22:32:58.000000000 +0200
@@ -2115,6 +2115,21 @@ long do_no_restart_syscall(struct restar
return -EINTR;
}
+void set_current_blocked(const sigset_t *newset)
+{
+ struct task_struct *tsk = current;
+
+ spin_lock_irq(&tsk->sighand->siglock);
+ if (signal_pending(tsk) && !thread_group_empty(tsk)) {
+ sigset_t newblocked;
+ signandsets(&newblocked, newset, ¤t->blocked);
+ retarget_shared_pending(tsk, &newblocked);
+ }
+ tsk->blocked = *newset;
+ recalc_sigpending();
+ spin_unlock_irq(&tsk->sighand->siglock);
+}
+
/*
* This is also useful for kernel threads that want to temporarily
* (or permanently) block certain signals.
@@ -2146,11 +2161,7 @@ int sigprocmask(int how, sigset_t *set,
return -EINVAL;
}
- spin_lock_irq(&tsk->sighand->siglock);
- tsk->blocked = newset;
- recalc_sigpending();
- spin_unlock_irq(&tsk->sighand->siglock);
-
+ set_current_blocked(&newset);
return 0;
}
next prev parent reply other threads:[~2011-04-18 13:47 UTC|newest]
Thread overview: 88+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-18 13:44 [RFC PATCH v2 0/7] signal: sigprocmask fixes Oleg Nesterov
2011-04-18 13:44 ` [PATCH v2 1/7] signal: introduce retarget_shared_pending() Oleg Nesterov
2011-04-22 12:04 ` Matt Fleming
2011-04-25 10:49 ` Tejun Heo
2011-04-18 13:45 ` [PATCH v2 2/7] signal: retarget_shared_pending: consider shared/unblocked signals only Oleg Nesterov
2011-04-22 12:22 ` Matt Fleming
2011-04-25 10:52 ` Tejun Heo
2011-04-25 15:20 ` Oleg Nesterov
2011-04-25 16:19 ` Tejun Heo
2011-04-25 17:02 ` Oleg Nesterov
2011-04-25 17:11 ` Tejun Heo
2011-04-26 19:45 ` Oleg Nesterov
2011-04-28 15:26 ` [PATCHSET] signals-review branch Oleg Nesterov
2011-04-30 12:51 ` Tejun Heo
2011-04-18 13:45 ` [PATCH v2 3/7] signal: retarget_shared_pending: optimize while_each_thread() loop Oleg Nesterov
2011-04-22 12:26 ` Matt Fleming
2011-04-25 11:03 ` Tejun Heo
2011-04-18 13:45 ` [PATCH v2 4/7] signal: sigprocmask: narrow the scope of ->siglock Oleg Nesterov
2011-04-22 12:31 ` Matt Fleming
2011-04-25 11:05 ` Tejun Heo
2011-04-18 13:45 ` Oleg Nesterov [this message]
2011-04-22 12:46 ` [PATCH v2 5/7] signal: sigprocmask() should do retarget_shared_pending() Matt Fleming
2011-04-25 11:14 ` Tejun Heo
2011-04-18 13:46 ` [PATCH v2 6/7] x86: signal: handle_signal() should use set_current_blocked() Oleg Nesterov
2011-04-22 13:45 ` Matt Fleming
2011-04-25 11:19 ` Tejun Heo
2011-04-18 13:46 ` [PATCH v2 7/7] x86: signal: sys_rt_sigreturn() " Oleg Nesterov
2011-04-22 14:14 ` Matt Fleming
2011-04-23 18:12 ` Oleg Nesterov
2011-04-25 11:21 ` Tejun Heo
2011-04-18 13:47 ` [PATCH v2 8/7] signal: cleanup sys_rt_sigprocmask() Oleg Nesterov
2011-04-22 14:30 ` Matt Fleming
2011-04-23 18:20 ` Oleg Nesterov
2011-04-23 18:47 ` Matt Fleming
2011-04-25 11:26 ` Tejun Heo
2011-04-18 17:16 ` [RFC PATCH v2 0/7] signal: sigprocmask fixes Linus Torvalds
2011-04-18 17:32 ` Oleg Nesterov
2011-04-18 17:40 ` Linus Torvalds
2011-04-23 17:59 ` [PATCH 0/3] do_sigtimedwait() needs retarget_shared_pending() Oleg Nesterov
2011-04-23 17:59 ` [PATCH 1/3] signal: sys_rt_sigtimedwait: simplify the timeout logic Oleg Nesterov
2011-04-25 11:37 ` Tejun Heo
2011-04-25 17:26 ` Oleg Nesterov
2011-04-25 17:34 ` Linus Torvalds
2011-04-25 17:56 ` Oleg Nesterov
2011-04-25 19:38 ` Linus Torvalds
2011-04-26 10:18 ` Matt Fleming
2011-04-23 17:59 ` [PATCH 2/3] signal: introduce do_sigtimedwait() to factor out compat/native code Oleg Nesterov
2011-04-25 11:39 ` Tejun Heo
2011-04-25 11:49 ` Tejun Heo
2011-04-25 15:33 ` Oleg Nesterov
2011-04-25 16:25 ` Tejun Heo
2011-04-26 10:28 ` Matt Fleming
2011-04-23 18:00 ` [PATCH 3/3] signal: do_sigtimedwait() needs retarget_shared_pending() Oleg Nesterov
2011-04-25 11:52 ` Tejun Heo
2011-04-25 16:01 ` Oleg Nesterov
2011-04-25 16:27 ` Tejun Heo
2011-04-25 17:07 ` Oleg Nesterov
2011-04-25 17:12 ` Tejun Heo
2011-04-26 10:40 ` Matt Fleming
2011-04-26 10:42 ` Matt Fleming
2011-04-26 19:48 ` [PATCH v2 0/6] sigtimedwait/sigprocmask need retarget_shared_pending() Oleg Nesterov
2011-04-26 19:48 ` [PATCH v2 1/6] signal: sys_rt_sigtimedwait: simplify the timeout logic Oleg Nesterov
2011-04-26 19:49 ` [PATCH v2 2/6] signal: introduce do_sigtimedwait() to factor out compat/native code Oleg Nesterov
2011-04-27 10:09 ` Tejun Heo
2011-04-27 21:24 ` Matt Fleming
2011-05-11 16:21 ` Mike Frysinger
2011-05-12 18:54 ` Oleg Nesterov
2011-05-13 16:44 ` [PATCH] signal: trivial, fix the "timespec declared inside parameter list" warning Oleg Nesterov
2011-05-13 18:09 ` Mike Frysinger
2011-05-16 12:57 ` Oleg Nesterov
2011-05-16 12:57 ` [PATCH v2] " Oleg Nesterov
2011-05-16 17:39 ` Mike Frysinger
2011-05-18 23:37 ` Andrew Morton
2011-05-19 18:19 ` Oleg Nesterov
2011-05-19 19:21 ` Mike Frysinger
2011-04-26 19:49 ` [PATCH v2 3/6] signal: do_sigtimedwait() needs retarget_shared_pending() Oleg Nesterov
2011-04-26 19:49 ` [PATCH v2 4/6] signal: cleanup sys_sigprocmask() Oleg Nesterov
2011-04-27 10:12 ` Tejun Heo
2011-04-27 21:31 ` Matt Fleming
2011-04-26 19:50 ` [PATCH v2 5/6] signal: sys_sigprocmask() needs retarget_shared_pending() Oleg Nesterov
2011-04-26 21:43 ` Linus Torvalds
2011-04-27 12:57 ` Oleg Nesterov
2011-04-27 13:04 ` Tejun Heo
2011-05-01 20:07 ` [PATCH v2 0/1] " Oleg Nesterov
2011-05-01 20:08 ` [PATCH v2 1/1] " Oleg Nesterov
2011-04-26 19:50 ` [PATCH v2 6/6] signal: rename signandsets() to sigandnsets() Oleg Nesterov
2011-04-27 10:11 ` Tejun Heo
2011-04-27 21:43 ` Matt Fleming
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110418134557.GF15951@redhat.com \
--to=oleg@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=matt@console-pimps.org \
--cc=nyoushchenko@mvista.com \
--cc=tj@kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.