linux-kernel - Re: [PATCH 1/2] oom: do not live lock on frozen tasks

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: <20110926091440.GE10156@tiehlicka.suse.cz>
Date:	Mon, 26 Sep 2011 11:14:40 +0200
From:	Michal Hocko <mhocko@...e.cz>
To:	David Rientjes <rientjes@...gle.com>
Cc:	Oleg Nesterov <oleg@...hat.com>,
	Konstantin Khlebnikov <khlebnikov@...nvz.org>,
	linux-mm@...ck.org, Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>, Tejun Heo <tj@...nel.org>,
	Rusty Russell <rusty@...tcorp.com.au>
Subject: Re: [PATCH 1/2] oom: do not live lock on frozen tasks

On Mon 26-09-11 01:56:57, David Rientjes wrote:
> On Mon, 26 Sep 2011, Michal Hocko wrote:
> 
> > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > index 626303b..b9774f3 100644
> > --- a/mm/oom_kill.c
> > +++ b/mm/oom_kill.c
> > @@ -32,6 +32,7 @@
> >  #include <linux/mempolicy.h>
> >  #include <linux/security.h>
> >  #include <linux/ptrace.h>
> > +#include <linux/freezer.h>
> >  
> >  int sysctl_panic_on_oom;
> >  int sysctl_oom_kill_allocating_task;
> > @@ -451,6 +452,9 @@ static int oom_kill_task(struct task_struct *p, struct mem_cgroup *mem)
> >  				task_pid_nr(q), q->comm);
> >  			task_unlock(q);
> >  			force_sig(SIGKILL, q);
> > +
> > +			if (frozen(q))
> > +				thaw_process(q);
> >  		}
> >  
> >  	set_tsk_thread_flag(p, TIF_MEMDIE);
> 
> This is in the wrong place, oom_kill_task() iterates over all threads that 
> are _not_ in the same thread group as the chosen thread and kills them 
> without giving them access to memory reserves.  The chosen task, p, could 
> still be frozen and may not exit.

Ahh, right you are. I ave missed that one. Updated patch bellow.

> 
> Once that's fixed, feel free to add my
> 
> 	Acked-by: David Rientjes <rientjes@...gle.com>

Thanks

> 
> once Rafael sends his acked-by or reviewed-by.
---
>From f935ed4558c2fb033ef5c14e02b28e12a615f80e Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@...e.cz>
Date: Fri, 16 Sep 2011 11:23:15 +0200
Subject: [PATCH] oom: do not live lock on frozen tasks

Konstantin Khlebnikov has reported (https://lkml.org/lkml/2011/8/23/45)
that OOM can end up in a live lock if select_bad_process picks up a frozen
task.
Unfortunately we cannot mark such processes as unkillable to ignore them
because we could panic the system even though there is a chance that
somebody could thaw the process so we can make a forward process (e.g. a
process from another cpuset or with a different nodemask).

Let's thaw an OOM selected frozen process right after we've sent fatal
signal from oom_kill_task.
Thawing is safe if the frozen task doesn't access any suspended device
(e.g. by ioctl) on the way out to the userspace where we handle the
signal and die. Note, we are not interested in the kernel threads because
they are not oom killable.

Accessing suspended devices by a userspace processes shouldn't be an
issue because devices are suspended only after userspace is already
frozen and oom is disabled at that time.

run_guest (drivers/lguest/core.c) calls try_to_freeze with an user
context but it seems it is able to cope with signals because it
explicitly checks for pending signals so we should be safe.

Other than that userspace accesses the fridge only from the
signal handling routines so we are able to handle SIGKILL without any
negative side effects.

Signed-off-by: Michal Hocko <mhocko@...e.cz>
Reported-by: Konstantin Khlebnikov <khlebnikov@...nvz.org>
---
 mm/oom_kill.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 626303b..c419a7e 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -32,6 +32,7 @@
 #include <linux/mempolicy.h>
 #include <linux/security.h>
 #include <linux/ptrace.h>
+#include <linux/freezer.h>
 
 int sysctl_panic_on_oom;
 int sysctl_oom_kill_allocating_task;
@@ -451,10 +452,15 @@ static int oom_kill_task(struct task_struct *p, struct mem_cgroup *mem)
 				task_pid_nr(q), q->comm);
 			task_unlock(q);
 			force_sig(SIGKILL, q);
+
+			if (frozen(q))
+				thaw_process(q);
 		}
 
 	set_tsk_thread_flag(p, TIF_MEMDIE);
 	force_sig(SIGKILL, p);
+	if (frozen(p))
+		thaw_process(p);
 
 	return 0;
 }
-- 
1.7.5.4

-- 
Michal Hocko
SUSE Labs
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic
--
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/