On x86 platform, kernel v5.10.228, perf-report command aborts due to "free():
invalid pointer" when perf-record command is run with taken branch stack
sampling enabled. This regression can be reproduced with the following steps:

	- sudo perf record -b
	- sudo perf report

The root cause is that bi[i].to.ms.maps does not always point to thread->maps,
which is a buffer dynamically allocated by maps_new(). Instead, it may point to
&machine->kmaps, while kmaps is not a pointer but a variable. The original
upstream commit c1149037f65b ("perf hist: Add missing puts to
hist__account_cycles") worked well because machine->kmaps had been refactored to
a pointer by the previous commit 1a97cee604dc ("perf maps: Use a pointer for
kmaps").

The memory leak issue, which the reverted patch intended to fix, has been solved
by commit cf96b8e45a9b ("perf session: Add missing evlist__delete when deleting
a session"). The root cause is that the evlist is not being deleted on exit in
perf-report, perf-script, and perf-data. Consequently, the reference count of
the thread increased by thread__get() in hist_entry__init() is not decremented
in hist_entry__delete(). As a result, thread->maps is not properly freed.

To this end,

- PATCH 1/2 reverts commit a83fc293acd5c5050a4828eced4a71d2b2fffdd3 to fix the
  abort regression.
- PATCH 2/2 backports cf96b8e45a9b ("perf session: Add missing evlist__delete
  when deleting a session") to fix memory leak issue.

Riccardo Mancini (1):
  perf session: Add missing evlist__delete when deleting a session

Shuai Xue (1):
  Revert "perf hist: Add missing puts to hist__account_cycles"

 tools/perf/util/hist.c    | 10 +++-------
 tools/perf/util/session.c |  5 ++++-
 2 files changed, 7 insertions(+), 8 deletions(-)

-- 
2.39.3

On Mon, Nov 04, 2024 at 07:27:34PM +0800, Shuai Xue wrote:
> On x86 platform, kernel v5.10.228, perf-report command aborts due to "free():
> invalid pointer" when perf-record command is run with taken branch stack
> sampling enabled. This regression can be reproduced with the following steps:
> 
> 	- sudo perf record -b
> 	- sudo perf report
> 
> The root cause is that bi[i].to.ms.maps does not always point to thread->maps,
> which is a buffer dynamically allocated by maps_new(). Instead, it may point to
> &machine->kmaps, while kmaps is not a pointer but a variable. The original
> upstream commit c1149037f65b ("perf hist: Add missing puts to
> hist__account_cycles") worked well because machine->kmaps had been refactored to
> a pointer by the previous commit 1a97cee604dc ("perf maps: Use a pointer for
> kmaps").
> 
> The memory leak issue, which the reverted patch intended to fix, has been solved
> by commit cf96b8e45a9b ("perf session: Add missing evlist__delete when deleting
> a session"). The root cause is that the evlist is not being deleted on exit in
> perf-report, perf-script, and perf-data. Consequently, the reference count of
> the thread increased by thread__get() in hist_entry__init() is not decremented
> in hist_entry__delete(). As a result, thread->maps is not properly freed.
> 
> To this end,
> 
> - PATCH 1/2 reverts commit a83fc293acd5c5050a4828eced4a71d2b2fffdd3 to fix the
>   abort regression.
> - PATCH 2/2 backports cf96b8e45a9b ("perf session: Add missing evlist__delete
>   when deleting a session") to fix memory leak issue.
> 
> Riccardo Mancini (1):
>   perf session: Add missing evlist__delete when deleting a session
> 
> Shuai Xue (1):
>   Revert "perf hist: Add missing puts to hist__account_cycles"
> 
>  tools/perf/util/hist.c    | 10 +++-------
>  tools/perf/util/session.c |  5 ++++-
>  2 files changed, 7 insertions(+), 8 deletions(-)

perf actually works and builds on this kernel tree?  That's news to me,
but hey, I'll take these now as obviously someone is still trying to run
it.

But why not just use the latest version of perf instead?

thanks,

greg k-h

在 2024/11/10 13:12, Greg KH 写道:
> On Mon, Nov 04, 2024 at 07:27:34PM +0800, Shuai Xue wrote:
>> On x86 platform, kernel v5.10.228, perf-report command aborts due to "free():
>> invalid pointer" when perf-record command is run with taken branch stack
>> sampling enabled. This regression can be reproduced with the following steps:
>>
>> 	- sudo perf record -b
>> 	- sudo perf report
>>
>> The root cause is that bi[i].to.ms.maps does not always point to thread->maps,
>> which is a buffer dynamically allocated by maps_new(). Instead, it may point to
>> &machine->kmaps, while kmaps is not a pointer but a variable. The original
>> upstream commit c1149037f65b ("perf hist: Add missing puts to
>> hist__account_cycles") worked well because machine->kmaps had been refactored to
>> a pointer by the previous commit 1a97cee604dc ("perf maps: Use a pointer for
>> kmaps").
>>
>> The memory leak issue, which the reverted patch intended to fix, has been solved
>> by commit cf96b8e45a9b ("perf session: Add missing evlist__delete when deleting
>> a session"). The root cause is that the evlist is not being deleted on exit in
>> perf-report, perf-script, and perf-data. Consequently, the reference count of
>> the thread increased by thread__get() in hist_entry__init() is not decremented
>> in hist_entry__delete(). As a result, thread->maps is not properly freed.
>>
>> To this end,
>>
>> - PATCH 1/2 reverts commit a83fc293acd5c5050a4828eced4a71d2b2fffdd3 to fix the
>>    abort regression.
>> - PATCH 2/2 backports cf96b8e45a9b ("perf session: Add missing evlist__delete
>>    when deleting a session") to fix memory leak issue.
>>
>> Riccardo Mancini (1):
>>    perf session: Add missing evlist__delete when deleting a session
>>
>> Shuai Xue (1):
>>    Revert "perf hist: Add missing puts to hist__account_cycles"
>>
>>   tools/perf/util/hist.c    | 10 +++-------
>>   tools/perf/util/session.c |  5 ++++-
>>   2 files changed, 7 insertions(+), 8 deletions(-)
> 
> perf actually works and builds on this kernel tree?  That's news to me,
> but hey, I'll take these now as obviously someone is still trying to run
> it.


Yes, it does. Commit cf96b8e45a9b ("perf session: Add missing evlist__delete
when deleting a session") addresses a memory leak issue but is not applicable
for the 5.10-stable tree.


> But why not just use the latest version of perf instead?

Yes, the lastest verison of perf works well. There are many distribution are
based on the upstream 5.10-stable tree, and this issue breaks the perf usage.
So IMHO, it could be fixed in upstream.

Best Regards,
Shuai