On Thu, May 19, 2016 at 10:42:39AM -0400, Chris Metcalf wrote: > >>>>+ rcu_read_lock(); > >>>>+ p = cpu_curr(cpu); Here @cpu can schedule, hit TASK_DEAD and do put_task_struct() and kfree() the task. > >>>>+ get_task_struct(p); And here we then do a use-after-free. > >>>>+ rcu_read_unlock(); > >>>>+ task_isolation_debug_task(cpu, p); > >>>>+ put_task_struct(p); > So, I think what you're saying is that there is a race between when we > read per_cpu(runqueues, cpu).curr, and when we increment the > p->usage value in the task, and that the RCU read lock doesn't help > with that? Yep, as per the above. > My impression was that by being the ".curr" task, we are > guaranteed that it hasn't gone through do_exit() yet, and thus we > benefit from an RCU guarantee around being able to validly dereference > the pointer, i.e. it hasn't yet been freed and so dereferencing is safe. Nope... the only way to avoid this from happening is taking @cpu's rq->lock to prevent the remote CPU from scheduling. > I don't see how grabbing the ->curr from the runqueue is any more > fragile from an RCU perspective than grabbing the task from the pid in > kill_pid_info(). The whole pid data structure is RCU managed, rq->curr is not. > Anyway, whatever more clarity you can offer me, or suggestions for > APIs to use are welcome. The API proposed in the discussion below.. > >See also the discussion around: > > > >lkml.kernel.org/r/20160518170218.GY3192@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx > > This makes me wonder if I should use rcu_dereference(&cpu_curr(p)) > just for clarity, though I think it's just as correct either way. Nope, that's just as broken. So the 'simple' thing is: struct rq *rq = cpu_rq(cpu); struct task_struct *task; raw_spin_lock_irq(&rq->lock); task = rq->curr; get_task_struct(task); raw_spin_unlock_irq(&rq->lock); Because by holding rq->lock, the remote CPU cannot schedule and the current task _must_ still be valid. And note; the above can result in a task which already has PF_EXITING set. The complex thing is described in the linked thread and will likely make your head hurt. -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
