[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 at esentire.com>
>>
>> Jonathan Steel
>>     
>
>