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