Thread (15 messages) 15 messages, 2 authors, 2016-08-02

Re: [PATCH 6/6] kasan: improve double-free reports.

From: Alexander Potapenko <glider@google.com>
Date: 2016-08-02 17:15:15
Also in: lkml

On Mon, Aug 1, 2016 at 4:45 PM, Andrey Ryabinin [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Currently we just dump stack in case of double free bug.
Let's dump all info about the object that we have.

Signed-off-by: Andrey Ryabinin <redacted>
---
 mm/kasan/kasan.c  |  3 +--
 mm/kasan/kasan.h  |  2 ++
 mm/kasan/report.c | 54 ++++++++++++++++++++++++++++++++++++++----------------
 3 files changed, 41 insertions(+), 18 deletions(-)
diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
index 92750e3..88af13c 100644
--- a/mm/kasan/kasan.c
+++ b/mm/kasan/kasan.c
@@ -543,8 +543,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) {
-               pr_err("Double free");
-               dump_stack();
+               kasan_report_double_free(cache, object, shadow_byte);
                return true;
        }
diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h
index 9b7b31e..e5c2181 100644
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -99,6 +99,8 @@ static inline bool kasan_report_enabled(void)

 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,
+                       s8 shadow);

 #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 f437398..ee2bdb4 100644
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -116,6 +116,26 @@ static inline bool init_task_stack_addr(const void *addr)
                        sizeof(init_thread_union.stack));
 }

+static DEFINE_SPINLOCK(report_lock);
+
+static void kasan_start_report(unsigned long *flags)
+{
+       /*
+        * Make sure we don't end up in loop.
+        */
+       kasan_disable_current();
+       spin_lock_irqsave(&report_lock, *flags);
+       pr_err("==================================================================\n");
+}
+
+static void kasan_end_report(unsigned long *flags)
+{
+       pr_err("==================================================================\n");
+       add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);
Don't we want to add the taint as early as possible once we've
detected the error?
quoted hunk ↗ jump to hunk
+       spin_unlock_irqrestore(&report_lock, *flags);
+       kasan_enable_current();
+}
+
 static void print_track(struct kasan_track *track)
 {
        pr_err("PID = %u\n", track->pid);
@@ -129,8 +149,7 @@ static void print_track(struct kasan_track *track)
        }
 }

-static void kasan_object_err(struct kmem_cache *cache, struct page *page,
-                               void *object, char *unused_reason)
+static void kasan_object_err(struct kmem_cache *cache, void *object)
 {
        struct kasan_alloc_meta *alloc_info = get_alloc_info(cache, object);
@@ -147,6 +166,18 @@ static void kasan_object_err(struct kmem_cache *cache, struct page *page,
        print_track(&alloc_info->free_track);
 }

+void kasan_report_double_free(struct kmem_cache *cache, void *object,
+                       s8 shadow)
+{
+       unsigned long flags;
+
+       kasan_start_report(&flags);
+       pr_err("BUG: Double free or corrupt pointer\n");
How about "Double free or freeing an invalid pointer\n"?
I think "corrupt pointer" doesn't exactly reflect where the bug is.
quoted hunk ↗ jump to hunk
+       pr_err("Unexpected shadow byte: 0x%hhX\n", shadow);
+       kasan_object_err(cache, object);
+       kasan_end_report(&flags);
+}
+
 static void print_address_description(struct kasan_access_info *info)
 {
        const void *addr = info->access_addr;
@@ -160,8 +191,7 @@ static void print_address_description(struct kasan_access_info *info)
                        struct kmem_cache *cache = page->slab_cache;
                        object = nearest_obj(cache, page,
                                                (void *)info->access_addr);
-                       kasan_object_err(cache, page, object,
-                                       "kasan: bad access detected");
+                       kasan_object_err(cache, object);
                        return;
                }
                dump_page(page, "kasan: bad access detected");
@@ -226,19 +256,13 @@ static void print_shadow_for_address(const void *addr)
        }
 }

-static DEFINE_SPINLOCK(report_lock);
-
 static void kasan_report_error(struct kasan_access_info *info)
 {
        unsigned long flags;
        const char *bug_type;

-       /*
-        * Make sure we don't end up in loop.
-        */
-       kasan_disable_current();
-       spin_lock_irqsave(&report_lock, flags);
-       pr_err("==================================================================\n");
+       kasan_start_report(&flags);
+
        if (info->access_addr <
                        kasan_shadow_to_mem((void *)KASAN_SHADOW_START)) {
                if ((unsigned long)info->access_addr < PAGE_SIZE)
@@ -259,10 +283,8 @@ static void kasan_report_error(struct kasan_access_info *info)
                print_address_description(info);
                print_shadow_for_address(info->first_bad_addr);
        }
-       pr_err("==================================================================\n");
-       add_taint(TAINT_BAD_PAGE, LOCKDEP_NOW_UNRELIABLE);
-       spin_unlock_irqrestore(&report_lock, flags);
-       kasan_enable_current();
+
+       kasan_end_report(&flags);
 }

 void kasan_report(unsigned long addr, size_t size,
--
2.7.3


-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

--
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>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help