Re: [PATCH] kexec: segmentation fault in kimage_add_entry

Previous thread: Warning Code:USV64MT1 by Upgrade Team on Tuesday, September 16, 2008 - 11:20 am. (1 message)

Next thread: [PATCH] cdrom: update ioctl documentation by Németh Márton on Tuesday, September 16, 2008 - 12:51 pm. (1 message)
From: Jonathan Steel
Date: Tuesday, September 16, 2008 - 12:18 pm

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: Eric W. Biederman
Date: Tuesday, September 16, 2008 - 3:21 pm

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

--

From: Simon Horman
Date: Wednesday, September 17, 2008 - 9:30 pm

-- 
Simon Horman
  VA Linux Systems Japan K.K., Sydney, Australia Satellite Office
  H: www.vergenet.net/~horms/             W: www.valinux.co.jp/en

--

From: Simon Horman
Date: Tuesday, September 16, 2008 - 5:55 pm

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

--

From: Eric W. Biederman
Date: Tuesday, September 16, 2008 - 7:40 pm

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

From: Jonathan Steel
Date: Wednesday, September 17, 2008 - 6:18 am

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 

--

Previous thread: Warning Code:USV64MT1 by Upgrade Team on Tuesday, September 16, 2008 - 11:20 am. (1 message)

Next thread: [PATCH] cdrom: update ioctl documentation by Németh Márton on Tuesday, September 16, 2008 - 12:51 pm. (1 message)