linux-kernel - Re: [PATCH v3] perf: map pages in advance

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

Message-ID: <2cb3d067-fe92-4bdf-ae79-b24810a4bc2e@redhat.com>
Date: Mon, 23 Dec 2024 22:12:42 +0100
From: David Hildenbrand <david@...hat.com>
To: Lorenzo Stoakes <lorenzo.stoakes@...cle.com>,
 Peter Zijlstra <peterz@...radead.org>
Cc: "Chen, Zide" <zide.chen@...el.com>, Ingo Molnar <mingo@...hat.com>,
 Arnaldo Carvalho de Melo <acme@...nel.org>,
 Namhyung Kim <namhyung@...nel.org>, Mark Rutland <mark.rutland@....com>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Jiri Olsa <jolsa@...nel.org>, Ian Rogers <irogers@...gle.com>,
 Adrian Hunter <adrian.hunter@...el.com>,
 Kan Liang <kan.liang@...ux.intel.com>, linux-perf-users@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-mm@...ck.org,
 Matthew Wilcox <willy@...radead.org>, Yi Lai <yi1.lai@...ux.intel.com>
Subject: Re: [PATCH v3] perf: map pages in advance

On 23.12.24 12:10, Lorenzo Stoakes wrote:
> Peter - could you drop this patch for now until I have a chance to take a
> look at this issue on my return on 2nd Jan?
> 
> On Fri, Dec 20, 2024 at 10:53:14PM +0100, David Hildenbrand wrote:
>> On 20.12.24 22:29, David Hildenbrand wrote:
>>> On 20.12.24 20:36, Chen, Zide wrote:
>>>>
>>>>
>>>> On 12/20/2024 1:56 AM, David Hildenbrand wrote:
>>>>> On 20.12.24 10:31, Lorenzo Stoakes wrote:
>>>>>> On Thu, Dec 19, 2024 at 01:17:44PM -0800, Chen, Zide wrote:
>>>>>>
>>>>>>> With this patch, it seems perf tool has some problems in capturing the
>>>>>>> kernel data with Intel PT.
>>>>>>>
>>>>>>> Running the following commands, the size of perf.data is very small, and
>>>>>>> perf script can't find any valid records.
>>>>>>>
>>>>>>> perf record -e intel_pt//u -- /bin/ls
>>>>>>> perf script --insn-trace
>>>>>>>
>>>>>>>
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> I'm on leave (and should really go back to relaxing :>), returning on 2nd
>>>>>> Jan so can't really dig into this.
>>>>>>
>>>>>> But I tried it on my intel box and it 'works on my machine' with and
>>>>>> without patch with commands provided, so I'm not sure this is actually a
>>>>>> product of this change (which shouldn't impact this).
>>>>>
>>>>> Zide Chen, can you try with and without this patch to see if it
>>>>> introduces the issue?
>>>>
>>>> Yes, I re-did the test on a SPR server, and the result is same. Without
>>>> the patch, it went well; But with it, "perf script --insn-trace" doesn't
>>>> show valid records.
>>>>
>>>> This time I tested it on the clean 6.13-rc1 tag, base commit
>>>> 40384c840ea1944d7c5a392e8975ed088ecf0b37
>>>>
>>>> Also, with this patch, running tools/perf/tests/shell/test_intel_pt.sh:
>>>>
>>>> Error:
>>>> The - data has no samples!
>>>
>>> I just tested it on 6.13-rc1  vs. 6.13-rc1  with this patch.
>>>
>>> Indeed, there is quite difference. Below are the main parts that changed, only.
>>>
>>> We seem to be recording data, but maybe what we record gets corrupted somehow?
>>
>> Huge parts of the new file are full of 0s. Either we are mapping the wrong
>> pages, or reading from the pages (via PFNMAP) does not work as expected.
>>
> 
> Thanks David, and apologies Zide, appears there is an issue here clearly.
> 
> Could you try this with sudo operations? I was doing this locally and I
> wonder if there is now a permissioning error?

I ran it under root.

> 
> I'd be surprised if pfn map would cause an issue here as it should just
> directly map the kernel memory, however if the PT code assumes there will
> be faults there could be an issue. I did take a brief look at this last
> week and it seems the PT stuff relies on the aux functionality, so that
> could also be a source of problems here.

I started a bit at that code, no clue yet what's happening.

I was wondering if we end up mapping the wrong pages, meaning: the pages 
at mmap time end up being different to the pages later at fault time. 
The code is a bit confusing, but I thought we cannot change the 
effective event/pages while we have an active mmap. Maybe there is some 
corner case ...

Nothing else really jumped at me ... moving the mapping og pages after 
the event_mapped() callback also didn't change anything.

> 
> I am on leave at the moment returning on 2nd Jan, I will look at this as a
> priority when I return, as you can see above I've asked Peter to drop this
> for now.

Enjoy your time off an Happy Holidays!

-- 
Cheers,

David / dhildenb