2009-03-03 16:01:36

by Akinobu Mita

[permalink] [raw]
Subject: [PATCH] generic debug pagealloc

CONFIG_DEBUG_PAGEALLOC is now supported by x86, powerpc, sparc (64bit),
and s390. This patch implements it for the rest of the architectures
by filling the pages with poison byte patterns after free_pages() and
verifying the poison patterns before alloc_pages().

This generic one cannot detect invalid read accesses and it can only
detect invalid write accesses after a long delay. But it is an feasible way
for nommu architectures.

Signed-off-by: Akinobu Mita <[email protected]>
Cc: [email protected]
Cc: [email protected]
---
arch/avr32/mm/fault.c | 18 ----------
arch/powerpc/Kconfig | 3 +
arch/powerpc/Kconfig.debug | 1
arch/s390/Kconfig | 3 +
arch/s390/Kconfig.debug | 1
arch/sparc/Kconfig | 3 +
arch/sparc/Kconfig.debug | 3 +
arch/x86/Kconfig | 3 +
arch/x86/Kconfig.debug | 1
include/linux/mm_types.h | 4 ++
include/linux/poison.h | 3 +
lib/Kconfig.debug | 1
mm/Kconfig.debug | 8 ++++
mm/Makefile | 1
mm/debug-pagealloc.c | 81 +++++++++++++++++++++++++++++++++++++++++++++
15 files changed, 115 insertions(+), 19 deletions(-)

Index: 2.6-poison/include/linux/poison.h
===================================================================
--- 2.6-poison.orig/include/linux/poison.h
+++ 2.6-poison/include/linux/poison.h
@@ -17,6 +17,9 @@
*/
#define TIMER_ENTRY_STATIC ((void *) 0x74737461)

+/********** mm/debug-pagealloc.c **********/
+#define PAGE_POISON 0xaa
+
/********** mm/slab.c **********/
/*
* Magic nums for obj red zoning.
Index: 2.6-poison/mm/debug-pagealloc.c
===================================================================
--- /dev/null
+++ 2.6-poison/mm/debug-pagealloc.c
@@ -0,0 +1,81 @@
+#include <linux/kernel.h>
+#include <linux/highmem.h>
+
+static void poison_page(struct page *page)
+{
+ void *addr;
+
+ page->poison = true;
+ addr = kmap_atomic(page, KM_USER0);
+ memset(addr, PAGE_POISON, PAGE_SIZE);
+ kunmap_atomic(addr, KM_USER0);
+}
+
+static void poison_pages(struct page *page, int n)
+{
+ int i;
+
+ for (i = 0; i < n; i++)
+ poison_page(page + i);
+}
+
+static void dump_broken_mem(unsigned char *mem)
+{
+ int i;
+ int start = 0;
+ int end = PAGE_SIZE - 1;
+
+ for (i = 0; i < PAGE_SIZE; i++) {
+ if (mem[i] != PAGE_POISON) {
+ start = i;
+ break;
+ }
+ }
+ for (i = PAGE_SIZE - 1; i >= start; i--) {
+ if (mem[i] != PAGE_POISON) {
+ end = i;
+ break;
+ }
+ }
+ printk(KERN_ERR "Page corruption: %p-%p\n", mem + start, mem + end);
+ print_hex_dump(KERN_ERR, "", DUMP_PREFIX_ADDRESS, 16, 1, mem + start,
+ end - start + 1, 1);
+}
+
+static void unpoison_page(struct page *page)
+{
+ unsigned char *mem;
+ int i;
+
+ if (!page->poison)
+ return;
+
+ mem = kmap_atomic(page, KM_USER0);
+ for (i = 0; i < PAGE_SIZE; i++) {
+ if (mem[i] != PAGE_POISON) {
+ dump_broken_mem(mem);
+ break;
+ }
+ }
+ kunmap_atomic(mem, KM_USER0);
+ page->poison = false;
+}
+
+static void unpoison_pages(struct page *page, int n)
+{
+ int i;
+
+ for (i = 0; i < n; i++)
+ unpoison_page(page + i);
+}
+
+void kernel_map_pages(struct page *page, int numpages, int enable)
+{
+ if (!debug_pagealloc_enabled)
+ return;
+
+ if (enable)
+ unpoison_pages(page, numpages);
+ else
+ poison_pages(page, numpages);
+}
Index: 2.6-poison/mm/Kconfig.debug
===================================================================
--- /dev/null
+++ 2.6-poison/mm/Kconfig.debug
@@ -0,0 +1,8 @@
+config PAGE_POISONING
+ bool "Debug page memory allocations"
+ depends on !ARCH_SUPPORTS_DEBUG_PAGEALLOC
+ select DEBUG_PAGEALLOC
+ help
+ Fill the pages with poison patterns after free_pages() and verify
+ the patterns before alloc_pages(). This results in a large slowdown,
+ but helps to find certain types of memory corruptions.
Index: 2.6-poison/mm/Makefile
===================================================================
--- 2.6-poison.orig/mm/Makefile
+++ 2.6-poison/mm/Makefile
@@ -33,3 +33,4 @@ obj-$(CONFIG_MIGRATION) += migrate.o
obj-$(CONFIG_SMP) += allocpercpu.o
obj-$(CONFIG_QUICKLIST) += quicklist.o
obj-$(CONFIG_CGROUP_MEM_RES_CTLR) += memcontrol.o page_cgroup.o
+obj-$(CONFIG_PAGE_POISONING) += debug-pagealloc.o
Index: 2.6-poison/lib/Kconfig.debug
===================================================================
--- 2.6-poison.orig/lib/Kconfig.debug
+++ 2.6-poison/lib/Kconfig.debug
@@ -796,6 +796,7 @@ config SYSCTL_SYSCALL_CHECK
to properly maintain and use. This enables checks that help
you to keep things correct.

+source mm/Kconfig.debug
source kernel/trace/Kconfig

config PROVIDE_OHCI1394_DMA_INIT
Index: 2.6-poison/arch/powerpc/Kconfig.debug
===================================================================
--- 2.6-poison.orig/arch/powerpc/Kconfig.debug
+++ 2.6-poison/arch/powerpc/Kconfig.debug
@@ -30,6 +30,7 @@ config DEBUG_STACK_USAGE
config DEBUG_PAGEALLOC
bool "Debug page memory allocations"
depends on DEBUG_KERNEL && !HIBERNATION
+ depends on ARCH_SUPPORTS_DEBUG_PAGEALLOC
help
Unmap pages from the kernel linear mapping after free_pages().
This results in a large slowdown, but helps to find certain types
Index: 2.6-poison/arch/s390/Kconfig.debug
===================================================================
--- 2.6-poison.orig/arch/s390/Kconfig.debug
+++ 2.6-poison/arch/s390/Kconfig.debug
@@ -9,6 +9,7 @@ source "lib/Kconfig.debug"
config DEBUG_PAGEALLOC
bool "Debug page memory allocations"
depends on DEBUG_KERNEL
+ depends on ARCH_SUPPORTS_DEBUG_PAGEALLOC
help
Unmap pages from the kernel linear mapping after free_pages().
This results in a slowdown, but helps to find certain types of
Index: 2.6-poison/arch/sparc/Kconfig.debug
===================================================================
--- 2.6-poison.orig/arch/sparc/Kconfig.debug
+++ 2.6-poison/arch/sparc/Kconfig.debug
@@ -24,7 +24,8 @@ config STACK_DEBUG

config DEBUG_PAGEALLOC
bool "Debug page memory allocations"
- depends on SPARC64 && DEBUG_KERNEL && !HIBERNATION
+ depends on DEBUG_KERNEL && !HIBERNATION
+ depends on ARCH_SUPPORTS_DEBUG_PAGEALLOC
help
Unmap pages from the kernel linear mapping after free_pages().
This results in a large slowdown, but helps to find certain types
Index: 2.6-poison/arch/x86/Kconfig
===================================================================
--- 2.6-poison.orig/arch/x86/Kconfig
+++ 2.6-poison/arch/x86/Kconfig
@@ -160,6 +160,9 @@ config AUDIT_ARCH
config ARCH_SUPPORTS_OPTIMIZED_INLINING
def_bool y

+config ARCH_SUPPORTS_DEBUG_PAGEALLOC
+ def_bool y
+
# Use the generic interrupt handling code in kernel/irq/:
config GENERIC_HARDIRQS
bool
Index: 2.6-poison/arch/x86/Kconfig.debug
===================================================================
--- 2.6-poison.orig/arch/x86/Kconfig.debug
+++ 2.6-poison/arch/x86/Kconfig.debug
@@ -75,6 +75,7 @@ config DEBUG_STACK_USAGE
config DEBUG_PAGEALLOC
bool "Debug page memory allocations"
depends on DEBUG_KERNEL
+ depends on ARCH_SUPPORTS_DEBUG_PAGEALLOC
help
Unmap pages from the kernel linear mapping after free_pages().
This results in a large slowdown, but helps to find certain types
Index: 2.6-poison/arch/powerpc/Kconfig
===================================================================
--- 2.6-poison.orig/arch/powerpc/Kconfig
+++ 2.6-poison/arch/powerpc/Kconfig
@@ -227,6 +227,9 @@ config PPC_OF_PLATFORM_PCI
depends on PPC64 # not supported on 32 bits yet
default n

+config ARCH_SUPPORTS_DEBUG_PAGEALLOC
+ def_bool y
+
source "init/Kconfig"

source "kernel/Kconfig.freezer"
Index: 2.6-poison/arch/sparc/Kconfig
===================================================================
--- 2.6-poison.orig/arch/sparc/Kconfig
+++ 2.6-poison/arch/sparc/Kconfig
@@ -124,6 +124,9 @@ config ARCH_NO_VIRT_TO_BUS
config OF
def_bool y

+config ARCH_SUPPORTS_DEBUG_PAGEALLOC
+ def_bool y if SPARC64
+
source "init/Kconfig"

source "kernel/Kconfig.freezer"
Index: 2.6-poison/arch/avr32/mm/fault.c
===================================================================
--- 2.6-poison.orig/arch/avr32/mm/fault.c
+++ 2.6-poison/arch/avr32/mm/fault.c
@@ -250,21 +250,3 @@ asmlinkage void do_bus_error(unsigned lo
dump_dtlb();
die("Bus Error", regs, SIGKILL);
}
-
-/*
- * This functionality is currently not possible to implement because
- * we're using segmentation to ensure a fixed mapping of the kernel
- * virtual address space.
- *
- * It would be possible to implement this, but it would require us to
- * disable segmentation at startup and load the kernel mappings into
- * the TLB like any other pages. There will be lots of trickery to
- * avoid recursive invocation of the TLB miss handler, though...
- */
-#ifdef CONFIG_DEBUG_PAGEALLOC
-void kernel_map_pages(struct page *page, int numpages, int enable)
-{
-
-}
-EXPORT_SYMBOL(kernel_map_pages);
-#endif
Index: 2.6-poison/arch/s390/Kconfig
===================================================================
--- 2.6-poison.orig/arch/s390/Kconfig
+++ 2.6-poison/arch/s390/Kconfig
@@ -72,6 +72,9 @@ config PGSTE
config VIRT_CPU_ACCOUNTING
def_bool y

+config ARCH_SUPPORTS_DEBUG_PAGEALLOC
+ def_bool y
+
mainmenu "Linux Kernel Configuration"

config S390
Index: 2.6-poison/include/linux/mm_types.h
===================================================================
--- 2.6-poison.orig/include/linux/mm_types.h
+++ 2.6-poison/include/linux/mm_types.h
@@ -94,6 +94,10 @@ struct page {
void *virtual; /* Kernel virtual address (NULL if
not kmapped, ie. highmem) */
#endif /* WANT_PAGE_VIRTUAL */
+
+#ifdef CONFIG_PAGE_POISONING
+ bool poison;
+#endif /* CONFIG_PAGE_POISONING */
};

/*


2009-03-03 16:05:24

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] generic debug pagealloc


* Akinobu Mita <[email protected]> wrote:

> CONFIG_DEBUG_PAGEALLOC is now supported by x86, powerpc, sparc
> (64bit), and s390. This patch implements it for the rest of
> the architectures by filling the pages with poison byte
> patterns after free_pages() and verifying the poison patterns
> before alloc_pages().
>
> This generic one cannot detect invalid read accesses and it
> can only detect invalid write accesses after a long delay. But
> it is an feasible way for nommu architectures.

if every architecture supports it now then i guess this config
switch can go away:

> +config ARCH_SUPPORTS_DEBUG_PAGEALLOC
> + def_bool y

Ingo

2009-03-03 16:12:53

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH] generic debug pagealloc

On 3.3.2009 17:01, Akinobu Mita wrote:
...
> +static void dump_broken_mem(unsigned char *mem)
> +{
> + int i;
> + int start = 0;
> + int end = PAGE_SIZE - 1;
> +
> + for (i = 0; i< PAGE_SIZE; i++) {
> + if (mem[i] != PAGE_POISON) {
> + start = i;
> + break;
> + }
> + }
> + for (i = PAGE_SIZE - 1; i>= start; i--) {
> + if (mem[i] != PAGE_POISON) {
> + end = i;
> + break;
> + }
> + }
> + printk(KERN_ERR "Page corruption: %p-%p\n", mem + start, mem + end);
> + print_hex_dump(KERN_ERR, "", DUMP_PREFIX_ADDRESS, 16, 1, mem + start,
> + end - start + 1, 1);
> +}
> +
> +static void unpoison_page(struct page *page)
> +{
> + unsigned char *mem;
> + int i;
> +
> + if (!page->poison)
> + return;
> +
> + mem = kmap_atomic(page, KM_USER0);
> + for (i = 0; i< PAGE_SIZE; i++) {
> + if (mem[i] != PAGE_POISON) {
> + dump_broken_mem(mem);

Just an optimisation: pass the i to the dump_broken_mem as a start index.

> + break;
> + }
> + }
> + kunmap_atomic(mem, KM_USER0);
> + page->poison = false;
> +}

2009-03-03 21:36:55

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] generic debug pagealloc

On Wed, 4 Mar 2009 01:01:04 +0900
Akinobu Mita <[email protected]> wrote:

> +static void unpoison_page(struct page *page)
> +{
> + unsigned char *mem;
> + int i;
> +
> + if (!page->poison)
> + return;
> +
> + mem = kmap_atomic(page, KM_USER0);
> + for (i = 0; i < PAGE_SIZE; i++) {
> + if (mem[i] != PAGE_POISON) {
> + dump_broken_mem(mem);
> + break;
> + }
> + }
> + kunmap_atomic(mem, KM_USER0);
> + page->poison = false;
> +}
> +
> +static void unpoison_pages(struct page *page, int n)
> +{
> + int i;
> +
> + for (i = 0; i < n; i++)
> + unpoison_page(page + i);
> +}
> +
> +void kernel_map_pages(struct page *page, int numpages, int enable)
> +{
> + if (!debug_pagealloc_enabled)
> + return;
> +
> + if (enable)
> + unpoison_pages(page, numpages);
> + else
> + poison_pages(page, numpages);
> +}

kernel_map_pages() is called from the memory-allocation and
memory-freeing paths. Hence it can be called from interrupt contexts.

KM_USER0 must not be used from interrupt context - it will corrupt the
non-interrupt context's pte, causing unpleasing very hard to track down
memory corruption. Often memory which is getting written to the user's
disk. This makes users unhappy.

We could use KM_IRQ0 here. The code should disable local interrupts
when holding a KM_IRQ0 kmap.

If this code were to switch to using KM_IRQ0 then we still have a
problem - if any other place in the kernel does a memory allcoation or
free while holding a KM_IRQ0 kmap, then this new code will corrupt the
caller's pte.

So I guess we'll need to create a new kmap_atomic slot for this
application. It will need interrupt protection - the page allocator can
be called from interrupt context while it is already running in
non-interrupt context.


Alternatively, we could just not do the kmap_atomic() at all. i386
won't be using this code and IIRC the only other highmem architecture
is powerpc32, and ppc32 appears to also have its own DEBUG_PAGEALLOC
implementation. So you could remove the kmap_atomic() stuff and put

#ifdef CONFIG_HIGHMEM
#error i goofed
#endif

in there.

2009-03-03 22:12:33

by Russell King

[permalink] [raw]
Subject: Re: [PATCH] generic debug pagealloc

On Tue, Mar 03, 2009 at 01:36:10PM -0800, Andrew Morton wrote:
> Alternatively, we could just not do the kmap_atomic() at all. i386
> won't be using this code and IIRC the only other highmem architecture
> is powerpc32, and ppc32 appears to also have its own DEBUG_PAGEALLOC
> implementation. So you could remove the kmap_atomic() stuff and put

ARM will also be joining the highmem club in due course, maybe during
the next merge window depending on how things pan out.

The biggest issue we have is with kmaps interacting with ARM's DMA API
code. Nicolas Pitre currently has a work-around for it, but I believe
it can be better handled by extending the generic kmap infrastructure
to be able to grab a reference on an already kmapped page. I've asked
Nicolas to discuss this aspect of his patch set on lkml.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:

2009-03-04 14:13:18

by Akinobu Mita

[permalink] [raw]
Subject: Re: [PATCH] generic debug pagealloc

On Tue, Mar 03, 2009 at 05:05:03PM +0100, Ingo Molnar wrote:
> if every architecture supports it now then i guess this config
> switch can go away:
>
> > +config ARCH_SUPPORTS_DEBUG_PAGEALLOC
> > + def_bool y

The generic debug pagealloc needs the poison flag for each struct page.

So I introduced CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC for x86, powerpc,
sparc (64bit), and s390. If there is no CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC,
make config prompts the generic debug pagealloc in mm/Kconfig.debug for the
other architectures.

I was trying to add cleaner config dependency but
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC is my solution for now.

2009-03-04 14:14:08

by Akinobu Mita

[permalink] [raw]
Subject: Re: [PATCH] generic debug pagealloc

On Tue, Mar 03, 2009 at 05:12:35PM +0100, Jiri Slaby wrote:
> Just an optimisation: pass the i to the dump_broken_mem as a start index.

OK. I'll remove that redundancy in next verserion.

2009-03-04 14:17:42

by Akinobu Mita

[permalink] [raw]
Subject: Re: [PATCH] generic debug pagealloc

On Tue, Mar 03, 2009 at 01:36:10PM -0800, Andrew Morton wrote:
> Alternatively, we could just not do the kmap_atomic() at all. i386
> won't be using this code and IIRC the only other highmem architecture
> is powerpc32, and ppc32 appears to also have its own DEBUG_PAGEALLOC
> implementation. So you could remove the kmap_atomic() stuff and put
>
> #ifdef CONFIG_HIGHMEM
> #error i goofed
> #endif
>
> in there.

I'll take the variant of this. Then poison_page() will be

static void poison_page(struct page *page)
{
void *addr;

if (PageHighmem(page))
return; // i goofed

page->poison = true;
addr = page_address(page);
memset(addr, PAGE_POISON, PAGE_SIZE);
}

2009-03-06 09:14:53

by Benjamin Herrenschmidt

[permalink] [raw]
Subject: Re: [PATCH] generic debug pagealloc

On Tue, 2009-03-03 at 13:36 -0800, Andrew Morton wrote:
> Alternatively, we could just not do the kmap_atomic() at all. i386
> won't be using this code and IIRC the only other highmem architecture
> is powerpc32, and ppc32 appears to also have its own DEBUG_PAGEALLOC
> implementation. So you could remove the kmap_atomic() stuff and put
>
Actually, ppc32 DEBUG_PAGEALLOC is busted in several ways and probably
unfixable (though this is still being debated).

Cheers,
Ben.