linux-kernel - Re: [PATCH 07/10] mm, oom: fortify task_will_free

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: <20160616142940.GK6836@dhcp22.suse.cz>
Date:	Thu, 16 Jun 2016 16:29:40 +0200
From:	Michal Hocko <mhocko@...nel.org>
To:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Cc:	linux-mm@...ck.org, rientjes@...gle.com, oleg@...hat.com,
	vdavydov@...allels.com, akpm@...ux-foundation.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 07/10] mm, oom: fortify task_will_free_mem

On Thu 16-06-16 21:54:27, Tetsuo Handa wrote:
> Michal Hocko wrote:
> > On Sat 11-06-16 17:10:03, Tetsuo Handa wrote:
[...]
> I still don't like it. current->mm == NULL in
> 
> -	if (current->mm &&
> -	    (fatal_signal_pending(current) || task_will_free_mem(current))) {
> +	if (task_will_free_mem(current)) {
> 
> is not highly unlikely. You obviously break commit d7a94e7e11badf84
> ("oom: don't count on mm-less current process") on CONFIG_MMU=n kernels.

I still fail to see why you care about that case so much. The heuristic
was broken for other reasons before this patch. The patch fixes a class
of issues for both mmu and nommu. I can restore the current->mm check
for now but the more I am thinking about it the less I am sure the
commit you are referring to is evem correct/necessary.

It claims that the OOM killer would be stuck because the child would be
sitting in the final schedule() until the parent reaps it. That is not
true, though, because victim would be unhashed down in release_task()
path so it is not visible by the oom killer when it is waiting for the
parent.  I have completely missed that part when reviewing the patch. Or
am I missing something...

Anyway, would you be OK with the patch if I added the current->mm check
and resolve its necessity in a separate patch?
-- 
Michal Hocko
SUSE Labs