Re: [PATCH 5.10.y 0/2] Fixed perf abort when taken branch stack sampling enabled

[Shuai Xue <xueshuai@linux.alibaba.com>]

在 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