Re: [PATCH rt] Fix races in ptrace

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

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