linux-kernel - Re: [PATCH 05/15] stackdepot: use fixed-sized slots for stack records

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+fCnZftKPJ7zDWmPRjxYXQK91DX2eEw0nDNtYW856399v__Hg@mail.gmail.com>
Date:   Wed, 13 Sep 2023 19:07:21 +0200
From:   Andrey Konovalov <andreyknvl@...il.com>
To:     Alexander Potapenko <glider@...gle.com>
Cc:     andrey.konovalov@...ux.dev, Marco Elver <elver@...gle.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Vlastimil Babka <vbabka@...e.cz>, kasan-dev@...glegroups.com,
        Evgenii Stepanov <eugenis@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org,
        Andrey Konovalov <andreyknvl@...gle.com>
Subject: Re: [PATCH 05/15] stackdepot: use fixed-sized slots for stack records

On Wed, Aug 30, 2023 at 10:22 AM Alexander Potapenko <glider@...gle.com> wrote:
>
> On Tue, Aug 29, 2023 at 7:11 PM <andrey.konovalov@...ux.dev> wrote:
> >
> > From: Andrey Konovalov <andreyknvl@...gle.com>
> >
> > Instead of storing stack records in stack depot pools one right after
> > another, use 32-frame-sized slots.
>
> I am slightly concerned about the KMSAN use case here, which defines
> KMSAN_STACK_DEPTH to 64.

Hm, indeed. KASAN also defines the depth to 64 actually.

I think it's reasonable to change the default value to 64 to cover all
the existing users. And whoever wants to save up on memory can change
the Kconfig parameter (I'll add one as you suggested).

> I don't have a comprehensive stack depth breakdown, but a quick poking
> around syzkaller.appspot.com shows several cases where the stacks are
> actually longer than 32 frames.

Whichever value we choose, some of stack traces will not fit
unfortunately. But yeah, 64 seems to be a more reasonable value.

> Can you add a config parameter for the stack depth instead of
> mandating 32 frames everywhere?

Sure, will do in v2.

> As a side note, kmsan_internal_chain_origin()
> (https://elixir.bootlin.com/linux/latest/source/mm/kmsan/core.c#L214)
> creates small 3-frame records in the stack depot to link two stacks
> together, which will add unnecessary stackdepot pressure.
> But this can be fixed by storing both the new stack trace and the link
> to the old stack trace in the same record.

Do you mean this can be fixed in KMSAN? Or do you mean some kind of an
extension to the stack depot interface?