* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.