linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] kasan: detect invalid frees
@ 2017-12-27 12:44 Dmitry Vyukov
  2017-12-27 12:44 ` [PATCH 1/5] kasan: detect invalid frees for large objects Dmitry Vyukov
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Dmitry Vyukov @ 2017-12-27 12:44 UTC (permalink / raw)
  To: akpm, aryabinin; +Cc: linux-mm, linux-kernel, kasan-dev, Dmitry Vyukov

KASAN detects double-frees, but does not detect invalid-frees
(when a pointer into a middle of heap object is passed to free).
We recently had a very unpleasant case in crypto code which freed
an inner object inside of a heap allocation. This left unnoticed
during free, but totally corrupted heap and later lead to a bunch
of random crashes all over kernel code.

Detect invalid frees.

Dmitry Vyukov (5):
  kasan: detect invalid frees for large objects
  kasan: don't use __builtin_return_address(1)
  kasan: detect invalid frees for large mempool objects
  kasan: unify code between kasan_slab_free() and kasan_poison_kfree()
  kasan: detect invalid frees

 include/linux/kasan.h | 13 ++++----
 lib/test_kasan.c      | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++
 mm/kasan/kasan.c      | 57 +++++++++++++++++++----------------
 mm/kasan/kasan.h      |  3 +-
 mm/kasan/report.c     |  5 ++--
 mm/mempool.c          |  6 ++--
 mm/slab.c             |  6 ++--
 mm/slub.c             | 10 +++----
 8 files changed, 135 insertions(+), 48 deletions(-)

-- 
2.15.1.620.gb9897f4670-goog

--
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] 6+ messages in thread

* [PATCH 1/5] kasan: detect invalid frees for large objects
  2017-12-27 12:44 [PATCH 0/5] kasan: detect invalid frees Dmitry Vyukov
@ 2017-12-27 12:44 ` Dmitry Vyukov
  2017-12-27 12:44 ` [PATCH 2/5] kasan: don't use __builtin_return_address(1) Dmitry Vyukov
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Dmitry Vyukov @ 2017-12-27 12:44 UTC (permalink / raw)
  To: akpm, aryabinin; +Cc: linux-mm, linux-kernel, kasan-dev, Dmitry Vyukov

Detect frees of pointers into middle of large heap objects.

I dropped const from kasan_kfree_large() because it starts propagating
through a bunch of functions in kasan_report.c, slab/slub nearest_obj(),
all of their local variables, fixup_red_left(), etc.

Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Cc: kasan-dev@googlegroups.com
---
 include/linux/kasan.h |  4 ++--
 lib/test_kasan.c      | 33 +++++++++++++++++++++++++++++++++
 mm/kasan/kasan.c      | 12 +++++-------
 mm/kasan/kasan.h      |  3 +--
 mm/kasan/report.c     |  3 +--
 mm/slub.c             |  4 ++--
 6 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index e3eb834c9a35..fc9e642533f8 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -56,7 +56,7 @@ void kasan_poison_object_data(struct kmem_cache *cache, void *object);
 void kasan_init_slab_obj(struct kmem_cache *cache, const void *object);
 
 void kasan_kmalloc_large(const void *ptr, size_t size, gfp_t flags);
-void kasan_kfree_large(const void *ptr);
+void kasan_kfree_large(void *ptr);
 void kasan_poison_kfree(void *ptr);
 void kasan_kmalloc(struct kmem_cache *s, const void *object, size_t size,
 		  gfp_t flags);
@@ -108,7 +108,7 @@ static inline void kasan_init_slab_obj(struct kmem_cache *cache,
 				const void *object) {}
 
 static inline void kasan_kmalloc_large(void *ptr, size_t size, gfp_t flags) {}
-static inline void kasan_kfree_large(const void *ptr) {}
+static inline void kasan_kfree_large(void *ptr) {}
 static inline void kasan_poison_kfree(void *ptr) {}
 static inline void kasan_kmalloc(struct kmem_cache *s, const void *object,
 				size_t size, gfp_t flags) {}
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 2724f86c4cef..e9c5d765be66 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -94,6 +94,37 @@ static noinline void __init kmalloc_pagealloc_oob_right(void)
 	ptr[size] = 0;
 	kfree(ptr);
 }
+
+static noinline void __init kmalloc_pagealloc_uaf(void)
+{
+	char *ptr;
+	size_t size = KMALLOC_MAX_CACHE_SIZE + 10;
+
+	pr_info("kmalloc pagealloc allocation: use-after-free\n");
+	ptr = kmalloc(size, GFP_KERNEL);
+	if (!ptr) {
+		pr_err("Allocation failed\n");
+		return;
+	}
+
+	kfree(ptr);
+	ptr[0] = 0;
+}
+
+static noinline void __init kmalloc_pagealloc_invalid_free(void)
+{
+	char *ptr;
+	size_t size = KMALLOC_MAX_CACHE_SIZE + 10;
+
+	pr_info("kmalloc pagealloc allocation: invalid-free\n");
+	ptr = kmalloc(size, GFP_KERNEL);
+	if (!ptr) {
+		pr_err("Allocation failed\n");
+		return;
+	}
+
+	kfree(ptr + 1);
+}
 #endif
 
 static noinline void __init kmalloc_large_oob_right(void)
@@ -505,6 +536,8 @@ static int __init kmalloc_tests_init(void)
 	kmalloc_node_oob_right();
 #ifdef CONFIG_SLUB
 	kmalloc_pagealloc_oob_right();
+	kmalloc_pagealloc_uaf();
+	kmalloc_pagealloc_invalid_free();
 #endif
 	kmalloc_large_oob_right();
 	kmalloc_oob_krealloc_more();
diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
index 8aaee42fcfab..ecb64fda79e6 100644
--- a/mm/kasan/kasan.c
+++ b/mm/kasan/kasan.c
@@ -511,8 +511,7 @@ bool kasan_slab_free(struct kmem_cache *cache, void *object)
 
 	shadow_byte = READ_ONCE(*(s8 *)kasan_mem_to_shadow(object));
 	if (shadow_byte < 0 || shadow_byte >= KASAN_SHADOW_SCALE_SIZE) {
-		kasan_report_double_free(cache, object,
-				__builtin_return_address(1));
+		kasan_report_invalid_free(object, __builtin_return_address(1));
 		return true;
 	}
 
@@ -602,12 +601,11 @@ void kasan_poison_kfree(void *ptr)
 		kasan_poison_slab_free(page->slab_cache, ptr);
 }
 
-void kasan_kfree_large(const void *ptr)
+void kasan_kfree_large(void *ptr)
 {
-	struct page *page = virt_to_page(ptr);
-
-	kasan_poison_shadow(ptr, PAGE_SIZE << compound_order(page),
-			KASAN_FREE_PAGE);
+	if (ptr != page_address(virt_to_head_page(ptr)))
+		kasan_report_invalid_free(ptr, __builtin_return_address(1));
+	/* The object will be poisoned by page_alloc. */
 }
 
 int kasan_module_alloc(void *addr, size_t size)
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 7c0bcd1f4c0d..57f517d1dfce 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -107,8 +107,7 @@ static inline const void *kasan_shadow_to_mem(const void *shadow_addr)
 
 void kasan_report(unsigned long addr, size_t size,
 		bool is_write, unsigned long ip);
-void kasan_report_double_free(struct kmem_cache *cache, void *object,
-					void *ip);
+void kasan_report_invalid_free(void *object, void *ip);
 
 #if defined(CONFIG_SLAB) || defined(CONFIG_SLUB)
 void quarantine_put(struct kasan_free_meta *info, struct kmem_cache *cache);
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index eff12e040498..55916ad21722 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -326,8 +326,7 @@ static void print_shadow_for_address(const void *addr)
 	}
 }
 
-void kasan_report_double_free(struct kmem_cache *cache, void *object,
-				void *ip)
+void kasan_report_invalid_free(void *object, void *ip)
 {
 	unsigned long flags;
 
diff --git a/mm/slub.c b/mm/slub.c
index 3530b3c60ad6..67c8cee43cf6 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1354,7 +1354,7 @@ static inline void kmalloc_large_node_hook(void *ptr, size_t size, gfp_t flags)
 	kasan_kmalloc_large(ptr, size, flags);
 }
 
-static inline void kfree_hook(const void *x)
+static inline void kfree_hook(void *x)
 {
 	kmemleak_free(x);
 	kasan_kfree_large(x);
@@ -3911,7 +3911,7 @@ void kfree(const void *x)
 	page = virt_to_head_page(x);
 	if (unlikely(!PageSlab(page))) {
 		BUG_ON(!PageCompound(page));
-		kfree_hook(x);
+		kfree_hook(object);
 		__free_pages(page, compound_order(page));
 		return;
 	}
-- 
2.15.1.620.gb9897f4670-goog

--
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] 6+ messages in thread

* [PATCH 2/5] kasan: don't use __builtin_return_address(1)
  2017-12-27 12:44 [PATCH 0/5] kasan: detect invalid frees Dmitry Vyukov
  2017-12-27 12:44 ` [PATCH 1/5] kasan: detect invalid frees for large objects Dmitry Vyukov
@ 2017-12-27 12:44 ` Dmitry Vyukov
  2017-12-27 12:44 ` [PATCH 3/5] kasan: detect invalid frees for large mempool objects Dmitry Vyukov
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Dmitry Vyukov @ 2017-12-27 12:44 UTC (permalink / raw)
  To: akpm, aryabinin; +Cc: linux-mm, linux-kernel, kasan-dev, Dmitry Vyukov

__builtin_return_address(1) is unreliable without frame pointers.
With defconfig on kmalloc_pagealloc_invalid_free test I am getting:

BUG: KASAN: double-free or invalid-free in           (null)

Pass caller PC from callers explicitly.

Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Cc: kasan-dev@googlegroups.com
---
 include/linux/kasan.h | 9 +++++----
 mm/kasan/kasan.c      | 8 ++++----
 mm/kasan/kasan.h      | 2 +-
 mm/kasan/report.c     | 4 ++--
 mm/slab.c             | 6 +++---
 mm/slub.c             | 8 ++++----
 6 files changed, 19 insertions(+), 18 deletions(-)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index fc9e642533f8..f0d13c30acc6 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -56,14 +56,14 @@ void kasan_poison_object_data(struct kmem_cache *cache, void *object);
 void kasan_init_slab_obj(struct kmem_cache *cache, const void *object);
 
 void kasan_kmalloc_large(const void *ptr, size_t size, gfp_t flags);
-void kasan_kfree_large(void *ptr);
+void kasan_kfree_large(void *ptr, unsigned long ip);
 void kasan_poison_kfree(void *ptr);
 void kasan_kmalloc(struct kmem_cache *s, const void *object, size_t size,
 		  gfp_t flags);
 void kasan_krealloc(const void *object, size_t new_size, gfp_t flags);
 
 void kasan_slab_alloc(struct kmem_cache *s, void *object, gfp_t flags);
-bool kasan_slab_free(struct kmem_cache *s, void *object);
+bool kasan_slab_free(struct kmem_cache *s, void *object, unsigned long ip);
 
 struct kasan_cache {
 	int alloc_meta_offset;
@@ -108,7 +108,7 @@ static inline void kasan_init_slab_obj(struct kmem_cache *cache,
 				const void *object) {}
 
 static inline void kasan_kmalloc_large(void *ptr, size_t size, gfp_t flags) {}
-static inline void kasan_kfree_large(void *ptr) {}
+static inline void kasan_kfree_large(void *ptr, unsigned long ip) {}
 static inline void kasan_poison_kfree(void *ptr) {}
 static inline void kasan_kmalloc(struct kmem_cache *s, const void *object,
 				size_t size, gfp_t flags) {}
@@ -117,7 +117,8 @@ static inline void kasan_krealloc(const void *object, size_t new_size,
 
 static inline void kasan_slab_alloc(struct kmem_cache *s, void *object,
 				   gfp_t flags) {}
-static inline bool kasan_slab_free(struct kmem_cache *s, void *object)
+static inline bool kasan_slab_free(struct kmem_cache *s, void *object,
+				   unsigned long ip)
 {
 	return false;
 }
diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
index ecb64fda79e6..32f555ded938 100644
--- a/mm/kasan/kasan.c
+++ b/mm/kasan/kasan.c
@@ -501,7 +501,7 @@ static void kasan_poison_slab_free(struct kmem_cache *cache, void *object)
 	kasan_poison_shadow(object, rounded_up_size, KASAN_KMALLOC_FREE);
 }
 
-bool kasan_slab_free(struct kmem_cache *cache, void *object)
+bool kasan_slab_free(struct kmem_cache *cache, void *object, unsigned long ip)
 {
 	s8 shadow_byte;
 
@@ -511,7 +511,7 @@ bool kasan_slab_free(struct kmem_cache *cache, void *object)
 
 	shadow_byte = READ_ONCE(*(s8 *)kasan_mem_to_shadow(object));
 	if (shadow_byte < 0 || shadow_byte >= KASAN_SHADOW_SCALE_SIZE) {
-		kasan_report_invalid_free(object, __builtin_return_address(1));
+		kasan_report_invalid_free(object, ip);
 		return true;
 	}
 
@@ -601,10 +601,10 @@ void kasan_poison_kfree(void *ptr)
 		kasan_poison_slab_free(page->slab_cache, ptr);
 }
 
-void kasan_kfree_large(void *ptr)
+void kasan_kfree_large(void *ptr, unsigned long ip)
 {
 	if (ptr != page_address(virt_to_head_page(ptr)))
-		kasan_report_invalid_free(ptr, __builtin_return_address(1));
+		kasan_report_invalid_free(ptr, ip);
 	/* The object will be poisoned by page_alloc. */
 }
 
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 57f517d1dfce..2792de927fcd 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -107,7 +107,7 @@ static inline const void *kasan_shadow_to_mem(const void *shadow_addr)
 
 void kasan_report(unsigned long addr, size_t size,
 		bool is_write, unsigned long ip);
-void kasan_report_invalid_free(void *object, void *ip);
+void kasan_report_invalid_free(void *object, unsigned long ip);
 
 #if defined(CONFIG_SLAB) || defined(CONFIG_SLUB)
 void quarantine_put(struct kasan_free_meta *info, struct kmem_cache *cache);
diff --git a/mm/kasan/report.c b/mm/kasan/report.c
index 55916ad21722..75206991ece0 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -326,12 +326,12 @@ static void print_shadow_for_address(const void *addr)
 	}
 }
 
-void kasan_report_invalid_free(void *object, void *ip)
+void kasan_report_invalid_free(void *object, unsigned long ip)
 {
 	unsigned long flags;
 
 	kasan_start_report(&flags);
-	pr_err("BUG: KASAN: double-free or invalid-free in %pS\n", ip);
+	pr_err("BUG: KASAN: double-free or invalid-free in %pS\n", (void *)ip);
 	pr_err("\n");
 	print_address_description(object);
 	pr_err("\n");
diff --git a/mm/slab.c b/mm/slab.c
index 6bc4e609e24b..d074fd0790f4 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -3478,11 +3478,11 @@ static void cache_flusharray(struct kmem_cache *cachep, struct array_cache *ac)
  * Release an obj back to its cache. If the obj has a constructed state, it must
  * be in this state _before_ it is released.  Called with disabled ints.
  */
-static inline void __cache_free(struct kmem_cache *cachep, void *objp,
-				unsigned long caller)
+static __always_inline void __cache_free(struct kmem_cache *cachep, void *objp,
+					 unsigned long caller)
 {
 	/* Put the object into the quarantine, don't touch it for now. */
-	if (kasan_slab_free(cachep, objp))
+	if (kasan_slab_free(cachep, objp, _RET_IP_))
 		return;
 
 	___cache_free(cachep, objp, caller);
diff --git a/mm/slub.c b/mm/slub.c
index 67c8cee43cf6..b1e41572c6cb 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1354,13 +1354,13 @@ static inline void kmalloc_large_node_hook(void *ptr, size_t size, gfp_t flags)
 	kasan_kmalloc_large(ptr, size, flags);
 }
 
-static inline void kfree_hook(void *x)
+static __always_inline void kfree_hook(void *x)
 {
 	kmemleak_free(x);
-	kasan_kfree_large(x);
+	kasan_kfree_large(x, _RET_IP_);
 }
 
-static inline void *slab_free_hook(struct kmem_cache *s, void *x)
+static __always_inline void *slab_free_hook(struct kmem_cache *s, void *x)
 {
 	void *freeptr;
 
@@ -1388,7 +1388,7 @@ static inline void *slab_free_hook(struct kmem_cache *s, void *x)
 	 * kasan_slab_free() may put x into memory quarantine, delaying its
 	 * reuse. In this case the object's freelist pointer is changed.
 	 */
-	kasan_slab_free(s, x);
+	kasan_slab_free(s, x, _RET_IP_);
 	return freeptr;
 }
 
-- 
2.15.1.620.gb9897f4670-goog

--
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] 6+ messages in thread

* [PATCH 3/5] kasan: detect invalid frees for large mempool objects
  2017-12-27 12:44 [PATCH 0/5] kasan: detect invalid frees Dmitry Vyukov
  2017-12-27 12:44 ` [PATCH 1/5] kasan: detect invalid frees for large objects Dmitry Vyukov
  2017-12-27 12:44 ` [PATCH 2/5] kasan: don't use __builtin_return_address(1) Dmitry Vyukov
@ 2017-12-27 12:44 ` Dmitry Vyukov
  2017-12-27 12:44 ` [PATCH 4/5] kasan: unify code between kasan_slab_free() and kasan_poison_kfree() Dmitry Vyukov
  2017-12-27 12:44 ` [PATCH 5/5] kasan: detect invalid frees Dmitry Vyukov
  4 siblings, 0 replies; 6+ messages in thread
From: Dmitry Vyukov @ 2017-12-27 12:44 UTC (permalink / raw)
  To: akpm, aryabinin; +Cc: linux-mm, linux-kernel, kasan-dev, Dmitry Vyukov

Detect frees of pointers into middle of mempool objects.

I did a one-off test, but it turned out to be very tricky,
so I reverted it. First, mempool does not call kasan_poison_kfree()
unless allocation function fails. I stubbed an allocation function
to fail on second and subsequent allocations. But then mempool stopped
to call kasan_poison_kfree() at all, because it does it only when
allocation function is mempool_kmalloc(). We could support this
special failing test allocation function in mempool, but it also
can't live with kasan tests, because these are in a module.

Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Cc: kasan-dev@googlegroups.com
---
 include/linux/kasan.h |  4 ++--
 mm/kasan/kasan.c      | 11 ++++++++---
 mm/mempool.c          |  6 +++---
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/include/linux/kasan.h b/include/linux/kasan.h
index f0d13c30acc6..fc45f8952d1e 100644
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -57,7 +57,7 @@ void kasan_init_slab_obj(struct kmem_cache *cache, const void *object);
 
 void kasan_kmalloc_large(const void *ptr, size_t size, gfp_t flags);
 void kasan_kfree_large(void *ptr, unsigned long ip);
-void kasan_poison_kfree(void *ptr);
+void kasan_poison_kfree(void *ptr, unsigned long ip);
 void kasan_kmalloc(struct kmem_cache *s, const void *object, size_t size,
 		  gfp_t flags);
 void kasan_krealloc(const void *object, size_t new_size, gfp_t flags);
@@ -109,7 +109,7 @@ static inline void kasan_init_slab_obj(struct kmem_cache *cache,
 
 static inline void kasan_kmalloc_large(void *ptr, size_t size, gfp_t flags) {}
 static inline void kasan_kfree_large(void *ptr, unsigned long ip) {}
-static inline void kasan_poison_kfree(void *ptr) {}
+static inline void kasan_poison_kfree(void *ptr, unsigned long ip) {}
 static inline void kasan_kmalloc(struct kmem_cache *s, const void *object,
 				size_t size, gfp_t flags) {}
 static inline void kasan_krealloc(const void *object, size_t new_size,
diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
index 32f555ded938..77c103748728 100644
--- a/mm/kasan/kasan.c
+++ b/mm/kasan/kasan.c
@@ -588,17 +588,22 @@ void kasan_krealloc(const void *object, size_t size, gfp_t flags)
 		kasan_kmalloc(page->slab_cache, object, size, flags);
 }
 
-void kasan_poison_kfree(void *ptr)
+void kasan_poison_kfree(void *ptr, unsigned long ip)
 {
 	struct page *page;
 
 	page = virt_to_head_page(ptr);
 
-	if (unlikely(!PageSlab(page)))
+	if (unlikely(!PageSlab(page))) {
+		if (ptr != page_address(page)) {
+			kasan_report_invalid_free(ptr, ip);
+			return;
+		}
 		kasan_poison_shadow(ptr, PAGE_SIZE << compound_order(page),
 				KASAN_FREE_PAGE);
-	else
+	} else {
 		kasan_poison_slab_free(page->slab_cache, ptr);
+	}
 }
 
 void kasan_kfree_large(void *ptr, unsigned long ip)
diff --git a/mm/mempool.c b/mm/mempool.c
index 7d8c5a0010a2..5c9dce34719b 100644
--- a/mm/mempool.c
+++ b/mm/mempool.c
@@ -103,10 +103,10 @@ static inline void poison_element(mempool_t *pool, void *element)
 }
 #endif /* CONFIG_DEBUG_SLAB || CONFIG_SLUB_DEBUG_ON */
 
-static void kasan_poison_element(mempool_t *pool, void *element)
+static __always_inline void kasan_poison_element(mempool_t *pool, void *element)
 {
 	if (pool->alloc == mempool_alloc_slab || pool->alloc == mempool_kmalloc)
-		kasan_poison_kfree(element);
+		kasan_poison_kfree(element, _RET_IP_);
 	if (pool->alloc == mempool_alloc_pages)
 		kasan_free_pages(element, (unsigned long)pool->pool_data);
 }
@@ -119,7 +119,7 @@ static void kasan_unpoison_element(mempool_t *pool, void *element, gfp_t flags)
 		kasan_alloc_pages(element, (unsigned long)pool->pool_data);
 }
 
-static void add_element(mempool_t *pool, void *element)
+static __always_inline void add_element(mempool_t *pool, void *element)
 {
 	BUG_ON(pool->curr_nr >= pool->min_nr);
 	poison_element(pool, element);
-- 
2.15.1.620.gb9897f4670-goog

--
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] 6+ messages in thread

* [PATCH 4/5] kasan: unify code between kasan_slab_free() and kasan_poison_kfree()
  2017-12-27 12:44 [PATCH 0/5] kasan: detect invalid frees Dmitry Vyukov
                   ` (2 preceding siblings ...)
  2017-12-27 12:44 ` [PATCH 3/5] kasan: detect invalid frees for large mempool objects Dmitry Vyukov
@ 2017-12-27 12:44 ` Dmitry Vyukov
  2017-12-27 12:44 ` [PATCH 5/5] kasan: detect invalid frees Dmitry Vyukov
  4 siblings, 0 replies; 6+ messages in thread
From: Dmitry Vyukov @ 2017-12-27 12:44 UTC (permalink / raw)
  To: akpm, aryabinin; +Cc: linux-mm, linux-kernel, kasan-dev, Dmitry Vyukov

Both of these functions deal with freeing of slab objects.
However, kasan_poison_kfree() mishandles SLAB_TYPESAFE_BY_RCU
(must also not poison such objects) and does not detect double-frees.

Unify code between these functions.
This solves both of the problems and allows to add more common code
(e.g. detection of invalid frees).

Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Cc: kasan-dev@googlegroups.com
---
 mm/kasan/kasan.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
index 77c103748728..578843fab5dc 100644
--- a/mm/kasan/kasan.c
+++ b/mm/kasan/kasan.c
@@ -489,21 +489,11 @@ void kasan_slab_alloc(struct kmem_cache *cache, void *object, gfp_t flags)
 	kasan_kmalloc(cache, object, cache->object_size, flags);
 }
 
-static void kasan_poison_slab_free(struct kmem_cache *cache, void *object)
-{
-	unsigned long size = cache->object_size;
-	unsigned long rounded_up_size = round_up(size, KASAN_SHADOW_SCALE_SIZE);
-
-	/* RCU slabs could be legally used after free within the RCU period */
-	if (unlikely(cache->flags & SLAB_TYPESAFE_BY_RCU))
-		return;
-
-	kasan_poison_shadow(object, rounded_up_size, KASAN_KMALLOC_FREE);
-}
-
-bool kasan_slab_free(struct kmem_cache *cache, void *object, unsigned long ip)
+static bool __kasan_slab_free(struct kmem_cache *cache, void *object,
+			      unsigned long ip, bool quarantine)
 {
 	s8 shadow_byte;
+	unsigned long rounded_up_size;
 
 	/* RCU slabs could be legally used after free within the RCU period */
 	if (unlikely(cache->flags & SLAB_TYPESAFE_BY_RCU))
@@ -515,9 +505,10 @@ bool kasan_slab_free(struct kmem_cache *cache, void *object, unsigned long ip)
 		return true;
 	}
 
-	kasan_poison_slab_free(cache, object);
+	rounded_up_size = round_up(cache->object_size, KASAN_SHADOW_SCALE_SIZE);
+	kasan_poison_shadow(object, rounded_up_size, KASAN_KMALLOC_FREE);
 
-	if (unlikely(!(cache->flags & SLAB_KASAN)))
+	if (!quarantine || unlikely(!(cache->flags & SLAB_KASAN)))
 		return false;
 
 	set_track(&get_alloc_info(cache, object)->free_track, GFP_NOWAIT);
@@ -525,6 +516,11 @@ bool kasan_slab_free(struct kmem_cache *cache, void *object, unsigned long ip)
 	return true;
 }
 
+bool kasan_slab_free(struct kmem_cache *cache, void *object, unsigned long ip)
+{
+	return __kasan_slab_free(cache, object, ip, true);
+}
+
 void kasan_kmalloc(struct kmem_cache *cache, const void *object, size_t size,
 		   gfp_t flags)
 {
@@ -602,7 +598,7 @@ void kasan_poison_kfree(void *ptr, unsigned long ip)
 		kasan_poison_shadow(ptr, PAGE_SIZE << compound_order(page),
 				KASAN_FREE_PAGE);
 	} else {
-		kasan_poison_slab_free(page->slab_cache, ptr);
+		__kasan_slab_free(page->slab_cache, ptr, ip, false);
 	}
 }
 
-- 
2.15.1.620.gb9897f4670-goog

--
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] 6+ messages in thread

* [PATCH 5/5] kasan: detect invalid frees
  2017-12-27 12:44 [PATCH 0/5] kasan: detect invalid frees Dmitry Vyukov
                   ` (3 preceding siblings ...)
  2017-12-27 12:44 ` [PATCH 4/5] kasan: unify code between kasan_slab_free() and kasan_poison_kfree() Dmitry Vyukov
@ 2017-12-27 12:44 ` Dmitry Vyukov
  4 siblings, 0 replies; 6+ messages in thread
From: Dmitry Vyukov @ 2017-12-27 12:44 UTC (permalink / raw)
  To: akpm, aryabinin; +Cc: linux-mm, linux-kernel, kasan-dev, Dmitry Vyukov

Detect frees of pointers into middle of heap objects.

Signed-off-by: Dmitry Vyukov <dvyukov@google.com>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Cc: kasan-dev@googlegroups.com
---
 lib/test_kasan.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 mm/kasan/kasan.c |  6 ++++++
 2 files changed, 56 insertions(+)

diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index e9c5d765be66..a808d81b409d 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -523,6 +523,54 @@ static noinline void __init kasan_alloca_oob_right(void)
 	*(volatile char *)p;
 }
 
+static noinline void __init kmem_cache_double_free(void)
+{
+	char *p;
+	size_t size = 200;
+	struct kmem_cache *cache;
+
+	cache = kmem_cache_create("test_cache", size, 0, 0, NULL);
+	if (!cache) {
+		pr_err("Cache allocation failed\n");
+		return;
+	}
+	pr_info("double-free on heap object\n");
+	p = kmem_cache_alloc(cache, GFP_KERNEL);
+	if (!p) {
+		pr_err("Allocation failed\n");
+		kmem_cache_destroy(cache);
+		return;
+	}
+
+	kmem_cache_free(cache, p);
+	kmem_cache_free(cache, p);
+	kmem_cache_destroy(cache);
+}
+
+static noinline void __init kmem_cache_invalid_free(void)
+{
+	char *p;
+	size_t size = 200;
+	struct kmem_cache *cache;
+
+	cache = kmem_cache_create("test_cache", size, 0, SLAB_TYPESAFE_BY_RCU,
+				  NULL);
+	if (!cache) {
+		pr_err("Cache allocation failed\n");
+		return;
+	}
+	pr_info("invalid-free of heap object\n");
+	p = kmem_cache_alloc(cache, GFP_KERNEL);
+	if (!p) {
+		pr_err("Allocation failed\n");
+		kmem_cache_destroy(cache);
+		return;
+	}
+
+	kmem_cache_free(cache, p + 1);
+	kmem_cache_destroy(cache);
+}
+
 static int __init kmalloc_tests_init(void)
 {
 	/*
@@ -560,6 +608,8 @@ static int __init kmalloc_tests_init(void)
 	ksize_unpoisons_memory();
 	copy_user_test();
 	use_after_scope_test();
+	kmem_cache_double_free();
+	kmem_cache_invalid_free();
 
 	kasan_restore_multi_shot(multishot);
 
diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
index 578843fab5dc..3fb497d4fbf8 100644
--- a/mm/kasan/kasan.c
+++ b/mm/kasan/kasan.c
@@ -495,6 +495,12 @@ static bool __kasan_slab_free(struct kmem_cache *cache, void *object,
 	s8 shadow_byte;
 	unsigned long rounded_up_size;
 
+	if (unlikely(nearest_obj(cache, virt_to_head_page(object), object) !=
+	    object)) {
+		kasan_report_invalid_free(object, ip);
+		return true;
+	}
+
 	/* RCU slabs could be legally used after free within the RCU period */
 	if (unlikely(cache->flags & SLAB_TYPESAFE_BY_RCU))
 		return false;
-- 
2.15.1.620.gb9897f4670-goog

--
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] 6+ messages in thread

end of thread, other threads:[~2017-12-27 12:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-27 12:44 [PATCH 0/5] kasan: detect invalid frees Dmitry Vyukov
2017-12-27 12:44 ` [PATCH 1/5] kasan: detect invalid frees for large objects Dmitry Vyukov
2017-12-27 12:44 ` [PATCH 2/5] kasan: don't use __builtin_return_address(1) Dmitry Vyukov
2017-12-27 12:44 ` [PATCH 3/5] kasan: detect invalid frees for large mempool objects Dmitry Vyukov
2017-12-27 12:44 ` [PATCH 4/5] kasan: unify code between kasan_slab_free() and kasan_poison_kfree() Dmitry Vyukov
2017-12-27 12:44 ` [PATCH 5/5] kasan: detect invalid frees Dmitry Vyukov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).