linux-rt-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH rt] Fix races in ptrace
@ 2013-07-24 16:23 Alexander Fyodorov
  2013-08-12 16:41 ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 11+ messages in thread
From: Alexander Fyodorov @ 2013-07-24 16:23 UTC (permalink / raw)
  To: linux-rt-users

Hi all,

I ran into a race on 2.6.33-rt kernel which I think still exists in the latest RT patches. The following patch fixes it but it may be incomplete: probably spinlock should be taken in all other places that check saved_state too.  I only compile-tested this patch for 3.6-rt.


read_lock(&tasklist_lock) in ptrace_stop() is converted to mutex on RT kernel, and it can remove __TASK_TRACED from task->state (by moving it to task->saved_state). If parent does wait() on child followed by a sys_ptrace call, the following race can happen:

 - child sets __TASK_TRACED in ptrace_stop()
 - parent does wait() which eventually calls wait_task_stopped() and returns child's pid
 - child blocks on read_lock(&tasklist_lock) in ptrace_stop() and moves __TASK_TRACED flag to saved_state
 - parent calls sys_ptrace, which calls ptrace_check_attach() and wait_task_inactive()

ptrace_check_attach() checks only child->state and will fail, and wait_task_inactive() checks both child->state and child->saved_state but it doesn't hold child->pi_lock so it can fail because of memory ordering too. Locking child->pi_lock and checking saved_state solves both races.

Signed-off-by: Alexander Fyodorov <halcy <at> yandex.ru>

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index a232bb5..79b5f5d 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -159,8 +159,20 @@ int ptrace_check_attach(struct task_struct *child, bool ignore_state)
 		spin_lock_irq(&child->sighand->siglock);
 		WARN_ON_ONCE(task_is_stopped(child));
 		if (ignore_state || (task_is_traced(child) &&
-				     !(child->jobctl & JOBCTL_LISTENING)))
+				     !(child->jobctl & JOBCTL_LISTENING))) {
 			ret = 0;
+		} else {
+			/*
+			 * Test again before failing, but this time take
+			 * the spinlock and look at child->saved_state
+			 */
+			raw_spin_lock(&child->pi_lock);
+			if ((task_is_traced(child) ||
+					(child->saved_state & __TASK_TRACED)) &&
+					!(child->jobctl & JOBCTL_LISTENING))
+				ret = 0;
+			raw_spin_unlock(&child->pi_lock);
+		}
 		spin_unlock_irq(&child->sighand->siglock);
 	}
 	read_unlock(&tasklist_lock);
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 5051ca6..a9cb295 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1215,8 +1215,21 @@ unsigned long wait_task_inactive(struct task_struct *p, long match_state)
 		 */
 		while (task_running(rq, p)) {
 			if (match_state && unlikely(p->state != match_state)
-			    && unlikely(p->saved_state != match_state))
-				return 0;
+			    && unlikely(p->saved_state != match_state)) {
+				/*
+				 * Check again under spinlock
+				 */
+				raw_spin_lock_irqsave(&p->pi_lock, flags);
+
+				if (unlikely(p->state != match_state &&
+					     p->saved_state != match_state)) {
+					raw_spin_unlock_irqrestore(&p->pi_lock,
+							flags);
+					return 0;
+				}
+
+				raw_spin_unlock_irqrestore(&p->pi_lock, flags);
+			}
 			cpu_relax();
 		}
 

^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2013-11-30 20:07 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-07-24 16:23 [PATCH rt] Fix races in ptrace Alexander Fyodorov
2013-08-12 16:41 ` Sebastian Andrzej Siewior
2013-08-12 21:13   ` Alexander Fyodorov
2013-08-21 17:24     ` Sebastian Andrzej Siewior
2013-08-22 14:23       ` Alexander Fyodorov
2013-08-29 16:33         ` Sebastian Andrzej Siewior
2013-08-29 17:26           ` Alexander Fyodorov
2013-08-29 18:28             ` Sebastian Andrzej Siewior
2013-08-29 18:47               ` Alexander Fyodorov
2013-08-29 18:49                 ` Sebastian Andrzej Siewior
2013-11-30 20:07                   ` [PATCH v2] ptrace: fix ptrace vs tasklist_lock race Sebastian Andrzej Siewior

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