linux-kernel - Re: [patch] mm: memcontrol: do not iterate uninitialized memcgs

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: <20140925025758.GA6903@mtj.dyndns.org>
Date:	Wed, 24 Sep 2014 22:57:58 -0400
From:	Tejun Heo <tj@...nel.org>
To:	Johannes Weiner <hannes@...xchg.org>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Hugh Dickins <hughd@...gle.com>, Michal Hocko <mhocko@...e.cz>,
	linux-mm@...ck.org, cgroups@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [patch] mm: memcontrol: do not iterate uninitialized memcgs

Hello,

On Wed, Sep 24, 2014 at 10:31:18PM -0400, Johannes Weiner wrote:
..
> not meet the ordering requirements for memcg, and so we still may see
> partially initialized memcgs from the iterators.

It's mainly the other way around - a fully initialized css may not
show up in an iteration, but given that there's no memory ordering or
synchronization around the flag, anything can happen.

...
> +		if (next_css == &root->css ||
> +		    css_tryget_online(next_css)) {
> +			struct mem_cgroup *memcg;
> +
> +			memcg = mem_cgroup_from_css(next_css);
> +			if (memcg->initialized) {
> +				/*
> +				 * Make sure the caller's accesses to
> +				 * the memcg members are issued after
> +				 * we see this flag set.

I usually prefer if the comment points to the exact location that the
matching memory barriers live.  Sometimes it's difficult to locate the
partner barrier even w/ the functional explanation.

> +				 */
> +				smp_rmb();
> +				return memcg;

In an unlikely event this rmb becomes an issue, a self-pointing
pointer which is set/read using smp_store_release() and
smp_load_acquire() respectively can do with plain barrier() on the
reader side on archs which don't need data dependency barrier
(basically everything except alpha).  Not sure whether that'd be more
or less readable than this tho.

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/