linux-kernel - Re: [PATCH] oom: handle overflow in mem_cgroup_out_of

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]

Date:	Thu, 27 Jan 2011 09:24:34 +0900
From:	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Greg Thelen <gthelen@...gle.com>,
	Johannes Weiner <hannes@...xchg.org>,
	David Rientjes <rientjes@...gle.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	Minchan Kim <minchan.kim@...il.com>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] oom: handle overflow in mem_cgroup_out_of_memory()

On Wed, 26 Jan 2011 14:29:09 -0800
Andrew Morton <akpm@...ux-foundation.org> wrote:

> On Wed, 26 Jan 2011 12:32:04 -0800
> Greg Thelen <gthelen@...gle.com> wrote:
> 
> > > That being said, does this have any practical impact at all?  I mean,
> > > this code runs when the cgroup limit is breached.  But if the number
> > > of allowed pages (not bytes!) can not fit into 32 bits, it means you
> > > have a group of processes using more than 16T.  On a 32-bit machine.
> > 
> > The value of this patch is up for debate.  I do not have an example
> > situation where this truncation causes the wrong thing to happen.  I
> > suppose it might be possible for a racing update to
> > memory.limit_in_bytes which grows the limit from a reasonable (example:
> > 100M) limit to a large limit (example 1<<45) could benefit from this
> > patch.  I admit that this case seems pathological and may not be likely
> > or even worth bothering over.  If neither the memcg nor the oom
> > maintainers want the patch, then feel free to drop it.  I just noticed
> > the issue and thought it might be worth addressing.
> 
> Ah.  I was scratching my head over that.
> 
> In zillions of places the kernel assumes that a 32-bit kernel has less
> than 2^32 pages of memory, so the code as it stands is, umm, idiomatic.
> 

I think we can assume that. 

> But afaict the only way the patch makes a real-world difference is if
> res_counter_read_u64() is busted?
> 
> And, as you point out, res_counter_read_u64() is indeed busted on
> 32-bit machines.  It has 25 callsites in mm/memcontrol.c - has anyone
> looked at the implications of this?  What happens in all those
> callsites if the counter is read during a count rollover?
> 

I'll review. Against the roll-over, I think we just need to take lock.
So, res_counter_read_u64() implementation was wrong. It should take lock.
Please give me time.


THanks,
-Kame




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