A segmentation fault can occur in kimage_add_entry in kexec.c when
loading a kernel image into memory. The fault occurs because a page is
requested by calling kimage_alloc_page with gfp_mask GFP_KERNEL and the
function may actually return a page with gfp_mask GFP_HIGHUSER. The high
mem page is returned because it was swapped with the kernel page due to
the kernel page being a page that will shortly be copied to.
This patch ensures that kimage_alloc_page returns a page that was
created with the correct gfp flags.
--- linux-2.6.26.5.orig/kernel/kexec.c 2008-09-16 13:17:56.000000000
-0400
+++ linux-2.6.26.5/kernel/kexec.c 2008-09-16 13:26:35.000000000 -0400
@@ -743,8 +743,15 @@ static struct page *kimage_alloc_page(st
*old = addr | (*old & ~PAGE_MASK);
/* The old page I have found cannot be a
- * destination page, so return it.
- */
+ * destination page, so return it if its
+ * gfp_flags honor the ones passed in.
+ */
+ if (!(gfp_mask & __GFP_HIGHMEM) &&
+ PageHighMem(old_page)) {
+ kimage_free_pages(old_page);
+ continue;
+ }
+
addr = old_addr;
page = old_page;
break;
Signed-off-by: Jonathan Steel <jon.steel@esentire.com>
Jonathan Steel
--
From: Jonathan Steel <jon.steel@esentire.com>
A segmentation fault can occur in kimage_add_entry in kexec.c when
loading a kernel image into memory. The fault occurs because a page is
requested by calling kimage_alloc_page with gfp_mask GFP_KERNEL and the
function may actually return a page with gfp_mask GFP_HIGHUSER. The high
mem page is returned because it was swapped with the kernel page due to
the kernel page being a page that will shortly be copied to.
This patch ensures that kimage_alloc_page returns a page that was
created with the correct gfp flags.
I have verified the change and fixed the whitespace damage of the
original patch. Jonathan did a great job of tracking this down
after he hit the problem. -- Eric
Signed-off-by: Jonathan Steel <jon.steel@esentire.com>
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
kernel/kexec.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/kernel/kexec.c b/kernel/kexec.c
index 59f3f0d..aef2653 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -753,8 +753,14 @@ static struct page *kimage_alloc_page(struct kimage *image,
*old = addr | (*old & ~PAGE_MASK);
/* The old page I have found cannot be a
- * destination page, so return it.
+ * destination page, so return it if it's
+ * gfp_flags honor the ones passed in.
*/
+ if (!(gfp_mask & __GFP_HIGHMEM) &&
+ PageHighMem(old_page)) {
+ kimage_free_pages(old_page);
+ continue;
+ }
addr = old_addr;
page = old_page;
break;
--
1.5.3.rc6.17.g1911
--
-- Simon Horman VA Linux Systems Japan K.K., Sydney, Australia Satellite Office H: www.vergenet.net/~horms/ W: www.valinux.co.jp/en --
I wonder if this problem might also affect other users of kimage_alloc_pages(), and if so, perhaps it should guard -- Simon Horman VA Linux Systems Japan K.K., Sydney, Australia Satellite Office H: www.vergenet.net/~horms/ W: www.valinux.co.jp/en --
No. kimage_alloc_pages() is a light wrapper around alloc_pages that simply marks the pages as reserved so they don't get used for other things while we have a hold of them. kimage_alloc_page() does a check to see if the page we have just allocated is the a page we are going to copy to and if so it does the copy of the data now, and frees the page that was holding the data. As an optimization it returns the page holding the data. The problem is that we allocate control pages as GFP_KERNEL and data pages GFP_HIGHUSER. And so they are not completely interchangeable. Since this check and swap only happens inside of kimage_alloc_page it only affects kimage_alloc_page. Eric n --
kimage_alloc_page is used on only two separate occasions. The first is when a page is allocated with the GFP_HIGHUSER to get a new source page for the kernel image. That case will not be affected by this change because the if will always evaluate to false. The second case is the case that this patch is intended to fix, when allocating a kernel page for the page list. If anybody else ever uses the function they just want it to return a new alloced page, so there can't be any harm in making sure they get back a page of the type they asked for. If anything, not --
