[PATCH] kexec, kernel 2.6.26.5, segmentation fault in kimage_add_entry

[PATCH] kexec, kernel 2.6.26.5, segmentation fault in kimage_add_entry

[Jonathan Steel]

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

[PATCH] kexec: segmentation fault in kimage_add_entry

[Eric W. Biederman]

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

Re: [PATCH] kexec, kernel 2.6.26.5, segmentation fault in kimage_add_entry

[Simon Horman]

On Tue, Sep 16, 2008 at 03:18:23PM -0400, Jonathan Steel wrote:
> 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 wonder if this problem might also affect other users of
kimage_alloc_pages(), and if so, perhaps it should guard
against this?

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

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

Re: [PATCH] kexec, kernel 2.6.26.5, segmentation fault in kimage_add_entry

[Eric W. Biederman]

Simon Horman <horms@verge.net.au> writes:

> On Tue, Sep 16, 2008 at 03:18:23PM -0400, Jonathan Steel wrote:
>> 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 wonder if this problem might also affect other users of
> kimage_alloc_pages(), and if so, perhaps it should guard
> against this?

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

Re: [PATCH] kexec, kernel 2.6.26.5, segmentation fault in kimage_add_entry

[Jonathan Steel]

Simon Horman wrote:
> On Tue, Sep 16, 2008 at 03:18:23PM -0400, Jonathan Steel wrote:
>   
>> 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 wonder if this problem might also affect other users of
> kimage_alloc_pages(), and if so, perhaps it should guard
> against this?
>   
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 
doing so could cause more problems in the future.
>   
>> --- 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
>>     
>
>   

Re: [PATCH] kexec: segmentation fault in kimage_add_entry

[Simon Horman]

On Tue, Sep 16, 2008 at 03:21:53PM -0700, Eric W. Biederman wrote:
> 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>

For what it is worth: Acked-by: Simon Horman <horms@verge.net.au>

> ---
>  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
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec

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