linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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, &current->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;
 }
 


  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 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).