From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 817CEC433EF for ; Tue, 19 Jul 2022 11:41:42 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D83786B0071; Tue, 19 Jul 2022 07:41:41 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D312F6B0073; Tue, 19 Jul 2022 07:41:41 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BF8BC6B0074; Tue, 19 Jul 2022 07:41:41 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id AA37A6B0071 for ; Tue, 19 Jul 2022 07:41:41 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id 7D59B35461 for ; Tue, 19 Jul 2022 11:41:41 +0000 (UTC) X-FDA: 79703659602.08.1B256E1 Received: from mail-yw1-f181.google.com (mail-yw1-f181.google.com [209.85.128.181]) by imf22.hostedemail.com (Postfix) with ESMTP id 1BF79C0088 for ; Tue, 19 Jul 2022 11:41:40 +0000 (UTC) Received: by mail-yw1-f181.google.com with SMTP id 00721157ae682-31e0d4ad6caso77066717b3.10 for ; Tue, 19 Jul 2022 04:41:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=xi609iWd+t1xpxJ70rNlw8Yfb5LgcoJNw18fVkapUSc=; b=mXoGlvDqg1Xak8fNrrAPXaZLGTkisG3gqkOez/GJCX8klSIoirhV61XNHDkNUM6q1+ 4CNX4Qkn7bmgiWDcO+MBzUqGkOg5vVvPWcIUnwBVQ/1uIQKqDzYri+SIW4EgWz/M/CDY Na6bItbeF65BGH3AcvtMtFv5vVGtLDhv1zx7ufNpTGOL3CcB7mNgXeOUPrQu3+fl4ACV ubc62d8XfJXpXj4ENO1Git/6Vja0GgEta/nKSIcqzW1UYVeKueUr0d8j+aifNnco4LD4 k1Un1EpFT+up0iOJPjAnouXDRgC1RhaPLXcVoPKzXbZuUd9EBZ39YQys70pJnr7/RzUc RBkw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=xi609iWd+t1xpxJ70rNlw8Yfb5LgcoJNw18fVkapUSc=; b=QjIG7KUTKIRO+5opKGDgRQ7nfrk5HliJkLkuo5JNR5p2Vx1Vnvn0Boupmu4Gn9IcvU OTGeDbxKK7nfyV6zJmbTlchOEwDRM0SPytVaGBcSblSjSTzgKhoC7d3bErkKVe7MmVBi OkIJshoyVDz65UC8VxqP5jZ0h93NWPpPi0NqzkMPnnywkBz0p6FKver9EGJ+iJwC3h9A 7rH0RM0B/eNsj7yPAK1bJB7Gc4St4NgvDBWvnGwiy6wKHRAzIO/mmHbK3ATyG1H0aQSR FjWu5t/EC3gsDDUDBEne0smrHpqsHU1SZF4AOUq08ThqRfugKbNOQ0Kh2OkiAGEuLbeM TUVw== X-Gm-Message-State: AJIora9OEqlwZaDNHRgb38VqlovMgDR+vOzxLHgOJslefJOBt/i9x5pm ZXE9c375qgWZvdksmJUMIISpjgiougqu7SmHZCAKXg== X-Google-Smtp-Source: AGRyM1umpwwQl+mM7wE1Ymxs73iZxo2rJwoQ4M5S8YNTd/RKBQkHaxU30mH7+ChcbBiei6IG2NMlaleqf9tGlEKXblM= X-Received: by 2002:a0d:e60d:0:b0:31c:8046:8ff with SMTP id p13-20020a0de60d000000b0031c804608ffmr36027909ywe.412.1658230900185; Tue, 19 Jul 2022 04:41:40 -0700 (PDT) MIME-Version: 1.0 References: <0e910197bfbcf505122f6dae2ee9b90ff8ee31f7.1658189199.git.andreyknvl@google.com> In-Reply-To: <0e910197bfbcf505122f6dae2ee9b90ff8ee31f7.1658189199.git.andreyknvl@google.com> From: Marco Elver Date: Tue, 19 Jul 2022 13:41:04 +0200 Message-ID: Subject: Re: [PATCH mm v2 30/33] kasan: implement stack ring for tag-based modes To: andrey.konovalov@linux.dev Cc: Alexander Potapenko , Andrey Konovalov , Dmitry Vyukov , Andrey Ryabinin , kasan-dev@googlegroups.com, Peter Collingbourne , Evgenii Stepanov , Florian Mayer , Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrey Konovalov Content-Type: text/plain; charset="UTF-8" ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=mXoGlvDq; spf=pass (imf22.hostedemail.com: domain of elver@google.com designates 209.85.128.181 as permitted sender) smtp.mailfrom=elver@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1658230901; a=rsa-sha256; cv=none; b=DXlNpRpQY2Q6lZA4AlrP7yLnT1nYJQp/Y97c8dfDHf/bp+NJ03IhBu0a05GZqeqsab6vzY 7arvPYfNWrTMG9l/J5nb0tKXNbf9fpSkLfZ6zDeFVIrvf0ctzIFaw5UKB6osbiAUPhW2o5 qE704nh8gCUgVH8S9jZmIrGrFwKF02I= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1658230901; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=xi609iWd+t1xpxJ70rNlw8Yfb5LgcoJNw18fVkapUSc=; b=Dm0fsQ/x2aRBOuvK6OR34N8Enwiu/Yyi1nWUFC6a51/zrkrDbfmuR0GRdc1philOAe5CzD MGKdqsotE7NjPNo61QvqztlH82Hy7AuiingOM1LK5bhQWZ/kkwRrwTqOjDU7q514V9CKG9 KCYgjZ8oC/2R+7tWkC8zqfWg3iFtbY0= Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=google.com header.s=20210112 header.b=mXoGlvDq; spf=pass (imf22.hostedemail.com: domain of elver@google.com designates 209.85.128.181 as permitted sender) smtp.mailfrom=elver@google.com; dmarc=pass (policy=reject) header.from=google.com X-Rspam-User: X-Rspamd-Server: rspam01 X-Stat-Signature: qr747mch14xwpyxm86sdbpcxz5j9e5r3 X-Rspamd-Queue-Id: 1BF79C0088 X-HE-Tag: 1658230900-892445 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, 19 Jul 2022 at 02:15, wrote: > > From: Andrey Konovalov > > Implement storing stack depot handles for alloc/free stack traces for > slab objects for the tag-based KASAN modes in a ring buffer. > > This ring buffer is referred to as the stack ring. > > On each alloc/free of a slab object, the tagged address of the object and > the current stack trace are recorded in the stack ring. > > On each bug report, if the accessed address belongs to a slab object, the > stack ring is scanned for matching entries. The newest entries are used to > print the alloc/free stack traces in the report: one entry for alloc and > one for free. > > The number of entries in the stack ring is fixed in this patch, but one of > the following patches adds a command-line argument to control it. > > Signed-off-by: Andrey Konovalov > > --- > > Changes v1->v2: > - Only use the atomic type for pos, use READ/WRITE_ONCE() for the rest. > - Rename KASAN_STACK_RING_ENTRIES to KASAN_STACK_RING_SIZE. > - Rename object local variable in kasan_complete_mode_report_info() to > ptr to match the name in kasan_stack_ring_entry. > - Detect stack ring entry slots that are being written to. > - Use read-write lock to disallow reading half-written stack ring entries. > - Add a comment about the stack ring being best-effort. > --- > mm/kasan/kasan.h | 21 ++++++++++++ > mm/kasan/report_tags.c | 76 ++++++++++++++++++++++++++++++++++++++++++ > mm/kasan/tags.c | 50 +++++++++++++++++++++++++++ > 3 files changed, 147 insertions(+) > > diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h > index 7df107dc400a..cfff81139d67 100644 > --- a/mm/kasan/kasan.h > +++ b/mm/kasan/kasan.h > @@ -2,6 +2,7 @@ > #ifndef __MM_KASAN_KASAN_H > #define __MM_KASAN_KASAN_H > > +#include > #include > #include > #include > @@ -233,6 +234,26 @@ struct kasan_free_meta { > > #endif /* CONFIG_KASAN_GENERIC */ > > +#if defined(CONFIG_KASAN_SW_TAGS) || defined(CONFIG_KASAN_HW_TAGS) > + > +struct kasan_stack_ring_entry { > + void *ptr; > + size_t size; > + u32 pid; > + depot_stack_handle_t stack; > + bool is_free; > +}; > + > +#define KASAN_STACK_RING_SIZE (32 << 10) > + > +struct kasan_stack_ring { > + rwlock_t lock; > + atomic64_t pos; > + struct kasan_stack_ring_entry entries[KASAN_STACK_RING_SIZE]; > +}; > + > +#endif /* CONFIG_KASAN_SW_TAGS || CONFIG_KASAN_HW_TAGS */ > + > #if IS_ENABLED(CONFIG_KASAN_KUNIT_TEST) > /* Used in KUnit-compatible KASAN tests. */ > struct kunit_kasan_status { > diff --git a/mm/kasan/report_tags.c b/mm/kasan/report_tags.c > index 5cbac2cdb177..a996489e6dac 100644 > --- a/mm/kasan/report_tags.c > +++ b/mm/kasan/report_tags.c > @@ -4,8 +4,12 @@ > * Copyright (c) 2020 Google, Inc. > */ > > +#include > + > #include "kasan.h" > > +extern struct kasan_stack_ring stack_ring; > + > static const char *get_bug_type(struct kasan_report_info *info) > { > /* > @@ -24,5 +28,77 @@ static const char *get_bug_type(struct kasan_report_info *info) > > void kasan_complete_mode_report_info(struct kasan_report_info *info) > { > + unsigned long flags; > + u64 pos; > + struct kasan_stack_ring_entry *entry; > + void *ptr; > + u32 pid; > + depot_stack_handle_t stack; > + bool is_free; > + bool alloc_found = false, free_found = false; > + > info->bug_type = get_bug_type(info); > + > + if (!info->cache || !info->object) > + return; > + } > + > + write_lock_irqsave(&stack_ring.lock, flags); > + > + pos = atomic64_read(&stack_ring.pos); > + > + /* > + * The loop below tries to find stack ring entries relevant to the > + * buggy object. This is a best-effort process. > + * > + * First, another object with the same tag can be allocated in place of > + * the buggy object. Also, since the number of entries is limited, the > + * entries relevant to the buggy object can be overwritten. > + */ > + > + for (u64 i = pos - 1; i != pos - 1 - KASAN_STACK_RING_SIZE; i--) { > + if (alloc_found && free_found) > + break; > + > + entry = &stack_ring.entries[i % KASAN_STACK_RING_SIZE]; > + > + /* Paired with smp_store_release() in save_stack_info(). */ > + ptr = (void *)smp_load_acquire(&entry->ptr); > + > + if (kasan_reset_tag(ptr) != info->object || > + get_tag(ptr) != get_tag(info->access_addr)) > + continue; > + > + pid = READ_ONCE(entry->pid); > + stack = READ_ONCE(entry->stack); > + is_free = READ_ONCE(entry->is_free); > + > + /* Try detecting if the entry was changed while being read. */ > + smp_mb(); > + if (ptr != (void *)READ_ONCE(entry->ptr)) > + continue; I thought the re-validation is no longer needed because of the rwlock protection? The rest looks fine now. > + if (is_free) { > + /* > + * Second free of the same object. > + * Give up on trying to find the alloc entry. > + */ > + if (free_found) > + break; > + > + info->free_track.pid = pid; > + info->free_track.stack = stack; > + free_found = true; > + } else { > + /* Second alloc of the same object. Give up. */ > + if (alloc_found) > + break; > + > + info->alloc_track.pid = pid; > + info->alloc_track.stack = stack; > + alloc_found = true; > + } > + } > + > + write_unlock_irqrestore(&stack_ring.lock, flags); > } > diff --git a/mm/kasan/tags.c b/mm/kasan/tags.c > index 39a0481e5228..07828021c1f5 100644 > --- a/mm/kasan/tags.c > +++ b/mm/kasan/tags.c > @@ -6,6 +6,7 @@ > * Copyright (c) 2020 Google, Inc. > */ > > +#include > #include > #include > #include > @@ -16,11 +17,60 @@ > #include > > #include "kasan.h" > +#include "../slab.h" > + > +/* Non-zero, as initial pointer values are 0. */ > +#define STACK_RING_BUSY_PTR ((void *)1) > + > +struct kasan_stack_ring stack_ring; > + > +static void save_stack_info(struct kmem_cache *cache, void *object, > + gfp_t gfp_flags, bool is_free) > +{ > + unsigned long flags; > + depot_stack_handle_t stack; > + u64 pos; > + struct kasan_stack_ring_entry *entry; > + void *old_ptr; > + > + stack = kasan_save_stack(gfp_flags, true); > + > + /* > + * Prevent save_stack_info() from modifying stack ring > + * when kasan_complete_mode_report_info() is walking it. > + */ > + read_lock_irqsave(&stack_ring.lock, flags); > + > +next: > + pos = atomic64_fetch_add(1, &stack_ring.pos); > + entry = &stack_ring.entries[pos % KASAN_STACK_RING_SIZE]; > + > + /* Detect stack ring entry slots that are being written to. */ > + old_ptr = READ_ONCE(entry->ptr); > + if (old_ptr == STACK_RING_BUSY_PTR) > + goto next; /* Busy slot. */ > + if (!try_cmpxchg(&entry->ptr, &old_ptr, STACK_RING_BUSY_PTR)) > + goto next; /* Busy slot. */ > + > + WRITE_ONCE(entry->size, cache->object_size); > + WRITE_ONCE(entry->pid, current->pid); > + WRITE_ONCE(entry->stack, stack); > + WRITE_ONCE(entry->is_free, is_free); > + > + /* > + * Paired with smp_load_acquire() in kasan_complete_mode_report_info(). > + */ > + smp_store_release(&entry->ptr, (s64)object); > + > + read_unlock_irqrestore(&stack_ring.lock, flags); > +} > > void kasan_save_alloc_info(struct kmem_cache *cache, void *object, gfp_t flags) > { > + save_stack_info(cache, object, flags, false); > } > > void kasan_save_free_info(struct kmem_cache *cache, void *object) > { > + save_stack_info(cache, object, GFP_NOWAIT, true); > } > -- > 2.25.1 >