linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrey Konovalov <andreyknvl@gmail.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Marco Elver <elver@google.com>,
	Alexander Potapenko <glider@google.com>,
	 Dmitry Vyukov <dvyukov@google.com>,
	Andrey Ryabinin <ryabinin.a.a@gmail.com>,
	 kasan-dev <kasan-dev@googlegroups.com>,
	Peter Collingbourne <pcc@google.com>,
	 Evgenii Stepanov <eugenis@google.com>,
	Florian Mayer <fmayer@google.com>,
	 Linux Memory Management List <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>,
	 Andrey Konovalov <andreyknvl@google.com>,
	andrey.konovalov@linux.dev
Subject: Re: [PATCH mm v3 00/34] kasan: switch tag-based modes to stack ring from per-object metadata
Date: Sun, 11 Sep 2022 13:50:03 +0200	[thread overview]
Message-ID: <CA+fCnZdok0KzOfYmXHQMNFmiuU1H26y8=PaRZ+F0YqTbgxH1Ww@mail.gmail.com> (raw)
In-Reply-To: <cover.1662411799.git.andreyknvl@google.com>

On Mon, Sep 5, 2022 at 11:05 PM <andrey.konovalov@linux.dev> wrote:
>
> From: Andrey Konovalov <andreyknvl@google.com>
>
> This series makes the tag-based KASAN modes use a ring buffer for storing
> stack depot handles for alloc/free stack traces for slab objects instead
> of per-object metadata. 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 advantages of this approach over storing stack trace handles in
> per-object metadata with the tag-based KASAN modes:
>
> - Allows to find relevant stack traces for use-after-free bugs without
>   using quarantine for freed memory. (Currently, if the object was
>   reallocated multiple times, the report contains the latest alloc/free
>   stack traces, not necessarily the ones relevant to the buggy allocation.)
> - Allows to better identify and mark use-after-free bugs, effectively
>   making the CONFIG_KASAN_TAGS_IDENTIFY functionality always-on.
> - Has fixed memory overhead.
>
> The disadvantage:
>
> - If the affected object was allocated/freed long before the bug happened
>   and the stack trace events were purged from the stack ring, the report
>   will have no stack traces.
>
> Discussion
> ==========
>
> The proposed implementation of the stack ring uses a single ring buffer for
> the whole kernel. This might lead to contention due to atomic accesses to
> the ring buffer index on multicore systems.
>
> At this point, it is unknown whether the performance impact from this
> contention would be significant compared to the slowdown introduced by
> collecting stack traces due to the planned changes to the latter part,
> see the section below.
>
> For now, the proposed implementation is deemed to be good enough, but this
> might need to be revisited once the stack collection becomes faster.
>
> A considered alternative is to keep a separate ring buffer for each CPU
> and then iterate over all of them when printing a bug report. This approach
> requires somehow figuring out which of the stack rings has the freshest
> stack traces for an object if multiple stack rings have them.
>
> Further plans
> =============
>
> This series is a part of an effort to make KASAN stack trace collection
> suitable for production. This requires stack trace collection to be fast
> and memory-bounded.
>
> The planned steps are:
>
> 1. Speed up stack trace collection (potentially, by using SCS;
>    patches on-hold until steps #2 and #3 are completed).
> 2. Keep stack trace handles in the stack ring (this series).
> 3. Add a memory-bounded mode to stack depot or provide an alternative
>    memory-bounded stack storage.
> 4. Potentially, implement stack trace collection sampling to minimize
>    the performance impact.
>
> Thanks!

Hi Andrew,

Could you consider picking up this series into mm?

Most of the patches have a Reviewed-by tag from Marco, and I've
addressed the last few comments he had in v3.

Thanks!


  parent reply	other threads:[~2022-09-11 11:50 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-05 21:05 andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 01/34] kasan: check KASAN_NO_FREE_META in __kasan_metadata_size andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 02/34] kasan: rename kasan_set_*_info to kasan_save_*_info andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 03/34] kasan: move is_kmalloc check out of save_alloc_info andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 04/34] kasan: split save_alloc_info implementations andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 05/34] kasan: drop CONFIG_KASAN_TAGS_IDENTIFY andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 06/34] kasan: introduce kasan_print_aux_stacks andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 07/34] kasan: introduce kasan_get_alloc_track andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 08/34] kasan: introduce kasan_init_object_meta andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 09/34] kasan: clear metadata functions for tag-based modes andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 10/34] kasan: move kasan_get_*_meta to generic.c andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 11/34] kasan: introduce kasan_requires_meta andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 12/34] kasan: introduce kasan_init_cache_meta andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 13/34] kasan: drop CONFIG_KASAN_GENERIC check from kasan_init_cache_meta andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 14/34] kasan: only define kasan_metadata_size for Generic mode andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 15/34] kasan: only define kasan_never_merge " andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 16/34] kasan: only define metadata offsets " andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 17/34] kasan: only define metadata structs " andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 18/34] kasan: only define kasan_cache_create " andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 19/34] kasan: pass tagged pointers to kasan_save_alloc/free_info andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 20/34] kasan: move kasan_get_alloc/free_track definitions andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 21/34] kasan: cosmetic changes in report.c andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 22/34] kasan: use virt_addr_valid in kasan_addr_to_page/slab andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 23/34] kasan: use kasan_addr_to_slab in print_address_description andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 24/34] kasan: make kasan_addr_to_page static andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 25/34] kasan: simplify print_report andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 26/34] kasan: introduce complete_report_info andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 27/34] kasan: fill in cache and object in complete_report_info andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 28/34] kasan: rework function arguments in report.c andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 29/34] kasan: introduce kasan_complete_mode_report_info andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 30/34] kasan: implement stack ring for tag-based modes andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 31/34] kasan: support kasan.stacktrace for SW_TAGS andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 32/34] kasan: dynamically allocate stack ring entries andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 33/34] kasan: better identify bug types for tag-based modes andrey.konovalov
2022-09-05 21:05 ` [PATCH mm v3 34/34] kasan: add another use-after-free test andrey.konovalov
2022-09-11 11:50 ` Andrey Konovalov [this message]
2022-09-12  9:39   ` [PATCH mm v3 00/34] kasan: switch tag-based modes to stack ring from per-object metadata Marco Elver
2022-09-12 20:06     ` Andrew Morton
2022-09-19  8:07       ` Yu Zhao
2022-09-20 18:59         ` Andrey Konovalov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CA+fCnZdok0KzOfYmXHQMNFmiuU1H26y8=PaRZ+F0YqTbgxH1Ww@mail.gmail.com' \
    --to=andreyknvl@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=andrey.konovalov@linux.dev \
    --cc=andreyknvl@google.com \
    --cc=dvyukov@google.com \
    --cc=elver@google.com \
    --cc=eugenis@google.com \
    --cc=fmayer@google.com \
    --cc=glider@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=pcc@google.com \
    --cc=ryabinin.a.a@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox