linux-kernel - Re: [patch for-3.12] mm, memcg: protect mem_cgroup_read

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] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.02.1310011958440.31300@chino.kir.corp.google.com>
Date:	Tue, 1 Oct 2013 20:08:43 -0700 (PDT)
From:	David Rientjes <rientjes@...gle.com>
To:	Johannes Weiner <hannes@...xchg.org>
cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Michal Hocko <mhocko@...e.cz>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	cgroups@...r.kernel.org, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [patch for-3.12] mm, memcg: protect mem_cgroup_read_events for
 cpu hotplug

On Tue, 1 Oct 2013, Johannes Weiner wrote:

> On Tue, Oct 01, 2013 at 04:31:23PM -0700, David Rientjes wrote:
> > for_each_online_cpu() needs the protection of {get,put}_online_cpus() so
> > cpu_online_mask doesn't change during the iteration.
> 
> There is no problem report here.
> 
> Is there a crash?
> 

No.

> If it's just accuracy of the read, why would we care about some
> inaccuracies in counters that can change before you even get the
> results to userspace?  And care to the point where we hold up CPU
> hotplugging for this?
> 

cpu_hotplug.lock is held while a cpu is going down, it's a coarse lock 
that is used kernel-wide to synchronize cpu hotplug activity.  Memcg has 
a cpu hotplug notifier, called while there may not be any cpu hotplug 
refcounts, which drains per-cpu event counts to memcg->nocpu_base.events 
to maintain a cumulative event count as cpus disappear.  Without 
get_online_cpus() in mem_cgroup_read_events(), it's possible to account 
for the event count on a dying cpu twice, and this value may be 
significantly large.

In fact, all memcg->pcp_counter_lock use should be nested by 
{get,put}_online_cpus().

This fixes that issue and ensures the reported statistics are not vastly 
over-reported during cpu hotplug.

> Also, the fact that you directly sent this to Linus suggests there is
> some urgency for this fix.  What's going on?
> 

I believe users of cpu hotplug still want event counts that are 
approximate to the real value and that this is 3.12 material.
--
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/