linux-kernel - Re: [PATCH V2 0/7] arm64/perf: Enable branch stack sampling

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: <14c0b04b-f9f0-632c-4813-f1952e3320bc@arm.com>
Date:   Tue, 13 Sep 2022 17:42:52 +0530
From:   Anshuman Khandual <anshuman.khandual@....com>
To:     James Clark <james.clark@....com>
Cc:     Rob Herring <robh@...nel.org>, Marc Zyngier <maz@...nel.org>,
        Ingo Molnar <mingo@...hat.com>, linux-kernel@...r.kernel.org,
        linux-perf-users@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, peterz@...radead.org,
        acme@...nel.org, mark.rutland@....com, will@...nel.org,
        catalin.marinas@....com
Subject: Re: [PATCH V2 0/7] arm64/perf: Enable branch stack sampling



On 9/13/22 16:25, James Clark wrote:
> 
> On 08/09/2022 06:10, Anshuman Khandual wrote:
>> This series enables perf branch stack sampling support on arm64 platform
>> via a new arch feature called Branch Record Buffer Extension (BRBE). All
>> relevant register definitions could be accessed here.
>>
>> https://developer.arm.com/documentation/ddi0601/2021-12/AArch64-Registers
>>
>> This series applies on v6.0-rc4 after the BRBE related perf ABI changes series
>> (V7) that was posted earlier, and a branch sample filter helper patch.
>>
>> https://lore.kernel.org/all/20220824044822.70230-1-anshuman.khandual@arm.com/
>>
>> https://lore.kernel.org/all/20220906084414.396220-1-anshuman.khandual@arm.com/
>>
>> Following issues have been resolved
>>
>> - Jame's concerns regarding permission inadequacy related to perfmon_capable()
>> - Jame's concerns regarding using perf_event_paranoid along with perfmon_capable()
> I don't see the resolution to this one. I'm not 100% sure of the code
> path used for LBR, but I think you just need to take perf_allow_kernel()
> into account somewhere to make this command have the same result with
> BRBE. Is there any contention that the permissions shouldn't behave in
> the same way across platforms? This is when perf_event_paranoid < 2:
> 
> Intel:
> 
>   $ perf record -j any -- ls
> 
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.014 MB perf.data (16 samples) ]
> 
> Arm:
> 
>   $ perf record -j any -- ls
> 
>   Error:
>   No permission to enable cycles event.
> 
Proposed solution here just follows what we did for the SPE driver recently.
I would not be surprised, if there is difference in semantics in permission
checking across various platform perf drivers. Ideally permission should not
even be checked in platform drivers - either capability or perf_event_paranoid.

Unfortunately changing the permission checking framework across generic perf
is beyond the scope for this BRBE proposal and might be taken up later via a
different series. Although I would be willing to accommodate any alternate
suggestions to improve permission checking here in the BRBE driver.