linux-kernel - [PATCH v2 3/6] signal: do_sigtimedwait() needs retarget_shared

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110426194925.GD8520@redhat.com>
Date:	Tue, 26 Apr 2011 21:49:25 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Tejun Heo <tj@...nel.org>,
	"Nikita V. Youshchenko" <nyoushchenko@...sta.com>,
	Matt Fleming <matt@...sole-pimps.org>,
	linux-kernel@...r.kernel.org
Subject: [PATCH v2 3/6] signal: do_sigtimedwait() needs
	retarget_shared_pending()

do_sigtimedwait() changes current->blocked and thus it needs
set_current_blocked()->retarget_shared_pending().

We could use set_current_blocked() directly. It is fine to change
->real_blocked from all-zeroes to ->blocked and vice versa lockless,
but this is not immediately clear, looks racy, and needs a huge
comment to explain why this is correct.

To keep the things simple this patch adds the new static helper,
__set_task_blocked() which should be called with ->siglock held. This
way we can change both ->real_blocked and ->blocked atomically under
->siglock as the current code does. This is more understandable.

Signed-off-by: Oleg Nesterov <oleg@...hat.com>
Acked-by: Tejun Heo <tj@...nel.org>
Reviewed-by: Matt Fleming <matt.fleming@...ux.intel.com>
---

 kernel/signal.c |   21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

--- sigprocmask/kernel/signal.c~3_sigtimedwait_retarget	2011-04-26 19:53:42.000000000 +0200
+++ sigprocmask/kernel/signal.c	2011-04-26 19:53:55.000000000 +0200
@@ -2115,11 +2115,8 @@ long do_no_restart_syscall(struct restar
 	return -EINTR;
 }
 
-void set_current_blocked(const sigset_t *newset)
+static void __set_task_blocked(struct task_struct *tsk, 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);
@@ -2127,6 +2124,14 @@ void set_current_blocked(const sigset_t 
 	}
 	tsk->blocked = *newset;
 	recalc_sigpending();
+}
+
+void set_current_blocked(const sigset_t *newset)
+{
+	struct task_struct *tsk = current;
+
+	spin_lock_irq(&tsk->sighand->siglock);
+	__set_task_blocked(tsk, newset);
 	spin_unlock_irq(&tsk->sighand->siglock);
 }
 
@@ -2343,7 +2348,8 @@ int do_sigtimedwait(const sigset_t *whic
 		/*
 		 * None ready, temporarily unblock those we're interested
 		 * while we are sleeping in so that we'll be awakened when
-		 * they arrive.
+		 * they arrive. Unblocking is always fine, we can avoid
+		 * set_current_blocked().
 		 */
 		tsk->real_blocked = tsk->blocked;
 		sigandsets(&tsk->blocked, &tsk->blocked, &mask);
@@ -2353,10 +2359,9 @@ int do_sigtimedwait(const sigset_t *whic
 		timeout = schedule_timeout_interruptible(timeout);
 
 		spin_lock_irq(&tsk->sighand->siglock);
-		sig = dequeue_signal(tsk, &mask, info);
-		tsk->blocked = tsk->real_blocked;
+		__set_task_blocked(tsk, &tsk->real_blocked);
 		siginitset(&tsk->real_blocked, 0);
-		recalc_sigpending();
+		sig = dequeue_signal(tsk, &mask, info);
 	}
 	spin_unlock_irq(&tsk->sighand->siglock);
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/