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>,
Thomas Gleixner <tglx@linutronix.de>,
linux-kernel@vger.kernel.org
Subject: [PATCH 4/6] signal: sigprocmask() should do retarget_shared_pending()
Date: Mon, 11 Apr 2011 19:21:37 +0200 [thread overview]
Message-ID: <20110411172137.GE32469@redhat.com> (raw)
In-Reply-To: <20110411171957.GA32469@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 changes sigprocmask() to call retarget_shared_pending() as
exit_signals() does. To calculate the "blocked" argument we add the new
helper, signorsets(), although we could do ~(newset & ~current->blocked)
instead. According to grep, nobody in arch/ defines __HAVE_ARCH_SIG_SETOPS,
we only need to change linux/signal.h.
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 sigprocmask() 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 | 3 +++
kernel/signal.c | 5 +++++
2 files changed, 8 insertions(+)
--- sigprocmask/include/linux/signal.h~4_sigprocmask_retarget 2011-04-06 21:33:50.000000000 +0200
+++ sigprocmask/include/linux/signal.h 2011-04-11 18:16:51.000000000 +0200
@@ -126,10 +126,14 @@ _SIG_SET_BINOP(sigandsets, _sig_and)
#define _sig_nand(x,y) ((x) & ~(y))
_SIG_SET_BINOP(signandsets, _sig_nand)
+#define _sig_nor(x,y) ((x) | ~(y))
+_SIG_SET_BINOP(signorsets, _sig_nor)
+
#undef _SIG_SET_BINOP
#undef _sig_or
#undef _sig_and
#undef _sig_nand
+#undef _sig_nor
#define _SIG_SET_OP(name, op) \
static inline void name(sigset_t *set) \
--- sigprocmask/kernel/signal.c~4_sigprocmask_retarget 2011-04-10 21:57:42.000000000 +0200
+++ sigprocmask/kernel/signal.c 2011-04-11 18:02:22.000000000 +0200
@@ -2131,6 +2131,11 @@ int sigprocmask(int how, sigset_t *set,
}
spin_lock_irq(&tsk->sighand->siglock);
+ if (signal_pending(tsk) && !thread_group_empty(tsk)) {
+ sigset_t not_newblocked;
+ signorsets(¬_newblocked, ¤t->blocked, &newset);
+ retarget_shared_pending(tsk, ¬_newblocked);
+ }
tsk->blocked = newset;
recalc_sigpending();
spin_unlock_irq(&tsk->sighand->siglock);
next prev parent reply other threads:[~2011-04-11 17:22 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-04-11 17:19 [RFC PATCH 0/6] signal: sigprocmask fixes Oleg Nesterov
2011-04-11 17:20 ` [PATCH 1/6] signal: introduce retarget_shared_pending() Oleg Nesterov
2011-04-12 11:39 ` Matt Fleming
2011-04-11 17:20 ` [PATCH 2/6] signal: retarget_shared_pending: consider shared/unblocked signals only Oleg Nesterov
2011-04-12 11:40 ` Matt Fleming
2011-04-12 19:53 ` Oleg Nesterov
2011-04-11 17:21 ` [PATCH 3/6] signal: sigprocmask: narrow the scope of ->sigloc Oleg Nesterov
2011-04-12 11:38 ` Matt Fleming
2011-04-11 17:21 ` Oleg Nesterov [this message]
2011-04-12 12:07 ` [PATCH 4/6] signal: sigprocmask() should do retarget_shared_pending() Matt Fleming
2011-04-12 14:32 ` Linus Torvalds
2011-04-14 19:36 ` Oleg Nesterov
2011-04-12 18:33 ` Tejun Heo
2011-04-14 20:10 ` Oleg Nesterov
2011-04-14 20:33 ` Linus Torvalds
2011-04-11 17:22 ` [PATCH 5/6] x86: signal: handle_signal() should use sigprocmask() Oleg Nesterov
2011-04-12 12:15 ` Matt Fleming
2011-04-11 17:22 ` [PATCH 6/6] x86: signal: sys_rt_sigreturn() " Oleg Nesterov
2011-04-12 12:17 ` Matt Fleming
2011-04-14 20:15 ` Oleg Nesterov
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=20110411172137.GE32469@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=tglx@linutronix.de \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).