On 23-Feb-2021, at 6:24 PM, Michael Ellerman [off-list ref] wrote:

Peter Zijlstra [off-list ref] writes:

quoted

On Tue, Feb 23, 2021 at 01:31:49AM -0500, Athira Rajeev wrote:

quoted

Running "perf mem record" in powerpc platforms with selinux enabled
resulted in soft lockup's. Below call-trace was seen in the logs:

...

quoted

quoted

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.

quoted

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