Re: [PATCH, V2] i386: instead of poisoning .init zone, change protection bits to force a fault

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Andrew Morton a écrit :
Eric Dumazet <[email protected]> wrote:

Chasing some invalid accesses to .init zone, I found that free_init_pages() was properly freeing the pages but virtual was still usable.

A poisoning (memset(page, 0xcc, PAGE_SIZE)) was done but this is not reliable.

A new config option DEBUG_INITDATA is introduced to mark this initdata as not present at all so that buggy code can trigger a fault.

This option is not meant for production machines because it may split one or two huge page (2MB or 4MB) into small pages and thus slow down kernel a bit.

(After that we could map non possible cpu percpu data to the initial percpudata that is included in .init and discarded in free_initmem())

...

--- a/arch/i386/mm/init.c	2006-01-25 10:17:24.000000000 +0100
+++ b/arch/i386/mm/init.c	2006-01-29 22:38:53.000000000 +0100
@@ -750,11 +750,18 @@
 	for (addr = begin; addr < end; addr += PAGE_SIZE) {
 		ClearPageReserved(virt_to_page(addr));
 		set_page_count(virt_to_page(addr), 1);
+#ifdef CONFIG_DEBUG_INITDATA
+		change_page_attr(virt_to_page(addr), 1, __pgprot(0));
+#else
 		memset((void *)addr, 0xcc, PAGE_SIZE);
+#endif
 		free_page(addr);
 		totalram_pages++;
 	}
 	printk(KERN_INFO "Freeing %s: %ldk freed\n", what, (end - begin) >> 10);
+#ifdef CONFIG_DEBUG_INITDATA
+	global_flush_tlb();
+#endif
 }

This doesn't seem very pointful.

We unmap the page, then return it to the page allocator.  Then someone
reallocates the page, tries to use it and goes oops.

If CONFIG_DEBUG_PAGEALLOC is also set, the kernel will remap the page when
it's allocated and everything works OK.  So this patch requires
CONFIG_DEBUG_PAGEALLOC.

But if CONFIG_DEBUG_PAGEALLOC is set, we'll have unmapped that page in
free_page() _anyway_, so why bother using this patch?

The only enhancement I can think of here is to not free the page, so it's
permanently leaked and permanently unmapped.

--- devel/arch/i386/mm/init.c~i386-instead-of-poisoning-init-zone-change-protection-fix	2006-02-04 14:33:33.000000000 -0800
+++ devel-akpm/arch/i386/mm/init.c	2006-02-04 14:34:07.000000000 -0800
@@ -751,11 +751,15 @@ void free_init_pages(char *what, unsigne
 		ClearPageReserved(virt_to_page(addr));
 		set_page_count(virt_to_page(addr), 1);
 #ifdef CONFIG_DEBUG_INITDATA
+		/*
+		 * Unmap the page, and leak it.  So any further accesses will
+		 * oops.
+		 */
 		change_page_attr(virt_to_page(addr), 1, __pgprot(0));
 #else
 		memset((void *)addr, 0xcc, PAGE_SIZE);
-#endif
 		free_page(addr);
+#endif
 		totalram_pages++;
 	}
 	printk(KERN_INFO "Freeing %s: %ldk freed\n", what, (end - begin) >> 10);
_

But is there much point in doing this?  Does it offer much more than
CONFIG_DEBUG_PAGEALLOC?



1) CONFIG_DEBUG_PAGEALLOC is very generic (and expensive), while CONFIG_DEBUG_INITDATA only makes sure init data becomes unreadable.

2) If CONFIG_DEBUG_INITDATA is on, the redzone is in action in the virtual memory that hold the initdata, while the physical pages that contained the initdata where freed and might be reused for other needs.

I think we have two different things here : Virtual mem redzoning (my patch), and physical ram poisoning (your patch).

CONFIG_DEBUG_INITDATA uses only a virtual mem redzoning (no underlying memory cost, apart of the page tables), while your solution doesnt free the pages, and the poisoining wont catch further accesses, just make some results funny or false.

The only bad effect of my patch is about the TLB cost, because of the hugepage(s) that should revert to 512 normal 4K pages.

Eric
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux