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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 20 Sep 2014 13:28:19 -0400
From:	Tejun Heo <tj@...nel.org>
To:	Zefan Li <lizefan@...wei.com>
Cc:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
	peterz@...radead.org, mingo@...hat.com, akpm@...ux-foundation.org,
	cgroups@...r.kernel.org, linux-kernel@...r.kernel.org,
	fernando_b1@....ntt.co.jp
Subject: Re: Racy manipulation of task_struct->flags in cgroups code causes
 hard to reproduce kernel panics

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/