Re: Racy manipulation of task_struct->flags in cgroups code causes hard to reproduce kernel panics

[Tejun Heo]

Hello,

On Sat, Sep 20, 2014 at 01:55:54PM +0800, Zefan Li wrote:
> > Then, what made current->flags to unexpectedly preserve PF_USED_MATH flag?
> > The user is running cgrulesengd process in order to utilize cpuset cgroup.
> > Thus, cpuset_update_task_spread_flag() is called when cgrulesengd process
> > writes someone's pid to /cgroup/cpuset/$group/tasks interface.
> > 
> > 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:}
> 
> We should make the updating of this flag atomic.

Ugh, why do we even implement that in cpuset.  This should be handled
by MPOL_INTERLEAVE.  It feels like people have been using cpuset as
the dumpsite that people used w/o thinking much.  Going forward, let's
please confine cpuset to collective cpu and memory affinity
configuration.  It really shouldn't be implementing novel features for
scheduler or mm.

Anyways, yeah, the patch looks correct to me.  Can you please send a
version w/ proper description and sob?

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/