From: Andrea Arcangeli - fix silent memleak in the pageattr code that I found while searching for the bug Andi fixed in the second patch below (basically reference counting in split page was done on the pmd instead of the pte). - Part of this patch is also needed to make the above work on x86 (otherwise one of my new above BUGS() will trigger signalling the fact a bug was there). The below patch creates a subtle dependency that (_PAGE_PCD << 24) must not be zero. It's not the cleanest thing ever, but since it's an hardware bitflag I doubt it's going to break. Signed-off-by: Andi Kleen Signed-off-by: Andrea Arcangeli Signed-off-by: Andrew Morton --- 25-akpm/arch/i386/mm/ioremap.c | 4 ++-- 25-akpm/arch/i386/mm/pageattr.c | 13 +++++++------ 25-akpm/arch/x86_64/mm/ioremap.c | 14 +++++++------- 25-akpm/arch/x86_64/mm/pageattr.c | 23 ++++++++++++++--------- 4 files changed, 30 insertions(+), 24 deletions(-) diff -puN arch/i386/mm/ioremap.c~fix-iounmap-and-a-pageattr-memleak-x86-and-x86-64 arch/i386/mm/ioremap.c --- 25/arch/i386/mm/ioremap.c~fix-iounmap-and-a-pageattr-memleak-x86-and-x86-64 Thu Oct 28 15:41:41 2004 +++ 25-akpm/arch/i386/mm/ioremap.c Thu Oct 28 15:41:41 2004 @@ -152,7 +152,7 @@ void __iomem * __ioremap(unsigned long p /* * Ok, go for it.. */ - area = get_vm_area(size, VM_IOREMAP); + area = get_vm_area(size, VM_IOREMAP | (flags << 24)); if (!area) return NULL; area->phys_addr = phys_addr; @@ -232,7 +232,7 @@ void iounmap(volatile void __iomem *addr return; } - if (p->flags && p->phys_addr < virt_to_phys(high_memory)) { + if ((p->flags >> 24) && p->phys_addr < virt_to_phys(high_memory)) { change_page_attr(virt_to_page(__va(p->phys_addr)), p->size >> PAGE_SHIFT, PAGE_KERNEL); diff -puN arch/i386/mm/pageattr.c~fix-iounmap-and-a-pageattr-memleak-x86-and-x86-64 arch/i386/mm/pageattr.c --- 25/arch/i386/mm/pageattr.c~fix-iounmap-and-a-pageattr-memleak-x86-and-x86-64 Thu Oct 28 15:41:41 2004 +++ 25-akpm/arch/i386/mm/pageattr.c Thu Oct 28 15:41:41 2004 @@ -117,22 +117,23 @@ __change_page_attr(struct page *page, pg kpte_page = virt_to_page(kpte); if (pgprot_val(prot) != pgprot_val(PAGE_KERNEL)) { if ((pte_val(*kpte) & _PAGE_PSE) == 0) { - pte_t old = *kpte; - pte_t standard = mk_pte(page, PAGE_KERNEL); set_pte_atomic(kpte, mk_pte(page, prot)); - if (pte_same(old,standard)) - get_page(kpte_page); } else { struct page *split = split_large_page(address, prot); if (!split) return -ENOMEM; - get_page(kpte_page); set_pmd_pte(kpte,address,mk_pte(split, PAGE_KERNEL)); + kpte_page = split; } + get_page(kpte_page); } else if ((pte_val(*kpte) & _PAGE_PSE) == 0) { set_pte_atomic(kpte, mk_pte(page, PAGE_KERNEL)); __put_page(kpte_page); - } + } else + BUG(); + + /* memleak and potential failed 2M page regeneration */ + BUG_ON(!page_count(kpte_page)); if (cpu_has_pse && (page_count(kpte_page) == 1)) { list_add(&kpte_page->lru, &df_list); diff -puN arch/x86_64/mm/ioremap.c~fix-iounmap-and-a-pageattr-memleak-x86-and-x86-64 arch/x86_64/mm/ioremap.c --- 25/arch/x86_64/mm/ioremap.c~fix-iounmap-and-a-pageattr-memleak-x86-and-x86-64 Thu Oct 28 15:41:41 2004 +++ 25-akpm/arch/x86_64/mm/ioremap.c Thu Oct 28 15:41:41 2004 @@ -128,11 +128,11 @@ void __iomem * __ioremap(unsigned long p if (phys_addr >= 0xA0000 && last_addr < 0x100000) return (__force void __iomem *)phys_to_virt(phys_addr); +#ifndef CONFIG_DISCONTIGMEM /* * Don't allow anybody to remap normal RAM that we're using.. */ if (phys_addr < virt_to_phys(high_memory)) { -#ifndef CONFIG_DISCONTIGMEM char *t_addr, *t_end; struct page *page; @@ -142,8 +142,8 @@ void __iomem * __ioremap(unsigned long p for(page = virt_to_page(t_addr); page <= virt_to_page(t_end); page++) if(!PageReserved(page)) return NULL; -#endif } +#endif /* * Mappings have to be page-aligned @@ -155,7 +155,7 @@ void __iomem * __ioremap(unsigned long p /* * Ok, go for it.. */ - area = get_vm_area(size, VM_IOREMAP); + area = get_vm_area(size, VM_IOREMAP | (flags << 24)); if (!area) return NULL; area->phys_addr = phys_addr; @@ -195,12 +195,12 @@ void __iomem *ioremap_nocache (unsigned if (!p) return p; - if (phys_addr + size < virt_to_phys(high_memory)) { + if (phys_addr + size - 1 < virt_to_phys(high_memory)) { struct page *ppage = virt_to_page(__va(phys_addr)); unsigned long npages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT; - BUG_ON(phys_addr+size > (unsigned long)high_memory); - BUG_ON(phys_addr + size < phys_addr); + BUG_ON(phys_addr+size >= (unsigned long)high_memory); + BUG_ON(phys_addr + size <= phys_addr); if (change_page_attr(ppage, npages, PAGE_KERNEL_NOCACHE) < 0) { iounmap(p); @@ -223,7 +223,7 @@ void iounmap(volatile void __iomem *addr return; } - if (p->flags && p->phys_addr < virt_to_phys(high_memory)) { + if ((p->flags >> 24) && p->phys_addr + p->size < virt_to_phys(high_memory)) { change_page_attr(virt_to_page(__va(p->phys_addr)), p->size >> PAGE_SHIFT, PAGE_KERNEL); diff -puN arch/x86_64/mm/pageattr.c~fix-iounmap-and-a-pageattr-memleak-x86-and-x86-64 arch/x86_64/mm/pageattr.c --- 25/arch/x86_64/mm/pageattr.c~fix-iounmap-and-a-pageattr-memleak-x86-and-x86-64 Thu Oct 28 15:41:41 2004 +++ 25-akpm/arch/x86_64/mm/pageattr.c Thu Oct 28 15:41:41 2004 @@ -124,28 +124,33 @@ __change_page_attr(unsigned long address kpte_flags = pte_val(*kpte); if (pgprot_val(prot) != pgprot_val(ref_prot)) { if ((kpte_flags & _PAGE_PSE) == 0) { - pte_t old = *kpte; - pte_t standard = mk_pte(page, ref_prot); - set_pte(kpte, mk_pte(page, prot)); - if (pte_same(old,standard)) - get_page(kpte_page); } else { + /* + * split_large_page will take the reference for this change_page_attr + * on the split page. + */ struct page *split = split_large_page(address, prot, ref_prot); if (!split) return -ENOMEM; - get_page(kpte_page); set_pte(kpte,mk_pte(split, ref_prot)); + kpte_page = split; } + get_page(kpte_page); } else if ((kpte_flags & _PAGE_PSE) == 0) { set_pte(kpte, mk_pte(page, ref_prot)); __put_page(kpte_page); - } + } else + BUG(); - if (page_count(kpte_page) == 1) { + switch (page_count(kpte_page)) { + case 1: save_page(address, kpte_page); revert_page(address, ref_prot); - } + break; + case 0: + BUG(); /* memleak and failed 2M page regeneration */ + } return 0; } _