[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <606fe227-8500-479f-a753-5cc7504c0c3c@lucifer.local>
Date: Thu, 7 Aug 2025 16:39:42 +0100
From: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: LKML <linux-kernel@...r.kernel.org>,
Linus Torvalds <torvalds@...uxfoundation.org>,
Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Arnaldo Carvalho de Melo <acme@...hat.com>,
Kees Cook <kees@...nel.org>
Subject: Re: [patch 0/6] perf: Convert mmap() related reference counts to
refcount_t
On Wed, Aug 06, 2025 at 10:12:52PM +0200, Thomas Gleixner wrote:
> The recently fixed reference count leaks could have been detected by using
> refcount_t and refcount_t would have mitigated the potential overflow at
> least.
>
> It turned out that converting the code as is does not work as the
> allocation code ends up doing a refcount_inc() for the first allocation,
> which causes refcount_t sanity checks to emit a UAF warning.
>
> The reason is that the code is sharing functionality at the wrong level and
> ends up being overly complicated for no reason. That's what inevitable led
> to the refcount leak problems.
>
> Address this by splitting the ringbuffer and the AUX buffer mapping and
> allocation parts out into seperate functions, which handle the reference
> counts in a sane way.
>
> That not only simplifies the code and makes it halfways comprehensible, but
> also allows to convert the mmap() related reference counts to refcount_t.
>
> It survives lightweight testing with perf and passes the perf/mmap
> selftest.
>
> The series applies on top of Linus tree and is also available from git:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git perf/refcounts
>
> Thanks,
>
> tglx
> ---
> include/linux/perf_event.h | 2
> kernel/events/core.c | 361 ++++++++++++++++++++++----------------------
> kernel/events/internal.h | 4
> kernel/events/ring_buffer.c | 2
> 4 files changed, 185 insertions(+), 184 deletions(-)
Found what appear to be a couple of bugs in 4/6, will pause review until
addressed as it seems that one patch fundamentally relies on the former,
etc. etc. and fixes will likely shuffle.
Will resume checks on respin/you indicate that my review has a mistake in it :)
Cheers, Lorenzo
Powered by blists - more mailing lists