linux-kernel - [PATCH 15/16] ptrace: make sure SIGNAL_NOTIFY_CONT is checked after ptrace

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: <1291654624-6230-16-git-send-email-tj@kernel.org>
Date:	Mon,  6 Dec 2010 17:57:03 +0100
From:	Tejun Heo <tj@...nel.org>
To:	oleg@...hat.com, roland@...hat.com, linux-kernel@...r.kernel.org,
	torvalds@...ux-foundation.org, akpm@...ux-foundation.org,
	rjw@...k.pl, jan.kratochvil@...hat.com
Cc:	Tejun Heo <tj@...nel.org>
Subject: [PATCH 15/16] ptrace: make sure SIGNAL_NOTIFY_CONT is checked after ptrace_signal()

ptrace_signal() releases siglock and signal delivery may continue
afterwards.  SIGNAL_NOTIFY_CONT can be set inbetween and should be
checked after returning from the function.

* Restart from the top if ptrace_signal() returns 0.

* Factor out CLD_CONTINUED check code into notify_parent_cont() and
  check before returning from get_signal_to_deliver() too.

With the latter, the former isn't strictly necessary but it's still
better to do it to document what's going on if for nothing else.

Signed-off-by: Tejun Heo <tj@...nel.org>
Cc: Oleg Nesterov <oleg@...hat.com>
Cc: Roland McGrath <roland@...hat.com>
---
 kernel/signal.c |   38 +++++++++++++++++++++++++++-----------
 1 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 7b6f972..5eddda6 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -2019,6 +2019,19 @@ static int ptrace_signal(int signr, siginfo_t *info,
 	return signr;
 }
 
+static inline void notify_parent_cont(void)
+{
+	/*
+	 * Every stopped thread should go through this function after
+	 * waking up.  Check to see if we should notify the parent.
+	 */
+	if (unlikely(current->signal->flags & SIGNAL_NOTIFY_CONT)) {
+		read_lock(&tasklist_lock);
+		do_notify_parent_cldstop(current->group_leader, CLD_CONTINUED);
+		read_unlock(&tasklist_lock);
+	}
+}
+
 int get_signal_to_deliver(siginfo_t *info, struct k_sigaction *return_ka,
 			  struct pt_regs *regs, void *cookie)
 {
@@ -2035,15 +2048,7 @@ relock:
 	 */
 	try_to_freeze();
 
-	/*
-	 * Every stopped thread should go through this function after
-	 * waking up.  Check to see if we should notify the parent.
-	 */
-	if (unlikely(current->signal->flags & SIGNAL_NOTIFY_CONT)) {
-		read_lock(&tasklist_lock);
-		do_notify_parent_cldstop(current, CLD_CONTINUED);
-		read_unlock(&tasklist_lock);
-	}
+	notify_parent_cont();
 
 	spin_lock_irq(&sighand->siglock);
 
@@ -2073,8 +2078,11 @@ relock:
 			if (signr != SIGKILL) {
 				signr = ptrace_signal(signr, info,
 						      regs, cookie);
-				if (!signr)
-					continue;
+				if (!signr) {
+					/* siglock was released, restart */
+					spin_unlock_irq(&sighand->siglock);
+					goto relock;
+				}
 			}
 
 			ka = &sighand->action[signr-1];
@@ -2177,6 +2185,14 @@ relock:
 		/* NOTREACHED */
 	}
 	spin_unlock_irq(&sighand->siglock);
+
+	/*
+	 * If ptrace_signal() returned a non-zero signal, control can reach
+	 * here without other pending signals or going through relocking
+	 * and a CONT notification may be left pending.  Check it.
+	 */
+	notify_parent_cont();
+
 	return signr;
 }
 
-- 
1.7.1

--
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/