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 linux-cve-announce 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]

Message-Id: <201409211415.GJG26578.MFQOHtSFVJLOOF@I-love.SAKURA.ne.jp>
Date:	Sun, 21 Sep 2014 14:15:11 +0900
From:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To:	tj@...nel.org, lizefan@...wei.com, miaox@...fujitsu.com
Cc:	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

Tejun Heo wrote:
> 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?
> 

This race condition exists since commit 950592f7b991 ("cpusets: update
tasks' page/slab spread flags in time") (i.e. Linux 2.6.31 and later)
but "struct task_struct"->atomic_flags was added in Linux 3.17.

If use of ->atomic_flags for cpuset is acceptable, how should we fix
older kernels? Backport ->atomic_flags?

> 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/