| 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
| ||
|
Message-ID: <20090503204542.GJ4615@lenovo>
Date: Mon, 4 May 2009 00:45:42 +0400
From: Cyrill Gorcunov <gorcunov@...il.com>
To: Pekka Enberg <penberg@...helsinki.fi>
Cc: David Rientjes <rientjes@...gle.com>, Ingo Molnar <mingo@...e.hu>,
Jack Steiner <steiner@....com>,
Andrew Morton <akpm@...ux-foundation.org>,
"H. Peter Anvin" <hpa@...or.com>,
Thomas Gleixner <tglx@...utronix.de>,
LKML <linux-kernel@...r.kernel.org>,
Christoph Lameter <cl@...ux-foundation.org>
Subject: Re: introducing __GFP_PANIC
[Pekka Enberg - Sun, May 03, 2009 at 08:38:01PM +0300]
| On Sun, May 3, 2009 at 8:23 PM, Cyrill Gorcunov <gorcunov@...il.com> wrote:
| > | > Hi Pekka,
| > | >
| > | > ufortunatelly __alloc_pages_internal is not the only place where
| > | > we do return NULL from kmalloc. As example - failslab facility
| > | > (in slab_alloc call). Anyway -- I'll take a closer look.
| > |
| > | Right. I think failslab needs some fixing _not_ to return NULL if
| > | __GFP_PANIC is set.
| > |
| >
| > Ok, as a first raw draft (_not_ covering all the cases) it could
| > be something like this. It touches only __alloc_pages_internal
| > and we have to bespread as well:
| >
| > 1) alloc_pages with order >= MAX_ORDER (gfp.h)
| > 2) the same for alloc_pages_node (both used by SLOB)
| > 3) all __kmalloc should be explored as well.
| > 4) ???
| >
| > Anyway -- take a look on __alloc_pages_internal part :)
| >
| > -- Cyrill
| >
| > ---
| > include/linux/gfp.h | 4 +++-
| > mm/page_alloc.c | 8 +++++---
| > 2 files changed, 8 insertions(+), 4 deletions(-)
| >
| > Index: linux-2.6.git/include/linux/gfp.h
| > =====================================================================
| > --- linux-2.6.git.orig/include/linux/gfp.h
| > +++ linux-2.6.git/include/linux/gfp.h
| > @@ -58,7 +58,9 @@ struct vm_area_struct;
| > #define __GFP_NOTRACK ((__force gfp_t)0)
| > #endif
| >
| > -#define __GFP_BITS_SHIFT 22 /* Room for 22 __GFP_FOO bits */
| > +#define __GFP_PANIC ((__force gfp_t)0x400000u) /* Panic on page alloction failure */
| > +
| > +#define __GFP_BITS_SHIFT 23 /* Room for 23 __GFP_FOO bits */
| > #define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) - 1))
| >
| > /* This equals 0, but use constants in case they ever change */
| > Index: linux-2.6.git/mm/page_alloc.c
| > =====================================================================
| > --- linux-2.6.git.orig/mm/page_alloc.c
| > +++ linux-2.6.git/mm/page_alloc.c
| > @@ -1496,7 +1496,7 @@ __alloc_pages_internal(gfp_t gfp_mask, u
| > might_sleep_if(wait);
| >
| > if (should_fail_alloc_page(gfp_mask, order))
| > - return NULL;
| > + goto nopage;
|
| The point of fault injection is to increase coverage out-of-memory
| error handling code. So this doesn't make much sense to me. Why would
| you want to cause a __GFP_PANIC call-sites to panic()? It doesn't help
| testing one bit.
|
| So I still think you should just fix up should_fail_alloc_page() _not_
| to return true if __GFP_PANIC is set.
|
Just before I get some sleep. Here is an another attempt.
Not covered cases I know of:
1) SLAB with builtin constant as size and we don't find appropriate
cache for it do return NULL
2) alloc_pages_node and alloc_pages do check for MAX_ORDER which
could be exceeded as well and return NULL then.
ok, here is a preliminary patch just to hear some complains :)
-- Cyrill
---
include/linux/gfp.h | 4 +++-
mm/failslab.c | 3 +++
mm/page_alloc.c | 8 ++++++--
3 files changed, 12 insertions(+), 3 deletions(-)
Index: linux-2.6.git/include/linux/gfp.h
=====================================================================
--- linux-2.6.git.orig/include/linux/gfp.h
+++ linux-2.6.git/include/linux/gfp.h
@@ -58,7 +58,9 @@ struct vm_area_struct;
#define __GFP_NOTRACK ((__force gfp_t)0)
#endif
-#define __GFP_BITS_SHIFT 22 /* Room for 22 __GFP_FOO bits */
+#define __GFP_PANIC ((__force gfp_t)0x400000u) /* Panic on page alloction failure */
+
+#define __GFP_BITS_SHIFT 23 /* Room for 23 __GFP_FOO bits */
#define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) - 1))
/* This equals 0, but use constants in case they ever change */
Index: linux-2.6.git/mm/failslab.c
=====================================================================
--- linux-2.6.git.orig/mm/failslab.c
+++ linux-2.6.git/mm/failslab.c
@@ -17,6 +17,9 @@ bool should_failslab(size_t size, gfp_t
if (gfpflags & __GFP_NOFAIL)
return false;
+ if (gfpflags & __GFP_PANIC)
+ return false;
+
if (failslab.ignore_gfp_wait && (gfpflags & __GFP_WAIT))
return false;
Index: linux-2.6.git/mm/page_alloc.c
=====================================================================
--- linux-2.6.git.orig/mm/page_alloc.c
+++ linux-2.6.git/mm/page_alloc.c
@@ -1185,6 +1185,8 @@ static int should_fail_alloc_page(gfp_t
return 0;
if (gfp_mask & __GFP_NOFAIL)
return 0;
+ if (gfp_mask & __GFP_PANIC)
+ return 0;
if (fail_page_alloc.ignore_gfp_highmem && (gfp_mask & __GFP_HIGHMEM))
return 0;
if (fail_page_alloc.ignore_gfp_wait && (gfp_mask & __GFP_WAIT))
@@ -1506,7 +1508,7 @@ restart:
* Happens if we have an empty zonelist as a result of
* GFP_THISNODE being used on a memoryless node
*/
- return NULL;
+ goto nopage;
}
page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, nodemask, order,
@@ -1685,7 +1687,9 @@ nopage:
dump_stack();
show_mem();
}
- return page;
+ if (unlikely(gfp_mask & __GFP_PANIC))
+ panic("Out of memory: panic due to __GFP_PANIC\n");
+ return NULL;
got_pg:
if (kmemcheck_enabled)
kmemcheck_pagealloc_alloc(page, order, gfp_mask);
--
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/
Powered by blists - more mailing lists