mm: kasan: don't touch metadata in kasan_[un]poison_element()

To avoid draining the mempools, KASAN shouldn't put the mempool elements into the quarantine upon mempool_free().

It shouldn't store
allocation/deallocation stacks upon mempool_alloc()/mempool_free() either.

Therefore make kasan_[un]poison_element() just change the shadow memory, not the metadata.

Signed-off-by Alexander Potapenko <[email protected]>

Reported-by Kuthonuzo Luruo <[email protected]>

---

mm/kasan/kasan.c +28 -0

+		kasan_unpoison_shadow(object, cache->object_size);
+	else
+		kasan_kmalloc(cache, object, cache->object_size, flags);
 }
 
 void kasan_poison_slab_free(struct kmem_cache *cache, void *object)

@@ -611,6 +615,31 @@ void kasan_kmalloc_large(const void *ptr, size_t size, gfp_t flags)
 		KASAN_PAGE_REDZONE);
 }
 
+void kasan_unpoison_kmalloc(const void *object, size_t size, gfp_t flags)
+{
+	struct page *page;
+	unsigned long redzone_start;
+	unsigned long redzone_end;
+
+	if (unlikely(object == ZERO_SIZE_PTR) || (object == NULL))
+		return;
+
+	page = virt_to_head_page(object);
+	redzone_start = round_up((unsigned long)(object + size),
+				KASAN_SHADOW_SCALE_SIZE);
+
+	if (unlikely(!PageSlab(page)))
+		redzone_end = (unsigned long)object +
+			(PAGE_SIZE << compound_order(page));
+	else
+		redzone_end = round_up(
+			(unsigned long)object + page->slab_cache->object_size,
+			KASAN_SHADOW_SCALE_SIZE);
+	kasan_unpoison_shadow(object, size);
+	kasan_poison_shadow((void *)redzone_start, redzone_end - redzone_start,
+		KASAN_KMALLOC_REDZONE);
+}
+
 void kasan_krealloc(const void *object, size_t size, gfp_t flags)
 {
 	struct page *page;

mm/kasan/kasan.c +10 -0

+{
+	struct page *page;
+
+	page = virt_to_head_page(ptr);
+
+	if (unlikely(!PageSlab(page)))
+		kasan_poison_shadow(ptr, PAGE_SIZE << compound_order(page),
+				KASAN_FREE_PAGE);
+	else
+		kasan_poison_slab_free(page->slab_cache, ptr);
 }
 
 void kasan_kfree_large(const void *ptr)

mm/mempool.c +0 -0

index 9e075f8..bcd48c6 100644

Quoted 2 lines · Alexander Potapenko

To avoid draining the mempools, KASAN shouldn't put the mempool elements into the quarantine upon mempool_free().

Correct, but unfortunately this patch doesn't fix that.

Quoted 8 lines · Andrey Ryabinin

Quoted 2 lines · Alexander Potapenko

To avoid draining the mempools, KASAN shouldn't put the mempool elements into the quarantine upon mempool_free().

Correct, but unfortunately this patch doesn't fix that.

So, I made this:

From: Andrey Ryabinin <[email protected]>
Subject: [PATCH] mm: mempool: kasan: don't poot mempool objects in quarantine

Currently we may put reserved by mempool elements into quarantine via kasan_kfree(). This is totally wrong since quarantine may really free these objects. So when mempool will try to use such element, use-after-free will happen. Or mempool may decide that it no longer need that element and double-free it.

So don't put object into quarantine in kasan_kfree(), just poison it. Rename kasan_kfree() to kasan_poison_kfree() to respect that.

Also, we shouldn't use kasan_slab_alloc()/kasan_krealloc() in kasan_unpoison_element() because those functions may update allocation stacktrace. This would be wrong for the most of the remove_element call sites.

(The only call site where we may want to update alloc stacktrace is
in mempool_alloc(). Kmemleak solves this by calling kmemleak_update_trace(), so we could make something like that too. But this is out of scope of this patch).

Fixes 55834c59098d ("mm: kasan: initial memory quarantine implementation")

Signed-off-by Andrey Ryabinin <[email protected]>

Reported-by Kuthonuzo Luruo <[email protected]>

---
 include/linux/kasan.h | 11 +++++++----
 mm/kasan/kasan.c      |  6 +++---
 mm/mempool.c          | 12 ++++--------
 3 files changed, 14 insertions(+), 15 deletions(-)

include/linux/kasan.h +7 -4

index 611927f..ac4b3c4 100644
@@ -59,14 +59,13 @@ void kasan_poison_object_data(struct kmem_cache *cache, 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(void *ptr);
+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);
-void kasan_poison_slab_free(struct kmem_cache *s, void *object);
 
 struct kasan_cache {
        int alloc_meta_offset;

@@ -76,6 +75,9 @@ struct kasan_cache {
 int kasan_module_alloc(void *addr, size_t size);
 void kasan_free_shadow(const struct vm_struct *vm);
 
+size_t ksize(const void *);
+static inline void kasan_unpoison_slab(const void *ptr) { ksize(ptr); }
+
 #else /* CONFIG_KASAN */
 
 static inline void kasan_unpoison_shadow(const void *address, size_t size) {}

@@ -102,7 +104,7 @@ static inline void kasan_poison_object_data(struct kmem_cache *cache,
 
 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(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) {}
 static inline void kasan_krealloc(const void *object, size_t new_size,

@@ -114,11 +116,12 @@ static inline bool kasan_slab_free(struct kmem_cache *s, void *object)
 {
        return false;
 }
-static inline void kasan_poison_slab_free(struct kmem_cache *s, void *object) {}
 
 static inline int kasan_module_alloc(void *addr, size_t size) { return 0; }
 static inline void kasan_free_shadow(const struct vm_struct *vm) {}
 
+static inline void kasan_unpoison_slab(const void *ptr) { }
+
 #endif /* CONFIG_KASAN */
 
 #endif /* LINUX_KASAN_H */

mm/kasan/kasan.c +3 -3

index 28439ac..6845f92 100644
@@ -508,7 +508,7 @@ void kasan_slab_alloc(struct kmem_cache *cache, void *object, gfp_t flags)
        kasan_kmalloc(cache, object, cache->object_size, flags);
 }
 
-void kasan_poison_slab_free(struct kmem_cache *cache, void *object)
+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);

@@ -626,7 +626,7 @@ void kasan_krealloc(const void *object, size_t size, gfp_t flags)
                kasan_kmalloc(page->slab_cache, object, size, flags);
 }
 
-void kasan_kfree(void *ptr)
+void kasan_poison_kfree(void *ptr)
 {
        struct page *page;
 

@@ -636,7 +636,7 @@ void kasan_kfree(void *ptr)
                kasan_poison_shadow(ptr, PAGE_SIZE << compound_order(page),
                                KASAN_FREE_PAGE);
        else
-               kasan_slab_free(page->slab_cache, ptr);
+               kasan_poison_slab_free(page->slab_cache, ptr);
 }
 
 void kasan_kfree_large(const void *ptr)

mm/mempool.c +4 -9

index 9e075f8..8f65464 100644
@@ -104,20 +104,16 @@ static inline void poison_element(mempool_t *pool, void *element)
 
 static void kasan_poison_element(mempool_t *pool, void *element)
 {
-       if (pool->alloc == mempool_alloc_slab)
-               kasan_poison_slab_free(pool->pool_data, element);
-       if (pool->alloc == mempool_kmalloc)
-               kasan_kfree(element);
+       if (pool->alloc == mempool_alloc_slab || pool->alloc == mempool_kmalloc)
+               kasan_poison_kfree(element);
        if (pool->alloc == mempool_alloc_pages)
                kasan_free_pages(element, (unsigned long)pool->pool_data);
 }
 
 static void kasan_unpoison_element(mempool_t *pool, void *element, gfp_t flags)
 {
-       if (pool->alloc == mempool_alloc_slab)
-               kasan_slab_alloc(pool->pool_data, element, flags);
-       if (pool->alloc == mempool_kmalloc)
-               kasan_krealloc(element, (size_t)pool->pool_data, flags);
+       if (pool->alloc == mempool_alloc_slab || pool->alloc == mempool_kmalloc)
+               kasan_unpoison_slab(element);
        if (pool->alloc == mempool_alloc_pages)
                kasan_alloc_pages(element, (unsigned long)pool->pool_data);
 }
--

2.7.3