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: > > CPU: 58 PID: 3751 Comm: sssd_nss Not tainted 5.11.0-rc7+ #2 > NIP: c000000000dff3d4 LR: c000000000dff3d0 CTR: 0000000000000000 > REGS: c000007fffab7d60 TRAP: 0100 Not tainted (5.11.0-rc7+) > <<>> > NIP [c000000000dff3d4] _raw_spin_lock_irqsave+0x94/0x120 > LR [c000000000dff3d0] _raw_spin_lock_irqsave+0x90/0x120 > Call Trace: > [c00000000fd471a0] [c00000000fd47260] 0xc00000000fd47260 (unreliable) > [c00000000fd471e0] [c000000000b5fbbc] skb_queue_tail+0x3c/0x90 > [c00000000fd47220] [c000000000296edc] audit_log_end+0x6c/0x180 > [c00000000fd47260] [c0000000006a3f20] common_lsm_audit+0xb0/0xe0 > [c00000000fd472a0] [c00000000066c664] slow_avc_audit+0xa4/0x110 > [c00000000fd47320] [c00000000066cff4] avc_has_perm+0x1c4/0x260 > [c00000000fd47430] [c00000000066e064] selinux_perf_event_open+0x74/0xd0 > [c00000000fd47450] [c000000000669888] security_perf_event_open+0x68/0xc0 > [c00000000fd47490] [c00000000013d788] record_and_restart+0x6e8/0x7f0 > [c00000000fd476c0] [c00000000013dabc] perf_event_interrupt+0x22c/0x560 > [c00000000fd477d0] [c00000000002d0fc] performance_monitor_exception+0x4c/0x60 > [c00000000fd477f0] [c00000000000b378] > performance_monitor_common_virt+0x1c8/0x1d0 > interrupt: f00 at _raw_spin_lock_irqsave+0x38/0x120 > NIP: c000000000dff378 LR: c000000000b5fbbc CTR: c0000000007d47f0 > REGS: c00000000fd47860 TRAP: 0f00 Not tainted (5.11.0-rc7+) > <<>> > NIP [c000000000dff378] _raw_spin_lock_irqsave+0x38/0x120 > LR [c000000000b5fbbc] skb_queue_tail+0x3c/0x90 > interrupt: f00 > [c00000000fd47b00] [0000000000000038] 0x38 (unreliable) > [c00000000fd47b40] [c00000000aae6200] 0xc00000000aae6200 > [c00000000fd47b80] [c000000000296edc] audit_log_end+0x6c/0x180 > [c00000000fd47bc0] [c00000000029f494] audit_log_exit+0x344/0xf80 > [c00000000fd47d10] [c0000000002a2b00] __audit_syscall_exit+0x2c0/0x320 > [c00000000fd47d60] [c000000000032878] do_syscall_trace_leave+0x148/0x200 > [c00000000fd47da0] [c00000000003d5b4] syscall_exit_prepare+0x324/0x390 > [c00000000fd47e10] [c00000000000d76c] system_call_common+0xfc/0x27c > > The above trace shows that while the CPU was handling a performance > monitor exception, there was a call to "security_perf_event_open" > function. In powerpc core-book3s, this function is called from > 'perf_allow_kernel' check during recording of data address in the sample > via perf_get_data_addr(). > > Commit da97e18458fb ("perf_event: Add support for LSM and SELinux checks") > introduced security enhancements to perf. As part of this commit, the new > security hook for perf_event_open was added in all places where perf > paranoid check was previously used. In powerpc core-book3s code, originally > had paranoid checks in 'perf_get_data_addr' and 'power_pmu_bhrb_read'. So > 'perf_paranoid_kernel' checks were replaced with 'perf_allow_kernel' in > these pmu helper functions as well. > > The intention of paranoid checks in core-book3s is to verify privilege > access before capturing some of the sample data. Along with paranoid > checks, 'perf_allow_kernel' also does a 'security_perf_event_open'. Since > these functions are accessed while recording sample, we end up in calling > selinux_perf_event_open in PMI context. Some of the security functions > use spinlock like sidtab_sid2str_put(). If a perf interrupt hits under > a spin lock and if we end up in calling selinux hook functions in PMI > handler, this could cause a dead lock. > > 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. 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.