linux-kernel - Re: [patch] oom: give current access to memory reserves if it has been killed

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: <20100409123823.GA6661@redhat.com>
Date:	Fri, 9 Apr 2010 14:38:23 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	David Rientjes <rientjes@...gle.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	anfei <anfei.zhou@...il.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	nishimura@....nes.nec.co.jp,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	Mel Gorman <mel@....ul.ie>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [patch] oom: give current access to memory reserves if it has
	been killed

On 04/08, David Rientjes wrote:
>
> On Thu, 1 Apr 2010, Oleg Nesterov wrote:
>
> > Look. We have a main thread M and the sub-thread T. T forks a lot of
> > processes which use a lot of memory. These processes _are_ the first
> > descendant children of the M+T thread group, they should be accounted.
> > But M->children list is empty.
> >
> > oom_forkbomb_penalty() and oom_kill_process() should do
> >
> > 	t = tsk;
> > 	do {
> > 		list_for_each_entry(child, &t->children, sibling) {
> > 			... take child into account ...
> > 		}
> > 	} while_each_thread(tsk, t);
> >
>
> In this case, it seems more appropriate that we would penalize T and not M

We can't. Any fatal signal sent to any sub-thread kills the whole thread
group. It is not possible to kill T but not M.

> since it's not necessarily responsible for the behavior of the children it
> forks. T is the buggy/malicious program, not M.

Since a) they share the same ->mm and b) they share their children, I
don't think we should separate T and M.

->children is per_thread. But this is only because we have some strange
historiral oddities like __WNOTHREAD. Otherwise, it is not correct to
assume that the child of T is not the child of M. Any process is the
child of its parent's thread group, not the thread which actually called
fork().

> > --- x/mm/oom_kill.c
> > +++ x/mm/oom_kill.c
> > @@ -97,13 +97,16 @@ static unsigned long oom_forkbomb_penalt
> >  		return 0;
> >  	list_for_each_entry(child, &tsk->children, sibling) {
> >  		struct task_cputime task_time;
> > -		unsigned long runtime;
> > +		unsigned long runtime, this_rss;
> >
> >  		task_lock(child);
> >  		if (!child->mm || child->mm == tsk->mm) {
> >  			task_unlock(child);
> >  			continue;
> >  		}
> > +		this_rss = get_mm_rss(child->mm);
> > +		task_unlock(child);
> > +
> >  	/*
>
> This patch looks good, will you send it to Andrew with a changelog and 
> sign-off line?  Also feel free to add:
>
> Acked-by: David Rientjes <rientjes@...gle.com>

Thanks! already in -mm.

Oleg.

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