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

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] [day] [month] [year] [list]

Message-ID: <7a0fc6f4-f185-4d5e-b532-c9e04b0a48c4@linux.alibaba.com>
Date: Sun, 10 Nov 2024 18:26:30 +0800
From: Shuai Xue <xueshuai@...ux.alibaba.com>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: stable@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-perf-users@...r.kernel.org, acme@...nel.org, adrian.hunter@...el.com,
 alexander.shishkin@...ux.intel.com, irogers@...gle.com,
 mark.rutland@....com, namhyung@...nel.org, peterz@...radead.org,
 acme@...hat.com, kprateek.nayak@....com, ravi.bangoria@....com,
 sandipan.das@....com, anshuman.khandual@....com, german.gomez@....com,
 james.clark@....com, terrelln@...com, seanjc@...gle.com,
 changbin.du@...wei.com, liuwenyu7@...wei.com, yangjihong1@...wei.com,
 mhiramat@...nel.org, ojeda@...nel.org, song@...nel.org, leo.yan@...aro.org,
 kjain@...ux.ibm.com, ak@...ux.intel.com, kan.liang@...ux.intel.com,
 atrajeev@...ux.vnet.ibm.com, siyanteng@...ngson.cn, liam.howlett@...cle.com,
 pbonzini@...hat.com, jolsa@...nel.org
Subject: Re: [PATCH 5.10.y 0/2] Fixed perf abort when taken branch stack
 sampling enabled



在 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