linux-kernel - RE: [patch v3 3/3] mm, oom: do not panic for oom kills triggered from sysrq

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: <050601d0bae5$14647770$3d2d6650$@alibaba-inc.com>
Date:	Fri, 10 Jul 2015 15:50:23 +0800
From:	"Hillf Danton" <hillf.zj@...baba-inc.com>
To:	"'David Rientjes'" <rientjes@...gle.com>
Cc:	"'Andrew Morton'" <akpm@...ux-foundation.org>,
	"'Sergey Senozhatsky'" <sergey.senozhatsky.work@...il.com>,
	"'Michal Hocko'" <mhocko@...e.cz>,
	"'linux-kernel'" <linux-kernel@...r.kernel.org>,
	<linux-mm@...ck.org>
Subject: RE: [patch v3 3/3] mm, oom: do not panic for oom kills triggered from sysrq

> > > diff --git a/Documentation/sysrq.txt b/Documentation/sysrq.txt
> > > --- a/Documentation/sysrq.txt
> > > +++ b/Documentation/sysrq.txt
> > > @@ -75,7 +75,8 @@ On all -  write a character to /proc/sysrq-trigger.  e.g.:
> > >
> > >  'e'     - Send a SIGTERM to all processes, except for init.
> > >
> > > -'f'	- Will call oom_kill to kill a memory hog process.
> > > +'f'	- Will call the oom killer to kill a memory hog process, but do not
> > > +	  panic if nothing can be killed.
> > >
> > >  'g'	- Used by kgdb (kernel debugger)
> > >
> > > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > > --- a/mm/oom_kill.c
> > > +++ b/mm/oom_kill.c
> > > @@ -607,6 +607,9 @@ void check_panic_on_oom(struct oom_control *oc, enum oom_constraint constraint,
> > >  		if (constraint != CONSTRAINT_NONE)
> > >  			return;
> > >  	}
> > > +	/* Do not panic for oom kills triggered by sysrq */
> > > +	if (oc->order == -1)
> > > +		return;
> > >  	dump_header(oc, NULL, memcg);
> > >  	panic("Out of memory: %s panic_on_oom is enabled\n",
> > >  		sysctl_panic_on_oom == 2 ? "compulsory" : "system-wide");
> > > @@ -686,11 +689,11 @@ bool out_of_memory(struct oom_control *oc)
> > >
> > >  	p = select_bad_process(oc, &points, totalpages);
> > >  	/* Found nothing?!?! Either we hang forever, or we panic. */
> > > -	if (!p) {
> > > +	if (!p && oc->order != -1) {
> > >  		dump_header(oc, NULL, NULL);
> > >  		panic("Out of memory and no killable processes...\n");
> > >  	}
> >
> > Given sysctl_panic_on_oom checked, AFAICU there seems
> > no chance for panic, no matter -1 or not.
> >
> 
> I'm not sure I understand your point.
> 
> There are two oom killer panics: when panic_on_oom is enabled and when the
> oom killer can't find an eligible process.
> 
> The change to the panic_on_oom panic is dealt with in check_panic_on_oom()
> and the no eligible process panic is dealt with here.
> 
> If the sysctl is disabled, and there are no eligible processes to kill,
> the change in behavior here is that we don't panic when triggered from
> sysrq.  That's the change in the hunk above.
> 
When no eligible processes is selected to kill, we are sure that we skip one
panic in check_panic_on_oom(), and we have no clear reason to panic again.

But we can simply answer the caller that there is no page, and let her
decide what to do.

So I prefer to fold the two panic into one.

Hillf
> > > -	if (p != (void *)-1UL) {
> > > +	if (p && p != (void *)-1UL) {
> > >  		oom_kill_process(oc, p, points, totalpages, NULL,
> > >  				 "Out of memory");
> > >  		killed = 1;

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