All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Sanitizing freed pages
@ 2015-05-04 21:16 ` Anisse Astier
  0 siblings, 0 replies; 24+ messages in thread
From: Anisse Astier @ 2015-05-04 21:16 UTC (permalink / raw)
  Cc: Anisse Astier, Andrew Morton, Mel Gorman, Kirill A. Shutemov,
	David Rientjes, Alan Cox, Linus Torvalds, Peter Zijlstra,
	PaX Team, Brad Spengler, Kees Cook, Andi Kleen, linux-mm,
	linux-kernel

Hi,

I'm trying revive an old debate here[1], though with a simpler approach than
was previously tried. This patch series implements a new option to sanitize
freed pages, a (very) small subset of what is done in PaX/grsecurity[3],
inspired by a previous submission [4].

The first patch is fairly independent, and could be taken as-is. The second is
the meat and should be straight-forward to review.

There are a few different uses that this can cover:
 - some cases of use-after-free could be detected (crashes), although this not
   as efficient as KAsan/kmemcheck
 - it can help with long-term memory consumption in an environment with
   multiple VMs and Kernel Same-page Merging on the host. [2]
 - finally, it can reduce infoleaks, although this is hard to measure.

The approach is voluntarily kept as simple as possible. A single configuration
option, no command line option, no sysctl nob. It can of course be changed,
although I'd be wary of runtime-configuration options that could be used for
races.

I haven't been able to measure a meaningful performance difference when
compiling a (in-cache) kernel; I'd be interested to see what difference it
makes with your particular workload/hardware (I suspect mine is CPU-bound on
this small laptop).

Changes since v1:
 - fix some issues raised by David Rientjes, Andi Kleen and PaX Team.
 - add hibernate fix (third patch)
 - add debug code, this is "just in case" someone has an issue with this
   feature. Not sure if it should be merged.


[1] https://lwn.net/Articles/334747/
[2] https://staff.aist.go.jp/k.suzaki/EuroSec12-SUZAKI-revised2.pdf
[3] http://en.wikibooks.org/wiki/Grsecurity/Appendix/Grsecurity_and_PaX_Configuration_Options#Sanitize_all_freed_memory
[4] http://article.gmane.org/gmane.linux.kernel.mm/34398



Anisse Astier (4):
  mm/page_alloc.c: cleanup obsolete KM_USER*
  mm/page_alloc.c: add config option to sanitize freed pages
  PM / Hibernate: fix SANITIZE_FREED_PAGES
  mm: Add debug code for SANITIZE_FREED_PAGES

 kernel/power/hibernate.c |  7 ++++++-
 kernel/power/power.h     |  4 ++++
 kernel/power/snapshot.c  | 24 ++++++++++++++++++++++
 mm/Kconfig               | 22 ++++++++++++++++++++
 mm/page_alloc.c          | 52 ++++++++++++++++++++++++++++++++++--------------
 5 files changed, 93 insertions(+), 16 deletions(-)

-- 
1.9.3


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 0/4] Sanitizing freed pages
@ 2015-05-04 21:16 ` Anisse Astier
  0 siblings, 0 replies; 24+ messages in thread
From: Anisse Astier @ 2015-05-04 21:16 UTC (permalink / raw)
  Cc: Anisse Astier, Andrew Morton, Mel Gorman, Kirill A. Shutemov,
	David Rientjes, Alan Cox, Linus Torvalds, Peter Zijlstra,
	PaX Team, Brad Spengler, Kees Cook, Andi Kleen, linux-mm,
	linux-kernel

Hi,

I'm trying revive an old debate here[1], though with a simpler approach than
was previously tried. This patch series implements a new option to sanitize
freed pages, a (very) small subset of what is done in PaX/grsecurity[3],
inspired by a previous submission [4].

The first patch is fairly independent, and could be taken as-is. The second is
the meat and should be straight-forward to review.

There are a few different uses that this can cover:
 - some cases of use-after-free could be detected (crashes), although this not
   as efficient as KAsan/kmemcheck
 - it can help with long-term memory consumption in an environment with
   multiple VMs and Kernel Same-page Merging on the host. [2]
 - finally, it can reduce infoleaks, although this is hard to measure.

The approach is voluntarily kept as simple as possible. A single configuration
option, no command line option, no sysctl nob. It can of course be changed,
although I'd be wary of runtime-configuration options that could be used for
races.

I haven't been able to measure a meaningful performance difference when
compiling a (in-cache) kernel; I'd be interested to see what difference it
makes with your particular workload/hardware (I suspect mine is CPU-bound on
this small laptop).

Changes since v1:
 - fix some issues raised by David Rientjes, Andi Kleen and PaX Team.
 - add hibernate fix (third patch)
 - add debug code, this is "just in case" someone has an issue with this
   feature. Not sure if it should be merged.


[1] https://lwn.net/Articles/334747/
[2] https://staff.aist.go.jp/k.suzaki/EuroSec12-SUZAKI-revised2.pdf
[3] http://en.wikibooks.org/wiki/Grsecurity/Appendix/Grsecurity_and_PaX_Configuration_Options#Sanitize_all_freed_memory
[4] http://article.gmane.org/gmane.linux.kernel.mm/34398



Anisse Astier (4):
  mm/page_alloc.c: cleanup obsolete KM_USER*
  mm/page_alloc.c: add config option to sanitize freed pages
  PM / Hibernate: fix SANITIZE_FREED_PAGES
  mm: Add debug code for SANITIZE_FREED_PAGES

 kernel/power/hibernate.c |  7 ++++++-
 kernel/power/power.h     |  4 ++++
 kernel/power/snapshot.c  | 24 ++++++++++++++++++++++
 mm/Kconfig               | 22 ++++++++++++++++++++
 mm/page_alloc.c          | 52 ++++++++++++++++++++++++++++++++++--------------
 5 files changed, 93 insertions(+), 16 deletions(-)

-- 
1.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v2 1/4] mm/page_alloc.c: cleanup obsolete KM_USER*
  2015-05-04 21:16 ` Anisse Astier
@ 2015-05-04 21:16   ` Anisse Astier
  -1 siblings, 0 replies; 24+ messages in thread
From: Anisse Astier @ 2015-05-04 21:16 UTC (permalink / raw)
  Cc: Anisse Astier, Andrew Morton, Mel Gorman, Kirill A. Shutemov,
	David Rientjes, Alan Cox, Linus Torvalds, Peter Zijlstra,
	PaX Team, Brad Spengler, Kees Cook, Andi Kleen, linux-mm,
	linux-kernel

It's been five years now that KM_* kmap flags have been removed and
that we can call clear_highpage from any context. So we remove
prep_zero_pages accordingly.

Signed-off-by: Anisse Astier <anisse@astier.eu>
---
 mm/page_alloc.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index ebffa0e..4d5ce6e 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -380,20 +380,6 @@ void prep_compound_page(struct page *page, unsigned long order)
 	}
 }
 
-static inline void prep_zero_page(struct page *page, unsigned int order,
-							gfp_t gfp_flags)
-{
-	int i;
-
-	/*
-	 * clear_highpage() will use KM_USER0, so it's a bug to use __GFP_ZERO
-	 * and __GFP_HIGHMEM from hard or soft interrupt context.
-	 */
-	VM_BUG_ON((gfp_flags & __GFP_HIGHMEM) && in_interrupt());
-	for (i = 0; i < (1 << order); i++)
-		clear_highpage(page + i);
-}
-
 #ifdef CONFIG_DEBUG_PAGEALLOC
 unsigned int _debug_guardpage_minorder;
 bool _debug_pagealloc_enabled __read_mostly;
@@ -975,7 +961,8 @@ static int prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
 	kasan_alloc_pages(page, order);
 
 	if (gfp_flags & __GFP_ZERO)
-		prep_zero_page(page, order, gfp_flags);
+		for (i = 0; i < (1 << order); i++)
+			clear_highpage(page + i);
 
 	if (order && (gfp_flags & __GFP_COMP))
 		prep_compound_page(page, order);
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 1/4] mm/page_alloc.c: cleanup obsolete KM_USER*
@ 2015-05-04 21:16   ` Anisse Astier
  0 siblings, 0 replies; 24+ messages in thread
From: Anisse Astier @ 2015-05-04 21:16 UTC (permalink / raw)
  Cc: Anisse Astier, Andrew Morton, Mel Gorman, Kirill A. Shutemov,
	David Rientjes, Alan Cox, Linus Torvalds, Peter Zijlstra,
	PaX Team, Brad Spengler, Kees Cook, Andi Kleen, linux-mm,
	linux-kernel

It's been five years now that KM_* kmap flags have been removed and
that we can call clear_highpage from any context. So we remove
prep_zero_pages accordingly.

Signed-off-by: Anisse Astier <anisse@astier.eu>
---
 mm/page_alloc.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index ebffa0e..4d5ce6e 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -380,20 +380,6 @@ void prep_compound_page(struct page *page, unsigned long order)
 	}
 }
 
-static inline void prep_zero_page(struct page *page, unsigned int order,
-							gfp_t gfp_flags)
-{
-	int i;
-
-	/*
-	 * clear_highpage() will use KM_USER0, so it's a bug to use __GFP_ZERO
-	 * and __GFP_HIGHMEM from hard or soft interrupt context.
-	 */
-	VM_BUG_ON((gfp_flags & __GFP_HIGHMEM) && in_interrupt());
-	for (i = 0; i < (1 << order); i++)
-		clear_highpage(page + i);
-}
-
 #ifdef CONFIG_DEBUG_PAGEALLOC
 unsigned int _debug_guardpage_minorder;
 bool _debug_pagealloc_enabled __read_mostly;
@@ -975,7 +961,8 @@ static int prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
 	kasan_alloc_pages(page, order);
 
 	if (gfp_flags & __GFP_ZERO)
-		prep_zero_page(page, order, gfp_flags);
+		for (i = 0; i < (1 << order); i++)
+			clear_highpage(page + i);
 
 	if (order && (gfp_flags & __GFP_COMP))
 		prep_compound_page(page, order);
-- 
1.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 2/4] mm/page_alloc.c: add config option to sanitize freed pages
  2015-05-04 21:16 ` Anisse Astier
@ 2015-05-04 21:16   ` Anisse Astier
  -1 siblings, 0 replies; 24+ messages in thread
From: Anisse Astier @ 2015-05-04 21:16 UTC (permalink / raw)
  Cc: Anisse Astier, Andrew Morton, Mel Gorman, Kirill A. Shutemov,
	David Rientjes, Alan Cox, Linus Torvalds, Peter Zijlstra,
	PaX Team, Brad Spengler, Kees Cook, Andi Kleen, linux-mm,
	linux-kernel

This new config option will sanitize all freed pages. This is a pretty
low-level change useful to track some cases of use-after-free, help
kernel same-page merging in VM environments, and counter a few info
leaks.

Signed-off-by: Anisse Astier <anisse@astier.eu>
---
 mm/Kconfig      | 12 ++++++++++++
 mm/page_alloc.c | 12 ++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/mm/Kconfig b/mm/Kconfig
index 390214d..e9fb3bd 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -635,3 +635,15 @@ config MAX_STACK_SIZE_MB
 	  changed to a smaller value in which case that is used.
 
 	  A sane initial value is 80 MB.
+
+config SANITIZE_FREED_PAGES
+	bool "Sanitize memory pages after free"
+	default n
+	help
+	  This option is used to make sure all pages freed are zeroed. This is
+	  quite low-level and doesn't handle your slab buffers.
+	  It has various applications, from preventing some info leaks to
+	  helping kernel same-page merging in virtualised environments.
+	  Depending on your workload it will greatly reduce performance.
+
+	  If unsure, say N.
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 4d5ce6e..c29e3a0 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -795,6 +795,12 @@ static bool free_pages_prepare(struct page *page, unsigned int order)
 		debug_check_no_obj_freed(page_address(page),
 					   PAGE_SIZE << order);
 	}
+
+#ifdef CONFIG_SANITIZE_FREED_PAGES
+	for (i = 0; i < (1 << order); i++)
+		clear_highpage(page + i);
+#endif
+
 	arch_free_page(page, order);
 	kernel_map_pages(page, 1 << order, 0);
 
@@ -960,9 +966,15 @@ static int prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
 	kernel_map_pages(page, 1 << order, 1);
 	kasan_alloc_pages(page, order);
 
+#ifndef CONFIG_SANITIZE_FREED_PAGES
+	/* SANITIZE_FREED_PAGES relies implicitly on the fact that pages are
+	 * cleared before use, so we don't need gfp zero in the default case
+	 * because all pages go through the free_pages_prepare code path when
+	 * switching from bootmem to the default allocator */
 	if (gfp_flags & __GFP_ZERO)
 		for (i = 0; i < (1 << order); i++)
 			clear_highpage(page + i);
+#endif
 
 	if (order && (gfp_flags & __GFP_COMP))
 		prep_compound_page(page, order);
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 2/4] mm/page_alloc.c: add config option to sanitize freed pages
@ 2015-05-04 21:16   ` Anisse Astier
  0 siblings, 0 replies; 24+ messages in thread
From: Anisse Astier @ 2015-05-04 21:16 UTC (permalink / raw)
  Cc: Anisse Astier, Andrew Morton, Mel Gorman, Kirill A. Shutemov,
	David Rientjes, Alan Cox, Linus Torvalds, Peter Zijlstra,
	PaX Team, Brad Spengler, Kees Cook, Andi Kleen, linux-mm,
	linux-kernel

This new config option will sanitize all freed pages. This is a pretty
low-level change useful to track some cases of use-after-free, help
kernel same-page merging in VM environments, and counter a few info
leaks.

Signed-off-by: Anisse Astier <anisse@astier.eu>
---
 mm/Kconfig      | 12 ++++++++++++
 mm/page_alloc.c | 12 ++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/mm/Kconfig b/mm/Kconfig
index 390214d..e9fb3bd 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -635,3 +635,15 @@ config MAX_STACK_SIZE_MB
 	  changed to a smaller value in which case that is used.
 
 	  A sane initial value is 80 MB.
+
+config SANITIZE_FREED_PAGES
+	bool "Sanitize memory pages after free"
+	default n
+	help
+	  This option is used to make sure all pages freed are zeroed. This is
+	  quite low-level and doesn't handle your slab buffers.
+	  It has various applications, from preventing some info leaks to
+	  helping kernel same-page merging in virtualised environments.
+	  Depending on your workload it will greatly reduce performance.
+
+	  If unsure, say N.
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 4d5ce6e..c29e3a0 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -795,6 +795,12 @@ static bool free_pages_prepare(struct page *page, unsigned int order)
 		debug_check_no_obj_freed(page_address(page),
 					   PAGE_SIZE << order);
 	}
+
+#ifdef CONFIG_SANITIZE_FREED_PAGES
+	for (i = 0; i < (1 << order); i++)
+		clear_highpage(page + i);
+#endif
+
 	arch_free_page(page, order);
 	kernel_map_pages(page, 1 << order, 0);
 
@@ -960,9 +966,15 @@ static int prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
 	kernel_map_pages(page, 1 << order, 1);
 	kasan_alloc_pages(page, order);
 
+#ifndef CONFIG_SANITIZE_FREED_PAGES
+	/* SANITIZE_FREED_PAGES relies implicitly on the fact that pages are
+	 * cleared before use, so we don't need gfp zero in the default case
+	 * because all pages go through the free_pages_prepare code path when
+	 * switching from bootmem to the default allocator */
 	if (gfp_flags & __GFP_ZERO)
 		for (i = 0; i < (1 << order); i++)
 			clear_highpage(page + i);
+#endif
 
 	if (order && (gfp_flags & __GFP_COMP))
 		prep_compound_page(page, order);
-- 
1.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 3/4] PM / Hibernate: fix SANITIZE_FREED_PAGES
  2015-05-04 21:16 ` Anisse Astier
@ 2015-05-04 21:16   ` Anisse Astier
  -1 siblings, 0 replies; 24+ messages in thread
From: Anisse Astier @ 2015-05-04 21:16 UTC (permalink / raw)
  Cc: Anisse Astier, Andrew Morton, Mel Gorman, Kirill A. Shutemov,
	David Rientjes, Alan Cox, Linus Torvalds, Peter Zijlstra,
	PaX Team, Brad Spengler, Kees Cook, Andi Kleen, linux-mm,
	linux-kernel

SANITIZE_FREED_PAGES feature relies on having all pages going through
the free_pages_prepare path in order to be cleared before being used. In
the hibernate use case, pages will automagically appear in the system
without being cleared.

This fix will make sure free pages are cleared on resume.

Signed-off-by: Anisse Astier <anisse@astier.eu>
---
 kernel/power/hibernate.c |  7 ++++++-
 kernel/power/power.h     |  4 ++++
 kernel/power/snapshot.c  | 21 +++++++++++++++++++++
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 2329daa..3193b9a 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -305,9 +305,14 @@ static int create_image(int platform_mode)
 			error);
 	/* Restore control flow magically appears here */
 	restore_processor_state();
-	if (!in_suspend)
+	if (!in_suspend) {
 		events_check_enabled = false;
 
+#ifdef CONFIG_SANITIZE_FREED_PAGES
+		clear_free_pages();
+		printk(KERN_INFO "PM: free pages cleared after restore\n");
+#endif
+	}
 	platform_leave(platform_mode);
 
  Power_up:
diff --git a/kernel/power/power.h b/kernel/power/power.h
index ce9b832..26b2101 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -92,6 +92,10 @@ extern int create_basic_memory_bitmaps(void);
 extern void free_basic_memory_bitmaps(void);
 extern int hibernate_preallocate_memory(void);
 
+#ifdef CONFIG_SANITIZE_FREED_PAGES
+extern void clear_free_pages(void);
+#endif
+
 /**
  *	Auxiliary structure used for reading the snapshot image data and
  *	metadata from and writing them to the list of page backup entries
diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 5235dd4..673ade1 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -1032,6 +1032,27 @@ void free_basic_memory_bitmaps(void)
 	pr_debug("PM: Basic memory bitmaps freed\n");
 }
 
+#ifdef CONFIG_SANITIZE_FREED_PAGES
+void clear_free_pages(void)
+{
+	struct memory_bitmap *bm = free_pages_map;
+	unsigned long pfn;
+
+	if (WARN_ON(!(free_pages_map)))
+		return;
+
+	memory_bm_position_reset(bm);
+	pfn = memory_bm_next_pfn(bm);
+	while (pfn != BM_END_OF_MAP) {
+		if (pfn_valid(pfn))
+			clear_highpage(pfn_to_page(pfn));
+
+		pfn = memory_bm_next_pfn(bm);
+	}
+	memory_bm_position_reset(bm);
+}
+#endif /* SANITIZE_FREED_PAGES */
+
 /**
  *	snapshot_additional_pages - estimate the number of additional pages
  *	be needed for setting up the suspend image data structures for given
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 3/4] PM / Hibernate: fix SANITIZE_FREED_PAGES
@ 2015-05-04 21:16   ` Anisse Astier
  0 siblings, 0 replies; 24+ messages in thread
From: Anisse Astier @ 2015-05-04 21:16 UTC (permalink / raw)
  Cc: Anisse Astier, Andrew Morton, Mel Gorman, Kirill A. Shutemov,
	David Rientjes, Alan Cox, Linus Torvalds, Peter Zijlstra,
	PaX Team, Brad Spengler, Kees Cook, Andi Kleen, linux-mm,
	linux-kernel

SANITIZE_FREED_PAGES feature relies on having all pages going through
the free_pages_prepare path in order to be cleared before being used. In
the hibernate use case, pages will automagically appear in the system
without being cleared.

This fix will make sure free pages are cleared on resume.

Signed-off-by: Anisse Astier <anisse@astier.eu>
---
 kernel/power/hibernate.c |  7 ++++++-
 kernel/power/power.h     |  4 ++++
 kernel/power/snapshot.c  | 21 +++++++++++++++++++++
 3 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 2329daa..3193b9a 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -305,9 +305,14 @@ static int create_image(int platform_mode)
 			error);
 	/* Restore control flow magically appears here */
 	restore_processor_state();
-	if (!in_suspend)
+	if (!in_suspend) {
 		events_check_enabled = false;
 
+#ifdef CONFIG_SANITIZE_FREED_PAGES
+		clear_free_pages();
+		printk(KERN_INFO "PM: free pages cleared after restore\n");
+#endif
+	}
 	platform_leave(platform_mode);
 
  Power_up:
diff --git a/kernel/power/power.h b/kernel/power/power.h
index ce9b832..26b2101 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -92,6 +92,10 @@ extern int create_basic_memory_bitmaps(void);
 extern void free_basic_memory_bitmaps(void);
 extern int hibernate_preallocate_memory(void);
 
+#ifdef CONFIG_SANITIZE_FREED_PAGES
+extern void clear_free_pages(void);
+#endif
+
 /**
  *	Auxiliary structure used for reading the snapshot image data and
  *	metadata from and writing them to the list of page backup entries
diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 5235dd4..673ade1 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -1032,6 +1032,27 @@ void free_basic_memory_bitmaps(void)
 	pr_debug("PM: Basic memory bitmaps freed\n");
 }
 
+#ifdef CONFIG_SANITIZE_FREED_PAGES
+void clear_free_pages(void)
+{
+	struct memory_bitmap *bm = free_pages_map;
+	unsigned long pfn;
+
+	if (WARN_ON(!(free_pages_map)))
+		return;
+
+	memory_bm_position_reset(bm);
+	pfn = memory_bm_next_pfn(bm);
+	while (pfn != BM_END_OF_MAP) {
+		if (pfn_valid(pfn))
+			clear_highpage(pfn_to_page(pfn));
+
+		pfn = memory_bm_next_pfn(bm);
+	}
+	memory_bm_position_reset(bm);
+}
+#endif /* SANITIZE_FREED_PAGES */
+
 /**
  *	snapshot_additional_pages - estimate the number of additional pages
  *	be needed for setting up the suspend image data structures for given
-- 
1.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 4/4] mm: Add debug code for SANITIZE_FREED_PAGES
  2015-05-04 21:16 ` Anisse Astier
@ 2015-05-04 21:16   ` Anisse Astier
  -1 siblings, 0 replies; 24+ messages in thread
From: Anisse Astier @ 2015-05-04 21:16 UTC (permalink / raw)
  Cc: Anisse Astier, Andrew Morton, Mel Gorman, Kirill A. Shutemov,
	David Rientjes, Alan Cox, Linus Torvalds, Peter Zijlstra,
	PaX Team, Brad Spengler, Kees Cook, Andi Kleen, linux-mm,
	linux-kernel

Add debug code for sanitize freed pages to print status and verify pages
at alloc to make sure they're clean. It can be useful if you have
crashes when using SANITIZE_FREED_PAGES.

Signed-off-by: Anisse Astier <anisse@astier.eu>
---
 kernel/power/snapshot.c |  8 ++++++--
 mm/Kconfig              | 10 ++++++++++
 mm/page_alloc.c         | 25 +++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 673ade1..dfbfb5f 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -1044,9 +1044,13 @@ void clear_free_pages(void)
 	memory_bm_position_reset(bm);
 	pfn = memory_bm_next_pfn(bm);
 	while (pfn != BM_END_OF_MAP) {
-		if (pfn_valid(pfn))
+		if (pfn_valid(pfn)) {
+#ifdef CONFIG_SANITIZE_FREED_PAGES_DEBUG
+			printk(KERN_INFO "Clearing page %p\n",
+					page_address(pfn_to_page(pfn)));
+#endif
 			clear_highpage(pfn_to_page(pfn));
-
+		}
 		pfn = memory_bm_next_pfn(bm);
 	}
 	memory_bm_position_reset(bm);
diff --git a/mm/Kconfig b/mm/Kconfig
index e9fb3bd..95364f2 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -647,3 +647,13 @@ config SANITIZE_FREED_PAGES
 	  Depending on your workload it will greatly reduce performance.
 
 	  If unsure, say N.
+
+config SANITIZE_FREED_PAGES_DEBUG
+	bool "Debug sanitize pages feature"
+	default n
+	depends on SANITIZE_FREED_PAGES && DEBUG_KERNEL
+	help
+	  This option adds some debugging code for the SANITIZE_FREED_PAGES
+	  option, as well as verification code to ensure pages are really
+	  zeroed. Don't enable unless you want to debug this feature.
+	  If unsure, say N.
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index c29e3a0..ba8aa25 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -975,6 +975,31 @@ static int prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
 		for (i = 0; i < (1 << order); i++)
 			clear_highpage(page + i);
 #endif
+#ifdef CONFIG_SANITIZE_FREED_PAGES_DEBUG
+	for (i = 0; i < (1 << order); i++) {
+		struct page *p = page + i;
+		int j;
+		bool err = false;
+		void *kaddr = kmap_atomic(p);
+
+		for (j = 0; j < PAGE_SIZE; j++) {
+			if (((char *)kaddr)[j] != 0) {
+				pr_err("page %p is not zero on alloc! %s\n",
+						page_address(p), (gfp_flags &
+							__GFP_ZERO) ?
+						"fixing." : "");
+				if (gfp_flags & __GFP_ZERO) {
+					err = true;
+					kunmap_atomic(kaddr);
+					clear_highpage(p);
+				}
+				break;
+			}
+		}
+		if (!err)
+			kunmap_atomic(kaddr);
+	}
+#endif
 
 	if (order && (gfp_flags & __GFP_COMP))
 		prep_compound_page(page, order);
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v2 4/4] mm: Add debug code for SANITIZE_FREED_PAGES
@ 2015-05-04 21:16   ` Anisse Astier
  0 siblings, 0 replies; 24+ messages in thread
From: Anisse Astier @ 2015-05-04 21:16 UTC (permalink / raw)
  Cc: Anisse Astier, Andrew Morton, Mel Gorman, Kirill A. Shutemov,
	David Rientjes, Alan Cox, Linus Torvalds, Peter Zijlstra,
	PaX Team, Brad Spengler, Kees Cook, Andi Kleen, linux-mm,
	linux-kernel

Add debug code for sanitize freed pages to print status and verify pages
at alloc to make sure they're clean. It can be useful if you have
crashes when using SANITIZE_FREED_PAGES.

Signed-off-by: Anisse Astier <anisse@astier.eu>
---
 kernel/power/snapshot.c |  8 ++++++--
 mm/Kconfig              | 10 ++++++++++
 mm/page_alloc.c         | 25 +++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 673ade1..dfbfb5f 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -1044,9 +1044,13 @@ void clear_free_pages(void)
 	memory_bm_position_reset(bm);
 	pfn = memory_bm_next_pfn(bm);
 	while (pfn != BM_END_OF_MAP) {
-		if (pfn_valid(pfn))
+		if (pfn_valid(pfn)) {
+#ifdef CONFIG_SANITIZE_FREED_PAGES_DEBUG
+			printk(KERN_INFO "Clearing page %p\n",
+					page_address(pfn_to_page(pfn)));
+#endif
 			clear_highpage(pfn_to_page(pfn));
-
+		}
 		pfn = memory_bm_next_pfn(bm);
 	}
 	memory_bm_position_reset(bm);
diff --git a/mm/Kconfig b/mm/Kconfig
index e9fb3bd..95364f2 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -647,3 +647,13 @@ config SANITIZE_FREED_PAGES
 	  Depending on your workload it will greatly reduce performance.
 
 	  If unsure, say N.
+
+config SANITIZE_FREED_PAGES_DEBUG
+	bool "Debug sanitize pages feature"
+	default n
+	depends on SANITIZE_FREED_PAGES && DEBUG_KERNEL
+	help
+	  This option adds some debugging code for the SANITIZE_FREED_PAGES
+	  option, as well as verification code to ensure pages are really
+	  zeroed. Don't enable unless you want to debug this feature.
+	  If unsure, say N.
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index c29e3a0..ba8aa25 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -975,6 +975,31 @@ static int prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
 		for (i = 0; i < (1 << order); i++)
 			clear_highpage(page + i);
 #endif
+#ifdef CONFIG_SANITIZE_FREED_PAGES_DEBUG
+	for (i = 0; i < (1 << order); i++) {
+		struct page *p = page + i;
+		int j;
+		bool err = false;
+		void *kaddr = kmap_atomic(p);
+
+		for (j = 0; j < PAGE_SIZE; j++) {
+			if (((char *)kaddr)[j] != 0) {
+				pr_err("page %p is not zero on alloc! %s\n",
+						page_address(p), (gfp_flags &
+							__GFP_ZERO) ?
+						"fixing." : "");
+				if (gfp_flags & __GFP_ZERO) {
+					err = true;
+					kunmap_atomic(kaddr);
+					clear_highpage(p);
+				}
+				break;
+			}
+		}
+		if (!err)
+			kunmap_atomic(kaddr);
+	}
+#endif
 
 	if (order && (gfp_flags & __GFP_COMP))
 		prep_compound_page(page, order);
-- 
1.9.3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 3/4] PM / Hibernate: fix SANITIZE_FREED_PAGES
  2015-05-04 21:16   ` Anisse Astier
@ 2015-05-04 21:50     ` PaX Team
  -1 siblings, 0 replies; 24+ messages in thread
From: PaX Team @ 2015-05-04 21:50 UTC (permalink / raw)
  To: Anisse Astier
  Cc: Andrew Morton, Mel Gorman, Kirill A. Shutemov, David Rientjes,
	Alan Cox, Linus Torvalds, Peter Zijlstra, Brad Spengler,
	Kees Cook, Andi Kleen, linux-mm, linux-kernel, Rafael J. Wysocki,
	Pavel Machek

On 4 May 2015 at 23:16, Anisse Astier wrote:

> SANITIZE_FREED_PAGES feature relies on having all pages going through
> the free_pages_prepare path in order to be cleared before being used. In
> the hibernate use case, pages will automagically appear in the system
> without being cleared.

is this based on debugging/code reading/discussions with hibernation folks
(i see none of them on CC, added them now) or is it just a brute force attempt
to fix the symptoms? if the former, it'd be nice to share some more details
and have Acks from the code owners.

> This fix will make sure free pages are cleared on resume.
> 
> Signed-off-by: Anisse Astier <anisse@astier.eu>
> ---
>  kernel/power/hibernate.c |  7 ++++++-
>  kernel/power/power.h     |  4 ++++
>  kernel/power/snapshot.c  | 21 +++++++++++++++++++++
>  3 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 2329daa..3193b9a 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -305,9 +305,14 @@ static int create_image(int platform_mode)
>  			error);
>  	/* Restore control flow magically appears here */
>  	restore_processor_state();
> -	if (!in_suspend)
> +	if (!in_suspend) {
>  		events_check_enabled = false;
>  
> +#ifdef CONFIG_SANITIZE_FREED_PAGES
> +		clear_free_pages();
> +		printk(KERN_INFO "PM: free pages cleared after restore\n");
> +#endif
> +	}
>  	platform_leave(platform_mode);
>  
>   Power_up:
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index ce9b832..26b2101 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -92,6 +92,10 @@ extern int create_basic_memory_bitmaps(void);
>  extern void free_basic_memory_bitmaps(void);
>  extern int hibernate_preallocate_memory(void);
>  
> +#ifdef CONFIG_SANITIZE_FREED_PAGES
> +extern void clear_free_pages(void);
> +#endif
> +
>  /**
>   *	Auxiliary structure used for reading the snapshot image data and
>   *	metadata from and writing them to the list of page backup entries
> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> index 5235dd4..673ade1 100644
> --- a/kernel/power/snapshot.c
> +++ b/kernel/power/snapshot.c
> @@ -1032,6 +1032,27 @@ void free_basic_memory_bitmaps(void)
>  	pr_debug("PM: Basic memory bitmaps freed\n");
>  }
>  
> +#ifdef CONFIG_SANITIZE_FREED_PAGES
> +void clear_free_pages(void)
> +{
> +	struct memory_bitmap *bm = free_pages_map;
> +	unsigned long pfn;
> +
> +	if (WARN_ON(!(free_pages_map)))
> +		return;
> +
> +	memory_bm_position_reset(bm);
> +	pfn = memory_bm_next_pfn(bm);
> +	while (pfn != BM_END_OF_MAP) {
> +		if (pfn_valid(pfn))
> +			clear_highpage(pfn_to_page(pfn));
> +
> +		pfn = memory_bm_next_pfn(bm);
> +	}
> +	memory_bm_position_reset(bm);
> +}
> +#endif /* SANITIZE_FREED_PAGES */
> +
>  /**
>   *	snapshot_additional_pages - estimate the number of additional pages
>   *	be needed for setting up the suspend image data structures for given
> -- 
> 1.9.3
> 
> 




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 3/4] PM / Hibernate: fix SANITIZE_FREED_PAGES
@ 2015-05-04 21:50     ` PaX Team
  0 siblings, 0 replies; 24+ messages in thread
From: PaX Team @ 2015-05-04 21:50 UTC (permalink / raw)
  To: Anisse Astier
  Cc: Andrew Morton, Mel Gorman, Kirill A. Shutemov, David Rientjes,
	Alan Cox, Linus Torvalds, Peter Zijlstra, Brad Spengler,
	Kees Cook, Andi Kleen, linux-mm, linux-kernel, Rafael J. Wysocki,
	Pavel Machek

On 4 May 2015 at 23:16, Anisse Astier wrote:

> SANITIZE_FREED_PAGES feature relies on having all pages going through
> the free_pages_prepare path in order to be cleared before being used. In
> the hibernate use case, pages will automagically appear in the system
> without being cleared.

is this based on debugging/code reading/discussions with hibernation folks
(i see none of them on CC, added them now) or is it just a brute force attempt
to fix the symptoms? if the former, it'd be nice to share some more details
and have Acks from the code owners.

> This fix will make sure free pages are cleared on resume.
> 
> Signed-off-by: Anisse Astier <anisse@astier.eu>
> ---
>  kernel/power/hibernate.c |  7 ++++++-
>  kernel/power/power.h     |  4 ++++
>  kernel/power/snapshot.c  | 21 +++++++++++++++++++++
>  3 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index 2329daa..3193b9a 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -305,9 +305,14 @@ static int create_image(int platform_mode)
>  			error);
>  	/* Restore control flow magically appears here */
>  	restore_processor_state();
> -	if (!in_suspend)
> +	if (!in_suspend) {
>  		events_check_enabled = false;
>  
> +#ifdef CONFIG_SANITIZE_FREED_PAGES
> +		clear_free_pages();
> +		printk(KERN_INFO "PM: free pages cleared after restore\n");
> +#endif
> +	}
>  	platform_leave(platform_mode);
>  
>   Power_up:
> diff --git a/kernel/power/power.h b/kernel/power/power.h
> index ce9b832..26b2101 100644
> --- a/kernel/power/power.h
> +++ b/kernel/power/power.h
> @@ -92,6 +92,10 @@ extern int create_basic_memory_bitmaps(void);
>  extern void free_basic_memory_bitmaps(void);
>  extern int hibernate_preallocate_memory(void);
>  
> +#ifdef CONFIG_SANITIZE_FREED_PAGES
> +extern void clear_free_pages(void);
> +#endif
> +
>  /**
>   *	Auxiliary structure used for reading the snapshot image data and
>   *	metadata from and writing them to the list of page backup entries
> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> index 5235dd4..673ade1 100644
> --- a/kernel/power/snapshot.c
> +++ b/kernel/power/snapshot.c
> @@ -1032,6 +1032,27 @@ void free_basic_memory_bitmaps(void)
>  	pr_debug("PM: Basic memory bitmaps freed\n");
>  }
>  
> +#ifdef CONFIG_SANITIZE_FREED_PAGES
> +void clear_free_pages(void)
> +{
> +	struct memory_bitmap *bm = free_pages_map;
> +	unsigned long pfn;
> +
> +	if (WARN_ON(!(free_pages_map)))
> +		return;
> +
> +	memory_bm_position_reset(bm);
> +	pfn = memory_bm_next_pfn(bm);
> +	while (pfn != BM_END_OF_MAP) {
> +		if (pfn_valid(pfn))
> +			clear_highpage(pfn_to_page(pfn));
> +
> +		pfn = memory_bm_next_pfn(bm);
> +	}
> +	memory_bm_position_reset(bm);
> +}
> +#endif /* SANITIZE_FREED_PAGES */
> +
>  /**
>   *	snapshot_additional_pages - estimate the number of additional pages
>   *	be needed for setting up the suspend image data structures for given
> -- 
> 1.9.3
> 
> 



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 4/4] mm: Add debug code for SANITIZE_FREED_PAGES
  2015-05-04 21:16   ` Anisse Astier
@ 2015-05-04 21:50     ` PaX Team
  -1 siblings, 0 replies; 24+ messages in thread
From: PaX Team @ 2015-05-04 21:50 UTC (permalink / raw)
  To: Anisse Astier
  Cc: Anisse Astier, Andrew Morton, Mel Gorman, Kirill A. Shutemov,
	David Rientjes, Alan Cox, Linus Torvalds, Peter Zijlstra,
	Brad Spengler, Kees Cook, Andi Kleen, linux-mm, linux-kernel

On 4 May 2015 at 23:16, Anisse Astier wrote:

> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index c29e3a0..ba8aa25 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -975,6 +975,31 @@ static int prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
>  		for (i = 0; i < (1 << order); i++)
>  			clear_highpage(page + i);
>  #endif
> +#ifdef CONFIG_SANITIZE_FREED_PAGES_DEBUG
> +	for (i = 0; i < (1 << order); i++) {
> +		struct page *p = page + i;
> +		int j;
> +		bool err = false;
> +		void *kaddr = kmap_atomic(p);
> +
> +		for (j = 0; j < PAGE_SIZE; j++) {

did you mean to use memchr_inv(kaddr, 0, PAGE_SIZE) instead? ;)

> +			if (((char *)kaddr)[j] != 0) {
> +				pr_err("page %p is not zero on alloc! %s\n",
> +						page_address(p), (gfp_flags &
> +							__GFP_ZERO) ?
> +						"fixing." : "");
> +				if (gfp_flags & __GFP_ZERO) {
> +					err = true;
> +					kunmap_atomic(kaddr);
> +					clear_highpage(p);
> +				}
> +				break;
> +			}
> +		}
> +		if (!err)
> +			kunmap_atomic(kaddr);
> +	}
> +#endif
>  
>  	if (order && (gfp_flags & __GFP_COMP))
>  		prep_compound_page(page, order);
> -- 
> 1.9.3
> 
> 




^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/4] mm/page_alloc.c: add config option to sanitize freed pages
  2015-05-04 21:16   ` Anisse Astier
@ 2015-05-04 21:50     ` PaX Team
  -1 siblings, 0 replies; 24+ messages in thread
From: PaX Team @ 2015-05-04 21:50 UTC (permalink / raw)
  To: Anisse Astier
  Cc: Andrew Morton, Mel Gorman, Kirill A. Shutemov, David Rientjes,
	Alan Cox, Linus Torvalds, Peter Zijlstra, Brad Spengler,
	Kees Cook, Andi Kleen, linux-mm, linux-kernel

On 4 May 2015 at 23:16, Anisse Astier wrote:

> @@ -960,9 +966,15 @@ static int prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
>  	kernel_map_pages(page, 1 << order, 1);
>  	kasan_alloc_pages(page, order);
>  
> +#ifndef CONFIG_SANITIZE_FREED_PAGES
> +	/* SANITIZE_FREED_PAGES relies implicitly on the fact that pages are
> +	 * cleared before use, so we don't need gfp zero in the default case
> +	 * because all pages go through the free_pages_prepare code path when
> +	 * switching from bootmem to the default allocator */
>  	if (gfp_flags & __GFP_ZERO)
>  		for (i = 0; i < (1 << order); i++)
>  			clear_highpage(page + i);
> +#endif

this hunk should not be applied before the hibernation fix otherwise
bisect will break.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 4/4] mm: Add debug code for SANITIZE_FREED_PAGES
@ 2015-05-04 21:50     ` PaX Team
  0 siblings, 0 replies; 24+ messages in thread
From: PaX Team @ 2015-05-04 21:50 UTC (permalink / raw)
  To: Anisse Astier
  Cc: Andrew Morton, Mel Gorman, Kirill A. Shutemov, David Rientjes,
	Alan Cox, Linus Torvalds, Peter Zijlstra, Brad Spengler,
	Kees Cook, Andi Kleen, linux-mm, linux-kernel

On 4 May 2015 at 23:16, Anisse Astier wrote:

> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index c29e3a0..ba8aa25 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -975,6 +975,31 @@ static int prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
>  		for (i = 0; i < (1 << order); i++)
>  			clear_highpage(page + i);
>  #endif
> +#ifdef CONFIG_SANITIZE_FREED_PAGES_DEBUG
> +	for (i = 0; i < (1 << order); i++) {
> +		struct page *p = page + i;
> +		int j;
> +		bool err = false;
> +		void *kaddr = kmap_atomic(p);
> +
> +		for (j = 0; j < PAGE_SIZE; j++) {

did you mean to use memchr_inv(kaddr, 0, PAGE_SIZE) instead? ;)

> +			if (((char *)kaddr)[j] != 0) {
> +				pr_err("page %p is not zero on alloc! %s\n",
> +						page_address(p), (gfp_flags &
> +							__GFP_ZERO) ?
> +						"fixing." : "");
> +				if (gfp_flags & __GFP_ZERO) {
> +					err = true;
> +					kunmap_atomic(kaddr);
> +					clear_highpage(p);
> +				}
> +				break;
> +			}
> +		}
> +		if (!err)
> +			kunmap_atomic(kaddr);
> +	}
> +#endif
>  
>  	if (order && (gfp_flags & __GFP_COMP))
>  		prep_compound_page(page, order);
> -- 
> 1.9.3
> 
> 



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/4] mm/page_alloc.c: add config option to sanitize freed pages
@ 2015-05-04 21:50     ` PaX Team
  0 siblings, 0 replies; 24+ messages in thread
From: PaX Team @ 2015-05-04 21:50 UTC (permalink / raw)
  To: Anisse Astier
  Cc: Andrew Morton, Mel Gorman, Kirill A. Shutemov, David Rientjes,
	Alan Cox, Linus Torvalds, Peter Zijlstra, Brad Spengler,
	Kees Cook, Andi Kleen, linux-mm, linux-kernel

On 4 May 2015 at 23:16, Anisse Astier wrote:

> @@ -960,9 +966,15 @@ static int prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
>  	kernel_map_pages(page, 1 << order, 1);
>  	kasan_alloc_pages(page, order);
>  
> +#ifndef CONFIG_SANITIZE_FREED_PAGES
> +	/* SANITIZE_FREED_PAGES relies implicitly on the fact that pages are
> +	 * cleared before use, so we don't need gfp zero in the default case
> +	 * because all pages go through the free_pages_prepare code path when
> +	 * switching from bootmem to the default allocator */
>  	if (gfp_flags & __GFP_ZERO)
>  		for (i = 0; i < (1 << order); i++)
>  			clear_highpage(page + i);
> +#endif

this hunk should not be applied before the hibernation fix otherwise
bisect will break.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 3/4] PM / Hibernate: fix SANITIZE_FREED_PAGES
  2015-05-04 21:50     ` PaX Team
@ 2015-05-04 22:29       ` Rafael J. Wysocki
  -1 siblings, 0 replies; 24+ messages in thread
From: Rafael J. Wysocki @ 2015-05-04 22:29 UTC (permalink / raw)
  To: pageexec
  Cc: Anisse Astier, Andrew Morton, Mel Gorman, Kirill A. Shutemov,
	David Rientjes, Alan Cox, Linus Torvalds, Peter Zijlstra,
	Brad Spengler, Kees Cook, Andi Kleen, linux-mm, linux-kernel,
	Pavel Machek, Linux PM list

On Monday, May 04, 2015 11:50:13 PM PaX Team wrote:
> On 4 May 2015 at 23:16, Anisse Astier wrote:
> 
> > SANITIZE_FREED_PAGES feature relies on having all pages going through
> > the free_pages_prepare path in order to be cleared before being used. In
> > the hibernate use case, pages will automagically appear in the system
> > without being cleared.
> 
> is this based on debugging/code reading/discussions with hibernation folks
> (i see none of them on CC, added them now) or is it just a brute force attempt
> to fix the symptoms? if the former, it'd be nice to share some more details
> and have Acks from the code owners.

I haven't seen it, for one, and I'm wondering why the "clearing" cannot be done
at the swsusp_free() time?

In any case, please CC hibernation-related patches and discussions thereof to
linux-pm@vger.kernel.org.

Thanks!


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 3/4] PM / Hibernate: fix SANITIZE_FREED_PAGES
@ 2015-05-04 22:29       ` Rafael J. Wysocki
  0 siblings, 0 replies; 24+ messages in thread
From: Rafael J. Wysocki @ 2015-05-04 22:29 UTC (permalink / raw)
  To: pageexec
  Cc: Anisse Astier, Andrew Morton, Mel Gorman, Kirill A. Shutemov,
	David Rientjes, Alan Cox, Linus Torvalds, Peter Zijlstra,
	Brad Spengler, Kees Cook, Andi Kleen, linux-mm, linux-kernel,
	Pavel Machek, Linux PM list

On Monday, May 04, 2015 11:50:13 PM PaX Team wrote:
> On 4 May 2015 at 23:16, Anisse Astier wrote:
> 
> > SANITIZE_FREED_PAGES feature relies on having all pages going through
> > the free_pages_prepare path in order to be cleared before being used. In
> > the hibernate use case, pages will automagically appear in the system
> > without being cleared.
> 
> is this based on debugging/code reading/discussions with hibernation folks
> (i see none of them on CC, added them now) or is it just a brute force attempt
> to fix the symptoms? if the former, it'd be nice to share some more details
> and have Acks from the code owners.

I haven't seen it, for one, and I'm wondering why the "clearing" cannot be done
at the swsusp_free() time?

In any case, please CC hibernation-related patches and discussions thereof to
linux-pm@vger.kernel.org.

Thanks!

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 3/4] PM / Hibernate: fix SANITIZE_FREED_PAGES
  2015-05-04 22:29       ` Rafael J. Wysocki
@ 2015-05-05 12:41         ` Anisse Astier
  -1 siblings, 0 replies; 24+ messages in thread
From: Anisse Astier @ 2015-05-05 12:41 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: PaX Team, Andrew Morton, Mel Gorman, Kirill A. Shutemov,
	David Rientjes, Alan Cox, Linus Torvalds, Peter Zijlstra,
	Brad Spengler, Kees Cook, Andi Kleen, linux-mm,
	Linux Kernel Mailing List, Pavel Machek, Linux PM list

On Tue, May 5, 2015 at 12:29 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> I haven't seen it, for one, and I'm wondering why the "clearing" cannot be done
> at the swsusp_free() time?

Because the validity of the free pages bitmap is short-lived since
device resume code might do some allocations.

>
> In any case, please CC hibernation-related patches and discussions thereof to
> linux-pm@vger.kernel.org.

Thanks, I had to forget something when sending this series :-/ ; I'm
preparing v3 that will be sent to linux-pm too.

Regards,

Anisse

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 3/4] PM / Hibernate: fix SANITIZE_FREED_PAGES
@ 2015-05-05 12:41         ` Anisse Astier
  0 siblings, 0 replies; 24+ messages in thread
From: Anisse Astier @ 2015-05-05 12:41 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: PaX Team, Andrew Morton, Mel Gorman, Kirill A. Shutemov,
	David Rientjes, Alan Cox, Linus Torvalds, Peter Zijlstra,
	Brad Spengler, Kees Cook, Andi Kleen, linux-mm,
	Linux Kernel Mailing List, Pavel Machek, Linux PM list

On Tue, May 5, 2015 at 12:29 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> I haven't seen it, for one, and I'm wondering why the "clearing" cannot be done
> at the swsusp_free() time?

Because the validity of the free pages bitmap is short-lived since
device resume code might do some allocations.

>
> In any case, please CC hibernation-related patches and discussions thereof to
> linux-pm@vger.kernel.org.

Thanks, I had to forget something when sending this series :-/ ; I'm
preparing v3 that will be sent to linux-pm too.

Regards,

Anisse

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/4] mm/page_alloc.c: add config option to sanitize freed pages
  2015-05-04 21:50     ` PaX Team
@ 2015-05-05 12:42       ` Anisse Astier
  -1 siblings, 0 replies; 24+ messages in thread
From: Anisse Astier @ 2015-05-05 12:42 UTC (permalink / raw)
  To: PaX Team
  Cc: Andrew Morton, Mel Gorman, Kirill A. Shutemov, David Rientjes,
	Alan Cox, Linus Torvalds, Peter Zijlstra, Brad Spengler,
	Kees Cook, Andi Kleen, linux-mm, Linux Kernel Mailing List

On Mon, May 4, 2015 at 11:50 PM, PaX Team <pageexec@freemail.hu> wrote:
> On 4 May 2015 at 23:16, Anisse Astier wrote:
>
>> @@ -960,9 +966,15 @@ static int prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
>>       kernel_map_pages(page, 1 << order, 1);
>>       kasan_alloc_pages(page, order);
>>
>> +#ifndef CONFIG_SANITIZE_FREED_PAGES
>> +     /* SANITIZE_FREED_PAGES relies implicitly on the fact that pages are
>> +      * cleared before use, so we don't need gfp zero in the default case
>> +      * because all pages go through the free_pages_prepare code path when
>> +      * switching from bootmem to the default allocator */
>>       if (gfp_flags & __GFP_ZERO)
>>               for (i = 0; i < (1 << order); i++)
>>                       clear_highpage(page + i);
>> +#endif
>
> this hunk should not be applied before the hibernation fix otherwise
> bisect will break.
>

It will be re-ordered in v3, thanks.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 2/4] mm/page_alloc.c: add config option to sanitize freed pages
@ 2015-05-05 12:42       ` Anisse Astier
  0 siblings, 0 replies; 24+ messages in thread
From: Anisse Astier @ 2015-05-05 12:42 UTC (permalink / raw)
  To: PaX Team
  Cc: Andrew Morton, Mel Gorman, Kirill A. Shutemov, David Rientjes,
	Alan Cox, Linus Torvalds, Peter Zijlstra, Brad Spengler,
	Kees Cook, Andi Kleen, linux-mm, Linux Kernel Mailing List

On Mon, May 4, 2015 at 11:50 PM, PaX Team <pageexec@freemail.hu> wrote:
> On 4 May 2015 at 23:16, Anisse Astier wrote:
>
>> @@ -960,9 +966,15 @@ static int prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
>>       kernel_map_pages(page, 1 << order, 1);
>>       kasan_alloc_pages(page, order);
>>
>> +#ifndef CONFIG_SANITIZE_FREED_PAGES
>> +     /* SANITIZE_FREED_PAGES relies implicitly on the fact that pages are
>> +      * cleared before use, so we don't need gfp zero in the default case
>> +      * because all pages go through the free_pages_prepare code path when
>> +      * switching from bootmem to the default allocator */
>>       if (gfp_flags & __GFP_ZERO)
>>               for (i = 0; i < (1 << order); i++)
>>                       clear_highpage(page + i);
>> +#endif
>
> this hunk should not be applied before the hibernation fix otherwise
> bisect will break.
>

It will be re-ordered in v3, thanks.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 4/4] mm: Add debug code for SANITIZE_FREED_PAGES
  2015-05-04 21:50     ` PaX Team
@ 2015-05-05 12:43       ` Anisse Astier
  -1 siblings, 0 replies; 24+ messages in thread
From: Anisse Astier @ 2015-05-05 12:43 UTC (permalink / raw)
  To: PaX Team
  Cc: Andrew Morton, Mel Gorman, Kirill A. Shutemov, David Rientjes,
	Alan Cox, Linus Torvalds, Peter Zijlstra, Brad Spengler,
	Kees Cook, Andi Kleen, linux-mm, Linux Kernel Mailing List

On Mon, May 4, 2015 at 11:50 PM, PaX Team <pageexec@freemail.hu> wrote:
> On 4 May 2015 at 23:16, Anisse Astier wrote:
>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index c29e3a0..ba8aa25 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -975,6 +975,31 @@ static int prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
>>               for (i = 0; i < (1 << order); i++)
>>                       clear_highpage(page + i);
>>  #endif
>> +#ifdef CONFIG_SANITIZE_FREED_PAGES_DEBUG
>> +     for (i = 0; i < (1 << order); i++) {
>> +             struct page *p = page + i;
>> +             int j;
>> +             bool err = false;
>> +             void *kaddr = kmap_atomic(p);
>> +
>> +             for (j = 0; j < PAGE_SIZE; j++) {
>
> did you mean to use memchr_inv(kaddr, 0, PAGE_SIZE) instead? ;)

Will be fixed in v3, although as I said I'm not sure if this debug
code should go in or not.

Thanks for you time.

Anisse

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v2 4/4] mm: Add debug code for SANITIZE_FREED_PAGES
@ 2015-05-05 12:43       ` Anisse Astier
  0 siblings, 0 replies; 24+ messages in thread
From: Anisse Astier @ 2015-05-05 12:43 UTC (permalink / raw)
  To: PaX Team
  Cc: Andrew Morton, Mel Gorman, Kirill A. Shutemov, David Rientjes,
	Alan Cox, Linus Torvalds, Peter Zijlstra, Brad Spengler,
	Kees Cook, Andi Kleen, linux-mm, Linux Kernel Mailing List

On Mon, May 4, 2015 at 11:50 PM, PaX Team <pageexec@freemail.hu> wrote:
> On 4 May 2015 at 23:16, Anisse Astier wrote:
>
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index c29e3a0..ba8aa25 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -975,6 +975,31 @@ static int prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags,
>>               for (i = 0; i < (1 << order); i++)
>>                       clear_highpage(page + i);
>>  #endif
>> +#ifdef CONFIG_SANITIZE_FREED_PAGES_DEBUG
>> +     for (i = 0; i < (1 << order); i++) {
>> +             struct page *p = page + i;
>> +             int j;
>> +             bool err = false;
>> +             void *kaddr = kmap_atomic(p);
>> +
>> +             for (j = 0; j < PAGE_SIZE; j++) {
>
> did you mean to use memchr_inv(kaddr, 0, PAGE_SIZE) instead? ;)

Will be fixed in v3, although as I said I'm not sure if this debug
code should go in or not.

Thanks for you time.

Anisse

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2015-05-05 12:43 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-05-04 21:16 [PATCH v2 0/4] Sanitizing freed pages Anisse Astier
2015-05-04 21:16 ` Anisse Astier
2015-05-04 21:16 ` [PATCH v2 1/4] mm/page_alloc.c: cleanup obsolete KM_USER* Anisse Astier
2015-05-04 21:16   ` Anisse Astier
2015-05-04 21:16 ` [PATCH v2 2/4] mm/page_alloc.c: add config option to sanitize freed pages Anisse Astier
2015-05-04 21:16   ` Anisse Astier
2015-05-04 21:50   ` PaX Team
2015-05-04 21:50     ` PaX Team
2015-05-05 12:42     ` Anisse Astier
2015-05-05 12:42       ` Anisse Astier
2015-05-04 21:16 ` [PATCH v2 3/4] PM / Hibernate: fix SANITIZE_FREED_PAGES Anisse Astier
2015-05-04 21:16   ` Anisse Astier
2015-05-04 21:50   ` PaX Team
2015-05-04 21:50     ` PaX Team
2015-05-04 22:29     ` Rafael J. Wysocki
2015-05-04 22:29       ` Rafael J. Wysocki
2015-05-05 12:41       ` Anisse Astier
2015-05-05 12:41         ` Anisse Astier
2015-05-04 21:16 ` [PATCH v2 4/4] mm: Add debug code for SANITIZE_FREED_PAGES Anisse Astier
2015-05-04 21:16   ` Anisse Astier
2015-05-04 21:50   ` PaX Team
2015-05-04 21:50     ` PaX Team
2015-05-05 12:43     ` Anisse Astier
2015-05-05 12:43       ` Anisse Astier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.