Messages in this thread Patch in this message |  | | | Date | Sun, 28 Mar 2010 01:03:06 +0100 | | From | Johannes Weiner <> | | Subject | Re: [PATCH -v3] x86: Make sure free_init_pages() free pages in boundary |
| |
Hi,
On Fri, Mar 26, 2010 at 06:19:33PM -0700, Yinghai Lu wrote: > > > When CONFIG_NO_BOOTMEM, it could use memory more effient, or more compact. > > Example is: > Allocated new RAMDISK: 00ec2000 - 0248ce57 > Move RAMDISK from 000000002ea04000 - 000000002ffcee56 to 00ec2000 - 0248ce56 > > The new RAMDISK's end is not page aligned. > Last page could use shared with other user. > > When free_init_pages are called for initrd or .init, the page could be freed > could have chance to corrupt other data. > > code segment in free_init_pages() > | for (; addr < end; addr += PAGE_SIZE) { > | ClearPageReserved(virt_to_page(addr)); > | init_page_count(virt_to_page(addr)); > | memset((void *)(addr & ~(PAGE_SIZE-1)), > | POISON_FREE_INITMEM, PAGE_SIZE); > | free_page(addr); > | totalram_pages++; > | } > last half page could be used as one whole free page. > > Try to make the boundaries to be page aligned. > > -v2: make the original initramdisk to be aligned, according to Johannes. > otherwise we have chance to lose one page. > we still need to keep initrd_end not aligned, otherwise it could > confuse decompresser. > -v3: change to WARN_ON instead according to Johannes. > > Reported-by: Stanislaw Gruszka <sgruszka@redhat.com> > Signed-off-by: Yinghai Lu <yinghai@kernel.org> > Acked-by: Johannes Weiner <hannes@cmpxchg.org>
Here is what I had in mind when I wrote what you did not read, maybe diff works better?
Main differences: o only fix the area allocation in relocate_initrd(), no need to do copy the alignment bits o keep alignment fixups in free_init_pages() out of line o use PAGE_SIZE(); you might dislike the name, it is still the proper operation here. if you want to fix it, please do it properly
diff --git a/arch/x86/kernel/head32.c b/arch/x86/kernel/head32.c index adedeef..ec7c672 100644 --- a/arch/x86/kernel/head32.c +++ b/arch/x86/kernel/head32.c @@ -46,7 +46,8 @@ void __init i386_start_kernel(void) if (boot_params.hdr.type_of_loader && boot_params.hdr.ramdisk_image) { u64 ramdisk_image = boot_params.hdr.ramdisk_image; u64 ramdisk_size = boot_params.hdr.ramdisk_size; - u64 ramdisk_end = ramdisk_image + ramdisk_size; + u64 ramdisk_end = PAGE_ALIGN(ramdisk_image + ramdisk_size); + reserve_early(ramdisk_image, ramdisk_end, "RAMDISK"); } #endif diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c index b5a9896..a26a8fd 100644 --- a/arch/x86/kernel/head64.c +++ b/arch/x86/kernel/head64.c @@ -105,7 +105,9 @@ void __init x86_64_start_reservations(char *real_mode_data) if (boot_params.hdr.type_of_loader && boot_params.hdr.ramdisk_image) { unsigned long ramdisk_image = boot_params.hdr.ramdisk_image; unsigned long ramdisk_size = boot_params.hdr.ramdisk_size; - unsigned long ramdisk_end = ramdisk_image + ramdisk_size; + unsigned long ramdisk_end = PAGE_ALIGN(ramdisk_image + + ramdisk_size); + reserve_early(ramdisk_image, ramdisk_end, "RAMDISK"); } #endif diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c index ca3f8fa..0594923 100644 --- a/arch/x86/kernel/setup.c +++ b/arch/x86/kernel/setup.c @@ -317,13 +317,14 @@ static void __init relocate_initrd(void) u64 ramdisk_image = boot_params.hdr.ramdisk_image; u64 ramdisk_size = boot_params.hdr.ramdisk_size; + u64 area_size = PAGE_ALIGN(ramdisk_size); u64 end_of_lowmem = max_low_pfn_mapped << PAGE_SHIFT; u64 ramdisk_here; unsigned long slop, clen, mapaddr; char *p, *q; /* We need to move the initrd down into lowmem */ - ramdisk_here = find_e820_area(0, end_of_lowmem, ramdisk_size, + ramdisk_here = find_e820_area(0, end_of_lowmem, area_size, PAGE_SIZE); if (ramdisk_here == -1ULL) @@ -332,7 +333,7 @@ static void __init relocate_initrd(void) /* Note: this includes all the lowmem currently occupied by the initrd, we rely on that fact to keep the data intact. */ - reserve_early(ramdisk_here, ramdisk_here + ramdisk_size, + reserve_early(ramdisk_here, ramdisk_here + area_size, "NEW RAMDISK"); initrd_start = ramdisk_here + PAGE_OFFSET; initrd_end = initrd_start + ramdisk_size; @@ -378,7 +379,7 @@ static void __init reserve_initrd(void) { u64 ramdisk_image = boot_params.hdr.ramdisk_image; u64 ramdisk_size = boot_params.hdr.ramdisk_size; - u64 ramdisk_end = ramdisk_image + ramdisk_size; + u64 ramdisk_end = PAGE_ALIGN(ramdisk_image + ramdisk_size); u64 end_of_lowmem = max_low_pfn_mapped << PAGE_SHIFT; if (!boot_params.hdr.type_of_loader || diff --git a/arch/x86/mm/init.c b/arch/x86/mm/init.c index e71c5cb..018e793 100644 --- a/arch/x86/mm/init.c +++ b/arch/x86/mm/init.c @@ -336,6 +336,11 @@ void free_init_pages(char *what, unsigned long begin, unsigned long end) if (addr >= end) return; + if (WARN_ON(addr & ~PAGE_MASK || end & ~PAGE_MASK)) { + addr = PAGE_ALIGN(addr); + end &= PAGE_MASK; + } + /* * If debugging page accesses then do not free this memory but * mark them not present - any buggy init-section access will @@ -355,11 +360,10 @@ void free_init_pages(char *what, unsigned long begin, unsigned long end) printk(KERN_INFO "Freeing %s: %luk freed\n", what, (end - begin) >> 10); - for (; addr < end; addr += PAGE_SIZE) { + for (; addr != end; addr += PAGE_SIZE) { ClearPageReserved(virt_to_page(addr)); init_page_count(virt_to_page(addr)); - memset((void *)(addr & ~(PAGE_SIZE-1)), - POISON_FREE_INITMEM, PAGE_SIZE); + memset((void *)addr, POISON_FREE_INITMEM, PAGE_SIZE); free_page(addr); totalram_pages++; } @@ -376,6 +380,6 @@ void free_initmem(void) #ifdef CONFIG_BLK_DEV_INITRD void free_initrd_mem(unsigned long start, unsigned long end) { - free_init_pages("initrd memory", start, end); + free_init_pages("initrd memory", start, PAGE_ALIGN(end)); } #endif
|  |