linux-kernel - Re: [PATCH v4 12/22] lib/stackdepot: use read/write lock

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: <CANpmjNOz7tBMK-HoyZNVR2KcgxEBY1Qym=DRa9gHLFkaNHLmVw@mail.gmail.com>
Date: Sat, 13 Jan 2024 10:23:31 +0100
From: Marco Elver <elver@...gle.com>
To: Andi Kleen <ak@...ux.intel.com>
Cc: Andrey Konovalov <andreyknvl@...il.com>, Oscar Salvador <osalvador@...e.de>, andrey.konovalov@...ux.dev, 
	Andrew Morton <akpm@...ux-foundation.org>, Alexander Potapenko <glider@...gle.com>, 
	Dmitry Vyukov <dvyukov@...gle.com>, Vlastimil Babka <vbabka@...e.cz>, kasan-dev@...glegroups.com, 
	Evgenii Stepanov <eugenis@...gle.com>, linux-mm@...ck.org, linux-kernel@...r.kernel.org, 
	Andrey Konovalov <andreyknvl@...gle.com>
Subject: Re: [PATCH v4 12/22] lib/stackdepot: use read/write lock

On Sat, 13 Jan 2024 at 10:19, Andi Kleen <ak@...ux.intel.com> wrote:
>
> On Sat, Jan 13, 2024 at 10:12:21AM +0100, Marco Elver wrote:
> > On Sat, 13 Jan 2024 at 02:24, Andi Kleen <ak@...ux.intel.com> wrote:
> > >
> > > On Fri, Jan 12, 2024 at 11:15:05PM +0100, Marco Elver wrote:
> > > > +             /*
> > > > +              * Stack traces of size 0 are never saved, and we can simply use
> > > > +              * the size field as an indicator if this is a new unused stack
> > > > +              * record in the freelist.
> > > > +              */
> > > > +             stack->size = 0;
> > >
> > > I would use WRITE_ONCE here too, at least for TSan.
> >
> > This is written with the pool_lock held.
>
> ...which doesn't help because the readers don't take it?

This function is only refilling the freelist. Readers don't see it yet
because it's in none of the hash table buckets. The freelist is only
ever accessed under the lock.

Once an entry is allocated from the freelist, its size is overwritten
with something non-zero (since it then contains a stack trace). Those
updates are released into the right hash table bucket with
list_add_rcu() (which implies a release).

Am I missing something else?