* [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
* Re: [PATCH rt] Fix races in ptrace
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
0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-08-12 16:41 UTC (permalink / raw)
To: Alexander Fyodorov; +Cc: linux-rt-users
* Alexander Fyodorov | 2013-07-24 20:23:27 [+0400]:
>Hi all,
Hi Alexander,
>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.
It looks interresting, thanks for the report. Do you have maybe a
test-case which can provoke the bug?
Sebastian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH rt] Fix races in ptrace
2013-08-12 16:41 ` Sebastian Andrzej Siewior
@ 2013-08-12 21:13 ` Alexander Fyodorov
2013-08-21 17:24 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 11+ messages in thread
From: Alexander Fyodorov @ 2013-08-12 21:13 UTC (permalink / raw)
To: Sebastian Andrzej Siewior; +Cc: linux-rt-users@vger.kernel.org
12.08.2013, 20:41, "Sebastian Andrzej Siewior" <bigeasy@linutronix.de>:
> * Alexander Fyodorov | 2013-07-24 20:23:27 [+0400]:
> Hi Alexander,
Hi Sebastian,
>> 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.
>
> It looks interresting, thanks for the report. Do you have maybe a
> test-case which can provoke the bug?
I can't publish the test but essentially it was doing the following: one process (application) has 20 threads sending signals to each other (creating contention for tasklist_lock and profiling events), and another one (profiler) attaches to the application and then does:
for (;;) {
pid = wait();
ptrace(PTRACE_CONT, pid);
}
Is there a need to write a test?
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH rt] Fix races in ptrace
2013-08-12 21:13 ` Alexander Fyodorov
@ 2013-08-21 17:24 ` Sebastian Andrzej Siewior
2013-08-22 14:23 ` Alexander Fyodorov
0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-08-21 17:24 UTC (permalink / raw)
To: Alexander Fyodorov; +Cc: linux-rt-users@vger.kernel.org
* Alexander Fyodorov | 2013-08-13 01:13:56 [+0400]:
>Hi Sebastian,
Hi Alexander,
>Is there a need to write a test?
It is nice to have so it can be checked if this bug is fixed "some place
else".
Now that I looked at it for a while, would this fix your trouble?
diff --git a/kernel/rtmutex.c b/kernel/rtmutex.c
--- a/kernel/rtmutex.c
+++ b/kernel/rtmutex.c
@@ -724,6 +724,7 @@ static void noinline __sched rt_spin_lock_slowlock(struct rt_mutex *lock)
struct task_struct *lock_owner, *self = current;
struct rt_mutex_waiter waiter, *top_waiter;
int ret;
+ int new_state;
rt_mutex_init_waiter(&waiter, true);
@@ -744,8 +745,11 @@ static void noinline __sched rt_spin_lock_slowlock(struct rt_mutex *lock)
* try_to_wake_up().
*/
pi_lock(&self->pi_lock);
+ new_state = TASK_UNINTERRUPTIBLE;
+ if (task_is_traced(self))
+ new_state |= __TASK_TRACED;
self->saved_state = self->state;
- __set_current_state(TASK_UNINTERRUPTIBLE);
+ __set_current_state(new_state);
pi_unlock(&self->pi_lock);
ret = task_blocks_on_rt_mutex(lock, &waiter, self, 0);
This should avoid that the trace state is lost while waiting on that
mutex and the checks for "is traced" may remain the same.
Sebastian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH rt] Fix races in ptrace
2013-08-21 17:24 ` Sebastian Andrzej Siewior
@ 2013-08-22 14:23 ` Alexander Fyodorov
2013-08-29 16:33 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 11+ messages in thread
From: Alexander Fyodorov @ 2013-08-22 14:23 UTC (permalink / raw)
To: Sebastian Andrzej Siewior; +Cc: linux-rt-users@vger.kernel.org
21.08.2013, 21:24, "Sebastian Andrzej Siewior" <bigeasy@linutronix.de>:
> Now that I looked at it for a while, would this fix your trouble?
>
> diff --git a/kernel/rtmutex.c b/kernel/rtmutex.c
> --- a/kernel/rtmutex.c
> +++ b/kernel/rtmutex.c
> @@ -724,6 +724,7 @@ static void noinline __sched rt_spin_lock_slowlock(struct rt_mutex *lock)
> struct task_struct *lock_owner, *self = current;
> struct rt_mutex_waiter waiter, *top_waiter;
> int ret;
> + int new_state;
>
> rt_mutex_init_waiter(&waiter, true);
>
> @@ -744,8 +745,11 @@ static void noinline __sched rt_spin_lock_slowlock(struct rt_mutex *lock)
> * try_to_wake_up().
> */
> pi_lock(&self->pi_lock);
> + new_state = TASK_UNINTERRUPTIBLE;
> + if (task_is_traced(self))
> + new_state |= __TASK_TRACED;
> self->saved_state = self->state;
> - __set_current_state(TASK_UNINTERRUPTIBLE);
> + __set_current_state(new_state);
> pi_unlock(&self->pi_lock);
>
> ret = task_blocks_on_rt_mutex(lock, &waiter, self, 0);
>
> This should avoid that the trace state is lost while waiting on that
> mutex and the checks for "is traced" may remain the same.
This was the first thing I tried but it did not work. ptrace_check_attach() passes TASK_TRACED to wait_task_inactive() which tests for equality:
unsigned long wait_task_inactive(struct task_struct *p, long match_state)
< ... >
if (match_state && unlikely(p->state != match_state)
&& unlikely(p->saved_state != match_state)) {
But this test will fail because we've added TASK_UNINTERRUPTIBLE and possibly removed some other bits from p->state. So I decided to add pi_lock locking instead - it is slower but it works with 100% guarantee.
Also adding __TASK_TRACED does not help with all the other places where saved_state is checked and more races might still be hidden.
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH rt] Fix races in ptrace
2013-08-22 14:23 ` Alexander Fyodorov
@ 2013-08-29 16:33 ` Sebastian Andrzej Siewior
2013-08-29 17:26 ` Alexander Fyodorov
0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-08-29 16:33 UTC (permalink / raw)
To: Alexander Fyodorov; +Cc: linux-rt-users@vger.kernel.org
* Alexander Fyodorov | 2013-08-22 18:23:18 [+0400]:
>This was the first thing I tried but it did not work. ptrace_check_attach() passes TASK_TRACED to wait_task_inactive() which tests for equality:
>
>unsigned long wait_task_inactive(struct task_struct *p, long match_state)
>< ... >
> if (match_state && unlikely(p->state != match_state)
> && unlikely(p->saved_state != match_state)) {
>
>But this test will fail because we've added TASK_UNINTERRUPTIBLE and possibly removed some other bits from p->state. So I decided to add pi_lock locking instead - it is slower but it works with 100% guarantee.
>
>Also adding __TASK_TRACED does not help with all the other places where saved_state is checked and more races might still be hidden.
I'm tempted to add this:
---
include/linux/sched.h | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/include/linux/sched.h b/include/linux/sched.h
index e0a05de..bd60b9d 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -175,7 +175,6 @@ extern char ___assert_task_state[1 - 2*!!(
TASK_UNINTERRUPTIBLE | __TASK_STOPPED | \
__TASK_TRACED)
-#define task_is_traced(task) ((task->state & __TASK_TRACED) != 0)
#define task_is_stopped(task) ((task->state & __TASK_STOPPED) != 0)
#define task_is_dead(task) ((task)->exit_state != 0)
#define task_is_stopped_or_traced(task) \
@@ -2532,6 +2531,24 @@ static inline int signal_pending_state(long state, struct task_struct *p)
return (state & TASK_INTERRUPTIBLE) || __fatal_signal_pending(p);
}
+static inline bool task_is_traced(struct task_struct *task)
+{
+ bool traced = false;
+
+ if (task->state & __TASK_TRACED)
+ return true;
+#ifdef CONFIG_PREEMPT_RT_FULL
+ /* in case the task is sleeping on tasklist_lock */
+ raw_spin_lock_irq(&task->pi_lock);
+ if (task->state & __TASK_TRACED)
+ traced = true;
+ else if (task->saved_state & __TASK_TRACED)
+ traced = true;
+ raw_spin_unlock_irq(&task->pi_lock);
+#endif
+ return traced;
+}
+
/*
* cond_resched() and cond_resched_lock(): latency reduction via
* explicit rescheduling in places that are safe. The return
to the next v3.10-rt release. I tested this with:
--- cat ptrace-test.c ---
#include <unistd.h>
#include <sys/types.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/ptrace.h>
#include <sys/types.h>
#include <sys/wait.h>
#include <sys/time.h>
#include <sys/resource.h>
#define NUM_CHILD 5
#define LOCK_CHILD 3
static pid_t pids[NUM_CHILD];
static void play(void)
{
pid_t pid;
int ret;
pid = getpid();
fprintf(stderr, "%s(%d) ready\n", __func__, pid);
ret = ptrace(PTRACE_TRACEME, 0, NULL, 0);
if (ret) {
fprintf(stderr, "ERR %d: %m\n", pid);
return;
}
do {
kill(pid, SIGUSR1);
} while (1);
}
static void kill_kids(void)
{
int i;
for (i = 0; i < NUM_CHILD; i++) {
if (pids[i])
kill(pids[i], SIGKILL);
}
}
static void get_prio_fork(void)
{
pid_t pid;
int i;
pid = fork();
if (pid < 0) {
fprintf(stderr, "fork(): %m\n");
exit(1);
}
if (pid)
return;
do {
for (i = 0; i < NUM_CHILD; i++) {
int ret;
ret = getpriority(PRIO_PROCESS, pids[i]);
if (ret < 0) {
printf("=> %m %d\n", pids[i]);
exit(1);
}
}
} while(1);
exit(0);
}
int main(void)
{
int i;
for (i = 0; i < NUM_CHILD; i++) {
pid_t pid;
pid = fork();
if (pid < 0) {
fprintf(stderr, "Fork error: %m\n");
kill_kids();
exit(1);
}
if (pid == 0) {
play();
exit(0);
}
pids[i] = pid;
}
for (i = 0; i < LOCK_CHILD; i++)
get_prio_fork();
do {
for (i = 0; i < NUM_CHILD; i++) {
int ret;
ret = waitpid(pids[i], NULL, 0);
if (ret < 0) {
fprintf(stderr, "%s(2) %d / %d %m\n", __func__,
pids[i], ret);
exit(1);
}
ret = ptrace(PTRACE_CONT, pids[i], NULL, 0);
if (ret) {
fprintf(stderr, "%s(3) %d / %d %m\n", __func__,
pids[i], ret);
ret = kill(pids[i], 0);
if (!ret)
fprintf(stderr, "process is available\n");
exit(1);
}
}
} while(1);
return 0;
}
--- cat end ---
and it seems to work. Any comments?
Sebastian
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH rt] Fix races in ptrace
2013-08-29 16:33 ` Sebastian Andrzej Siewior
@ 2013-08-29 17:26 ` Alexander Fyodorov
2013-08-29 18:28 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 11+ messages in thread
From: Alexander Fyodorov @ 2013-08-29 17:26 UTC (permalink / raw)
To: Sebastian Andrzej Siewior; +Cc: linux-rt-users@vger.kernel.org
> +static inline bool task_is_traced(struct task_struct *task)
> +{
> + bool traced = false;
> +
> + if (task->state & __TASK_TRACED)
> + return true;
> +#ifdef CONFIG_PREEMPT_RT_FULL
> + /* in case the task is sleeping on tasklist_lock */
> + raw_spin_lock_irq(&task->pi_lock);
> + if (task->state & __TASK_TRACED)
> + traced = true;
> + else if (task->saved_state & __TASK_TRACED)
> + traced = true;
> + raw_spin_unlock_irq(&task->pi_lock);
> +#endif
> + return traced;
> +}
Since this is a low-level function, maybe its better to use raw_spin_lock_irqsave()? In case someone in the future will call task_is_traced() with disabled interrupts. Otherwise looks good.
Still this is only half of the solution because the patch doesn't solve the race in wait_task_inactive() (and all other places which test both state and saved_state without holding pi_lock).
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH rt] Fix races in ptrace
2013-08-29 17:26 ` Alexander Fyodorov
@ 2013-08-29 18:28 ` Sebastian Andrzej Siewior
2013-08-29 18:47 ` Alexander Fyodorov
0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-08-29 18:28 UTC (permalink / raw)
To: Alexander Fyodorov; +Cc: linux-rt-users@vger.kernel.org
On 08/29/2013 07:26 PM, Alexander Fyodorov wrote:
>> +static inline bool task_is_traced(struct task_struct *task)
>> +{
>> + bool traced = false;
>> +
>> + if (task->state & __TASK_TRACED)
>> + return true;
>> +#ifdef CONFIG_PREEMPT_RT_FULL
>> + /* in case the task is sleeping on tasklist_lock */
>> + raw_spin_lock_irq(&task->pi_lock);
>> + if (task->state & __TASK_TRACED)
>> + traced = true;
>> + else if (task->saved_state & __TASK_TRACED)
>> + traced = true;
>> + raw_spin_unlock_irq(&task->pi_lock);
>> +#endif
>> + return traced;
>> +}
>
> Since this is a low-level function, maybe its better to use raw_spin_lock_irqsave()? In case someone in the future will call task_is_traced() with disabled interrupts. Otherwise looks good.
The other function around don't do this and excpect it process context.
Thanks so far.
>
> Still this is only half of the solution because the patch doesn't solve the race in wait_task_inactive() (and all other places which test both state and saved_state without holding pi_lock).
So you are concerned that missing pi_lock in wait_task_inactive(). This
is a problem if the task wakes up from sleeping on the lock while its
state is beeing checked. Hmm it indeed looks legal.
I keep that patch in queue but disabled and take another look once I
get back.
Does this missing pi_lock() affects you or is just a precaution?
>
Sebastian
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH rt] Fix races in ptrace
2013-08-29 18:28 ` Sebastian Andrzej Siewior
@ 2013-08-29 18:47 ` Alexander Fyodorov
2013-08-29 18:49 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 11+ messages in thread
From: Alexander Fyodorov @ 2013-08-29 18:47 UTC (permalink / raw)
To: Sebastian Andrzej Siewior; +Cc: linux-rt-users@vger.kernel.org
29.08.2013, 22:28, "Sebastian Andrzej Siewior" <bigeasy@linutronix.de>:
> On 08/29/2013 07:26 PM, Alexander Fyodorov wrote:
>> Still this is only half of the solution because the patch doesn't solve the race in wait_task_inactive() (and all other places which test both state and saved_state without holding pi_lock).
>
> So you are concerned that missing pi_lock in wait_task_inactive(). This
> is a problem if the task wakes up from sleeping on the lock while its
> state is beeing checked. Hmm it indeed looks legal.
> I keep that patch in queue but disabled and take another look once I
> get back.
> Does this missing pi_lock() affects you or is just a precaution?
Yes, my test application failed because of it, a detailed description is in the first message in the thread. I've also looked at other places that test saved_state but I don't understand the code enough to decide whether there are similar bugs there or not.
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH rt] Fix races in ptrace
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
0 siblings, 1 reply; 11+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-08-29 18:49 UTC (permalink / raw)
To: Alexander Fyodorov; +Cc: linux-rt-users@vger.kernel.org
On 08/29/2013 08:47 PM, Alexander Fyodorov wrote:
>
> Yes, my test application failed because of it, a detailed description is in the first message in the thread. I've also looked at other places that test saved_state but I don't understand the code enough to decide whether there are similar bugs there or not.
Yes, you did I just wanted to make sure since it looks like a very
small race window. Will take a look on the remaining part once I get
back…
Sebastian
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] ptrace: fix ptrace vs tasklist_lock race
2013-08-29 18:49 ` Sebastian Andrzej Siewior
@ 2013-11-30 20:07 ` Sebastian Andrzej Siewior
0 siblings, 0 replies; 11+ messages in thread
From: Sebastian Andrzej Siewior @ 2013-11-30 20:07 UTC (permalink / raw)
To: Alexander Fyodorov, Steven Rostedt
Cc: linux-rt-users@vger.kernel.org, Thomas Gleixner
As explained by Alexander Fyodorov <halcy@yandex.ru>:
|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()
The patch is based on his initial patch where an additional check is
added in case the __TASK_TRACED moved to ->saved_state. The pi_lock is
taken in case the caller is interrupted between looking into ->state and
->saved_state.
Additionally I also extended the check in task_is_stopped_or_traced()
and consider the two possible states in ptrace_freeze_traced().
In wait_task_inactive() the state is now checked under the pi_lock as
well. I merged Steven's patch in this one
(rfc-sched-rt-fix-wait_task_interactive-to-test-rt_spin_lock-state.patch)
which already checked both places but without the lock.
This seems to pass my tiny testcase…
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
include/linux/sched.h | 48 +++++++++++++++++++++++++++++++++++++++++++++---
kernel/ptrace.c | 7 ++++++-
kernel/sched/core.c | 24 +++++++++++++++++++++---
3 files changed, 72 insertions(+), 7 deletions(-)
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -165,11 +165,8 @@ extern char ___assert_task_state[1 - 2*!
TASK_UNINTERRUPTIBLE | __TASK_STOPPED | \
__TASK_TRACED)
-#define task_is_traced(task) ((task->state & __TASK_TRACED) != 0)
#define task_is_stopped(task) ((task->state & __TASK_STOPPED) != 0)
#define task_is_dead(task) ((task)->exit_state != 0)
-#define task_is_stopped_or_traced(task) \
- ((task->state & (__TASK_STOPPED | __TASK_TRACED)) != 0)
#define task_contributes_to_load(task) \
((task->state & TASK_UNINTERRUPTIBLE) != 0 && \
(task->flags & PF_FROZEN) == 0)
@@ -2410,6 +2407,51 @@ static inline int need_resched(void)
return unlikely(test_thread_flag(TIF_NEED_RESCHED));
}
+static inline bool __task_is_stopped_or_traced(struct task_struct *task)
+{
+ if (task->state & (__TASK_STOPPED | __TASK_TRACED))
+ return true;
+#ifdef CONFIG_PREEMPT_RT_FULL
+ if (task->saved_state & (__TASK_STOPPED | __TASK_TRACED))
+ return true;
+#endif
+ return false;
+}
+
+static inline bool task_is_stopped_or_traced(struct task_struct *task)
+{
+ bool traced_stopped;
+
+#ifdef CONFIG_PREEMPT_RT_FULL
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&task->pi_lock, flags);
+ traced_stopped = __task_is_stopped_or_traced(task);
+ raw_spin_unlock_irqrestore(&task->pi_lock, flags);
+#else
+ traced_stopped = __task_is_stopped_or_traced(task);
+#endif
+ return traced_stopped;
+}
+
+static inline bool task_is_traced(struct task_struct *task)
+{
+ bool traced = false;
+
+ if (task->state & __TASK_TRACED)
+ return true;
+#ifdef CONFIG_PREEMPT_RT_FULL
+ /* in case the task is sleeping on tasklist_lock */
+ raw_spin_lock_irq(&task->pi_lock);
+ if (task->state & __TASK_TRACED)
+ traced = true;
+ else if (task->saved_state & __TASK_TRACED)
+ traced = true;
+ raw_spin_unlock_irq(&task->pi_lock);
+#endif
+ return traced;
+}
+
/*
* cond_resched() and cond_resched_lock(): latency reduction via
* explicit rescheduling in places that are safe. The return
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -135,7 +135,12 @@ static bool ptrace_freeze_traced(struct
spin_lock_irq(&task->sighand->siglock);
if (task_is_traced(task) && !__fatal_signal_pending(task)) {
- task->state = __TASK_TRACED;
+ raw_spin_lock_irq(&task->pi_lock);
+ if (task->state & __TASK_TRACED)
+ task->state = __TASK_TRACED;
+ else
+ task->saved_state = __TASK_TRACED;
+ raw_spin_unlock_irq(&task->pi_lock);
ret = true;
}
spin_unlock_irq(&task->sighand->siglock);
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -1024,6 +1024,20 @@ struct migration_arg {
static int migration_cpu_stop(void *data);
+static bool check_task_state(struct task_struct *p, long match_state)
+{
+ bool match = false;
+
+ raw_spin_lock_irq(&p->pi_lock);
+ if (p->state == match_state)
+ match = true;
+ else if (p->saved_state == match_state)
+ match = true;
+ raw_spin_unlock_irq(&p->pi_lock);
+
+ return match;
+}
+
/*
* wait_task_inactive - wait for a thread to unschedule.
*
@@ -1068,8 +1082,11 @@ unsigned long wait_task_inactive(struct
* is actually now running somewhere else!
*/
while (task_running(rq, p)) {
- if (match_state && unlikely(p->state != match_state))
+ if (match_state) {
+ if (!unlikely(check_task_state(p, match_state)))
+ return 0;
return 0;
+ }
cpu_relax();
}
@@ -1083,7 +1100,8 @@ unsigned long wait_task_inactive(struct
running = task_running(rq, p);
on_rq = p->on_rq;
ncsw = 0;
- if (!match_state || p->state == match_state)
+ if (!match_state || p->state == match_state
+ || p->saved_state == match_state)
ncsw = p->nvcsw | LONG_MIN; /* sets MSB */
task_rq_unlock(rq, p, &flags);
@@ -1579,7 +1597,7 @@ static void try_to_wake_up_local(struct
*/
int wake_up_process(struct task_struct *p)
{
- WARN_ON(task_is_stopped_or_traced(p));
+ WARN_ON(__task_is_stopped_or_traced(p));
return try_to_wake_up(p, TASK_NORMAL, 0);
}
EXPORT_SYMBOL(wake_up_process);
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [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).