Re: [PATCH] powerpc/perf: Fix handling of privilege level checks in perf interrupt context

[Athira Rajeev <atrajeev@linux.vnet.ibm.com>]

> On 23-Feb-2021, at 6:24 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> 
> Peter Zijlstra <peterz@infradead.org> writes:
>> On Tue, Feb 23, 2021 at 01:31:49AM -0500, Athira Rajeev wrote:
>>> Running "perf mem record" in powerpc platforms with selinux enabled
>>> resulted in soft lockup's. Below call-trace was seen in the logs:
> ...
>>> 
>>> Since the purpose of this security hook is to control access to
>>> perf_event_open, it is not right to call this in interrupt context.
>>> But in case of powerpc PMU, we need the privilege checks for specific
>>> samples from branch history ring buffer and sampling register values.
>> 
>> I'm confused... why would you need those checks at event time? Either
>> the event has perf_event_attr::exclude_kernel and it then isn't allowed
>> to expose kernel addresses, or it doesn't and it is.
> 
> Well one of us is confused that's for sure ^_^
> 
> I missed/forgot that we had that logic in open.
> 
> I think the reason we got here is that in the past we didn't have the
> event in the low-level routines where we want to check,
> power_pmu_bhrb_read() and perf_get_data_addr(), so we hacked in a
> perf_paranoid_kernel() check. Which was wrong.
> 
> Then Joel's patch plumbed the event through and switched those paranoid
> checks to perf_allow_kernel().
> 
> Anyway, we'll just switch those to exclude_kernel checks.
> 
>> There should never be an event-time question of permission like this. If
>> you allow creation of an event, you're allowing the data it generates.
> 
> Ack.

Thanks for all the reviews. I will send a V2 with using 'event->attr.exclude_kernel' in the checks.

Athira 
> 
> cheers