Messages in this thread | | | Date | Sat, 20 Sep 2014 00:21:03 +0200 | From | Peter Zijlstra <> | Subject | Re: Racy manipulation of task_struct->flags in cgroups code causes hard to reproduce kernel panics |
| |
On Fri, Sep 19, 2014 at 08:53:33PM +0900, Tetsuo Handa wrote: > cpuset_update_task_spread_flag() is updating other thread's > "struct task_struct"->flags without exclusion control or atomic > operations! > > ---------- linux-2.6.32-358.23.2.el6/kernel/cpuset.c ---------- > 300:/* > 301: * update task's spread flag if cpuset's page/slab spread flag is set > 302: * > 303: * Called with callback_mutex/cgroup_mutex held > 304: */ > 305:static void cpuset_update_task_spread_flag(struct cpuset *cs, > 306: struct task_struct *tsk) > 307:{ > 308: if (is_spread_page(cs)) > 309: tsk->flags |= PF_SPREAD_PAGE; > 310: else > 311: tsk->flags &= ~PF_SPREAD_PAGE; > 312: if (is_spread_slab(cs)) > 313: tsk->flags |= PF_SPREAD_SLAB; > 314: else > 315: tsk->flags &= ~PF_SPREAD_SLAB; > 316:}
So that is indeed clearly buggy and should not be done. One should only every change current->flags.
Most sites do indeed do that, with exceptions:
fork.c: some assignments are done before the new task is visible - safe __kthread_bind: requires that the task is sleeping - safe workqueue/worker: same as __kthread_bind, the task is asleep - safe workqueue/rescue: is current - safe
Which does indeed leave this cpuset exception which is clearly and obviously broken.
The 'simple' solution would be to force 'suspend/freeze' the task while poking at its ->flags.
| |