Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <cover.1662411799.git.andreyknvl@google.com> <CA+fCnZdok0KzOfYmXHQMNFmiuU1H26y8=PaRZ+F0YqTbgxH1Ww@mail.gmail.com>
In-Reply-To: <CA+fCnZdok0KzOfYmXHQMNFmiuU1H26y8=PaRZ+F0YqTbgxH1Ww@mail.gmail.com>
From:   Marco Elver <elver@google.com>
Date:   Mon, 12 Sep 2022 11:39:07 +0200
Message-ID: <CANpmjNM3RqQpvxvZ4+J9DYvMjcZwWjwEGakQb8U4DL+Eu=6K5A@mail.gmail.com>
Subject: Re: [PATCH mm v3 00/34] kasan: switch tag-based modes to stack ring
 from per-object metadata
To:     Andrey Konovalov <andreyknvl@gmail.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        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
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sun, 11 Sept 2022 at 13:50, Andrey Konovalov <andreyknvl@gmail.com> wrote:
>
> 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!

I see them in -next, so they've been picked up?

FWIW, my concerns have been addressed, so for patches that don't yet
have my Reviewed:


Acked-by: Marco Elver <elver@google.com>