LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] kexec, kernel 2.6.26.5, segmentation fault in kimage_add_entry
@ 2008-09-16 19:18 Jonathan Steel
  2008-09-16 22:21 ` [PATCH] kexec: " Eric W. Biederman
  2008-09-17  0:55 ` [PATCH] kexec, kernel 2.6.26.5, " Simon Horman
  0 siblings, 2 replies; 6+ messages in thread
From: Jonathan Steel @ 2008-09-16 19:18 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: kexec, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] kexec: segmentation fault in kimage_add_entry
  2008-09-16 19:18 [PATCH] kexec, kernel 2.6.26.5, segmentation fault in kimage_add_entry Jonathan Steel
@ 2008-09-16 22:21 ` Eric W. Biederman
  2008-09-18  4:30   ` Simon Horman
  2008-09-17  0:55 ` [PATCH] kexec, kernel 2.6.26.5, " Simon Horman
  1 sibling, 1 reply; 6+ messages in thread
From: Eric W. Biederman @ 2008-09-16 22:21 UTC (permalink / raw)
  To: Andrew Morton, Linus Torvalds; +Cc: kexec, linux-kernel, 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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kexec, kernel 2.6.26.5, segmentation fault in kimage_add_entry
  2008-09-16 19:18 [PATCH] kexec, kernel 2.6.26.5, segmentation fault in kimage_add_entry Jonathan Steel
  2008-09-16 22:21 ` [PATCH] kexec: " Eric W. Biederman
@ 2008-09-17  0:55 ` Simon Horman
  2008-09-17  2:40   ` Eric W. Biederman
  2008-09-17 13:18   ` Jonathan Steel
  1 sibling, 2 replies; 6+ messages in thread
From: Simon Horman @ 2008-09-17  0:55 UTC (permalink / raw)
  To: Jonathan Steel; +Cc: Eric W. Biederman, kexec, linux-kernel

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kexec, kernel 2.6.26.5, segmentation fault in kimage_add_entry
  2008-09-17  0:55 ` [PATCH] kexec, kernel 2.6.26.5, " Simon Horman
@ 2008-09-17  2:40   ` Eric W. Biederman
  2008-09-17 13:18   ` Jonathan Steel
  1 sibling, 0 replies; 6+ messages in thread
From: Eric W. Biederman @ 2008-09-17  2:40 UTC (permalink / raw)
  To: Simon Horman; +Cc: Jonathan Steel, kexec, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kexec, kernel 2.6.26.5, segmentation fault in kimage_add_entry
  2008-09-17  0:55 ` [PATCH] kexec, kernel 2.6.26.5, " Simon Horman
  2008-09-17  2:40   ` Eric W. Biederman
@ 2008-09-17 13:18   ` Jonathan Steel
  1 sibling, 0 replies; 6+ messages in thread
From: Jonathan Steel @ 2008-09-17 13:18 UTC (permalink / raw)
  To: Simon Horman; +Cc: Eric W. Biederman, kexec, linux-kernel

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kexec: segmentation fault in kimage_add_entry
  2008-09-16 22:21 ` [PATCH] kexec: " Eric W. Biederman
@ 2008-09-18  4:30   ` Simon Horman
  0 siblings, 0 replies; 6+ messages in thread
From: Simon Horman @ 2008-09-18  4:30 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Andrew Morton, Linus Torvalds, Jonathan Steel, kexec, linux-kernel

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2008-09-18  4:30 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-09-16 19:18 [PATCH] kexec, kernel 2.6.26.5, segmentation fault in kimage_add_entry Jonathan Steel
2008-09-16 22:21 ` [PATCH] kexec: " Eric W. Biederman
2008-09-18  4:30   ` Simon Horman
2008-09-17  0:55 ` [PATCH] kexec, kernel 2.6.26.5, " Simon Horman
2008-09-17  2:40   ` Eric W. Biederman
2008-09-17 13:18   ` Jonathan Steel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).