From patchwork Tue Mar 28 14:00:53 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrey Konovalov X-Patchwork-Id: 744298 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3vssy83z3Pz9s7K for ; Wed, 29 Mar 2017 01:01:36 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.b="BHOq1wvc"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754105AbdC1OBe (ORCPT ); Tue, 28 Mar 2017 10:01:34 -0400 Received: from mail-wr0-f178.google.com ([209.85.128.178]:33015 "EHLO mail-wr0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752258AbdC1OBb (ORCPT ); Tue, 28 Mar 2017 10:01:31 -0400 Received: by mail-wr0-f178.google.com with SMTP id w43so92175537wrb.0 for ; Tue, 28 Mar 2017 07:01:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :in-reply-to:references; bh=rjD5muR7zxfZ1o7LBPh2uCP1pDMulmRWnX7rqsK4iSA=; b=BHOq1wvcKUZlXoYW2tV6DVFrhyKCKdnm8IdX6gEiSuEKRTQrzGLlGexHMCQNJfvJFT dxqy/Cun94RTGjGsHHU0/7kCOW9xujP6wAGKywRth6NGWVkHFu1pRCmb51wuGPn/Ixiu 3ZTdZFlBwM8dIni7qDDUSXp+xu6raiPGtRit73Uhswujc70ptgdI0kwE5608thFiVCSq 2kN+mWUuIbyCSjvJ0+KMYwPSNfw5A7bpxWv0NbEIDUqRtxqlYTxZKTWcHOlN1ZykfWCi 7VZ2Y0y/TRGW0AnIupf74Meh5Tj3Wz7NUewJl2KGg+ypOjBSw4TtHLrlg2+bjHz71eya kElw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:in-reply-to:references; bh=rjD5muR7zxfZ1o7LBPh2uCP1pDMulmRWnX7rqsK4iSA=; b=ek56WJofS1AbhpDd1UEQL4PwmmDxbVXS2V7czDS/WmXSZv4cV7M4fDtvpnEol1uXJi QK4cq5J/92k+hHq3PWC1TZ+yYR/1r0uGX4uIoeS7ONkEMCQPvnrDBa+o1Gedgu6WvegC R8d6JYtjORgG1lXf4AfLU6y0njnlnWe0P4hmnJikRw5U2VSrFQ1AnUq6+T6YIc0pKd1a JRrowuikYaID6Grr4EaJt9ynbynQCxrk5n0uYXPydSFoqWpzoNCdA7csOaTwc+c+rod/ fXDsYI5MjCO7Cwka7+SCdJLwNDYr7PjUk9MsQlFvdwXTrdzU04NJi8BiBv2NgyFl+utX xJvw== X-Gm-Message-State: AFeK/H2WMVI/BgIp9PScnrHcxzHckQxrCic9tcj4GBQQlYM+734X3CvhnK/eBF1+MVMyaPsy X-Received: by 10.223.154.199 with SMTP id a65mr23502708wrc.78.1490709662844; Tue, 28 Mar 2017 07:01:02 -0700 (PDT) Received: from andreyknvl0.muc.corp.google.com ([100.105.12.17]) by smtp.gmail.com with ESMTPSA id h16sm5047226wrc.5.2017.03.28.07.00.59 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 28 Mar 2017 07:01:00 -0700 (PDT) Received: by andreyknvl0.muc.corp.google.com (Postfix, from userid 206546) id 441E718097A; Tue, 28 Mar 2017 16:00:56 +0200 (CEST) From: Andrey Konovalov To: "David S . Miller" , Eric Dumazet , Willem de Bruijn , Craig Gallek Cc: netdev@vger.kernel.org, Dmitry Vyukov , Kostya Serebryany , Andrey Konovalov Subject: [PATCH v4 8/9] kasan: improve double-free report format Date: Tue, 28 Mar 2017 16:00:53 +0200 Message-Id: <0b6ea716d5d0787ba6ccdf517c2cea3336b5cfd8.1490383597.git.andreyknvl@google.com> X-Mailer: git-send-email 2.12.2.564.g063fe858b8-goog In-Reply-To: References: In-Reply-To: References: Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Changes double-free report header from: BUG: Double free or freeing an invalid pointer Unexpected shadow byte: 0xFB to: BUG: KASAN: double-free or invalid-free in kmalloc_oob_left+0xe5/0xef This makes a bug uniquely identifiable by the first report line. To account for removing of the unexpected shadow value, print shadow bytes at the end of the report as in reports for other kinds of bugs. To print caller funtion name in the report header, the caller address is passed from SLUB/SLAB free handlers. Signed-off-by: Andrey Konovalov --- include/linux/kasan.h | 2 +- mm/kasan/kasan.c | 5 +++-- mm/kasan/kasan.h | 2 +- mm/kasan/report.c | 30 ++++++++++++++---------------- mm/slab.c | 2 +- mm/slub.c | 12 +++++++----- 6 files changed, 27 insertions(+), 26 deletions(-) diff --git a/include/linux/kasan.h b/include/linux/kasan.h index 5734480c9590..554f843f1625 100644 --- a/include/linux/kasan.h +++ b/include/linux/kasan.h @@ -62,7 +62,7 @@ void kasan_kmalloc(struct kmem_cache *s, const void *object, size_t size, 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 pc); struct kasan_cache { int alloc_meta_offset; diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c index 98b27195e38b..83cc011bb9bc 100644 --- a/mm/kasan/kasan.c +++ b/mm/kasan/kasan.c @@ -567,7 +567,8 @@ 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 pc) { s8 shadow_byte; @@ -577,7 +578,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, shadow_byte); + kasan_report_double_free(cache, object, pc); return true; } diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h index 1c260e6b3b3c..75729173ade9 100644 --- a/mm/kasan/kasan.h +++ b/mm/kasan/kasan.h @@ -104,7 +104,7 @@ 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); + 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 e0f7dbf9e883..2368b8cf5f95 100644 --- a/mm/kasan/report.c +++ b/mm/kasan/report.c @@ -224,22 +224,8 @@ static void describe_object(struct kmem_cache *cache, void *object, describe_object_addr(cache, object, addr); } -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 freeing an invalid pointer\n"); - pr_err("Unexpected shadow byte: 0x%hhX\n", shadow); - dump_stack(); - describe_object(cache, object, NULL); - kasan_end_report(&flags); -} - -static void print_address_description(struct kasan_access_info *info) +static void print_address_description(void *addr) { - void *addr = (void *)info->access_addr; struct page *page = addr_to_page(addr); dump_stack(); @@ -314,6 +300,18 @@ static void print_shadow_for_address(const void *addr) } } +void kasan_report_double_free(struct kmem_cache *cache, void *object, + void *ip) +{ + unsigned long flags; + + kasan_start_report(&flags); + pr_err("BUG: KASAN: double-free or invalid-free in %pS\n", ip); + print_address_description(object); + print_shadow_for_address(object); + kasan_end_report(&flags); +} + static void kasan_report_error(struct kasan_access_info *info) { unsigned long flags; @@ -325,7 +323,7 @@ static void kasan_report_error(struct kasan_access_info *info) if (!addr_has_shadow(info)) { dump_stack(); } else { - print_address_description(info); + print_address_description((void *)info->access_addr); print_shadow_for_address(info->first_bad_addr); } diff --git a/mm/slab.c b/mm/slab.c index 807d86c76908..aba5f30ea63e 100644 --- a/mm/slab.c +++ b/mm/slab.c @@ -3508,7 +3508,7 @@ static 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, caller)) return; ___cache_free(cachep, objp, caller); diff --git a/mm/slub.c b/mm/slub.c index 7f4bc7027ed5..763570a0b15e 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -1325,7 +1325,8 @@ static inline void kfree_hook(const void *x) kasan_kfree_large(x); } -static inline void *slab_free_hook(struct kmem_cache *s, void *x) +static inline void *slab_free_hook(struct kmem_cache *s, void *x, + unsigned long addr) { void *freeptr; @@ -1354,12 +1355,13 @@ 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, addr); return freeptr; } static inline void slab_free_freelist_hook(struct kmem_cache *s, - void *head, void *tail) + void *head, void *tail, + unsigned long addr) { /* * Compiler cannot detect this function can be removed if slab_free_hook() @@ -1376,7 +1378,7 @@ static inline void slab_free_freelist_hook(struct kmem_cache *s, void *freeptr; do { - freeptr = slab_free_hook(s, object); + freeptr = slab_free_hook(s, object, addr); } while ((object != tail_obj) && (object = freeptr)); #endif } @@ -2958,7 +2960,7 @@ static __always_inline void slab_free(struct kmem_cache *s, struct page *page, void *head, void *tail, int cnt, unsigned long addr) { - slab_free_freelist_hook(s, head, tail); + slab_free_freelist_hook(s, head, tail, addr); /* * slab_free_freelist_hook() could have put the items into quarantine. * If so, no need to free them.