[patch 0/6] perf: Convert mmap() related reference counts to refcount_t

Thomas Gleixner posted 6 patches 1 month, 4 weeks ago
There is a newer version of this series
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(-)
[patch 0/6] perf: Convert mmap() related reference counts to refcount_t
Posted by Thomas Gleixner 1 month, 4 weeks ago
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(-)
Re: [patch 0/6] perf: Convert mmap() related reference counts to refcount_t
Posted by Lorenzo Stoakes 1 month, 4 weeks ago
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