Messages in this thread | | | Date | Sat, 27 Mar 2010 17:50:06 -0700 | From | Yinghai Lu <> | Subject | Re: [PATCH -v3] x86: Make sure free_init_pages() free pages in boundary |
| |
On 03/27/2010 05:03 PM, Johannes Weiner wrote: > 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/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++; > } something wrong here, if someone pass (0x10, 0x20), the will be aligned to [0x1000, 0] you will get dead loop
will update that.
YH
| |