Messages in this thread | | | Subject | Re: [RFC 1/4] perf: Move some access checks later in perf_event_open | From | Alexey Budankov <> | Date | Tue, 26 Jun 2018 20:24:54 +0300 |
| |
Hi,
On 26.06.2018 18:36, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > To enable per-PMU access controls in a following patch first move all call > sites of perf_paranoid_kernel() to after the event has been created. > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> > Cc: Jiri Olsa <jolsa@redhat.com> > Cc: Namhyung Kim <namhyung@kernel.org> > Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com> > Cc: Andi Kleen <ak@linux.intel.com> > Cc: Alexey Budankov <alexey.budankov@linux.intel.com> > Cc: linux-kernel@vger.kernel.org > Cc: x86@kernel.org > --- > kernel/events/core.c | 36 ++++++++++++++++++++++-------------- > 1 file changed, 22 insertions(+), 14 deletions(-) > > diff --git a/kernel/events/core.c b/kernel/events/core.c > index f490caca9aa4..12de95b0472e 100644 > --- a/kernel/events/core.c > +++ b/kernel/events/core.c > @@ -10189,10 +10189,6 @@ static int perf_copy_attr(struct perf_event_attr __user *uattr, > */ > attr->branch_sample_type = mask; > } > - /* privileged levels capture (kernel, hv): check permissions */ > - if ((mask & PERF_SAMPLE_BRANCH_PERM_PLM) > - && perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN)) > - return -EACCES; > } > > if (attr->sample_type & PERF_SAMPLE_REGS_USER) { > @@ -10409,11 +10405,6 @@ SYSCALL_DEFINE5(perf_event_open, > if (err) > return err; > > - if (!attr.exclude_kernel) { > - if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN)) > - return -EACCES; > - } > - > if (attr.namespaces) { > if (!capable(CAP_SYS_ADMIN)) > return -EACCES; > @@ -10427,11 +10418,6 @@ SYSCALL_DEFINE5(perf_event_open, > return -EINVAL; > } > > - /* Only privileged users can get physical addresses */ > - if ((attr.sample_type & PERF_SAMPLE_PHYS_ADDR) && > - perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN)) > - return -EACCES; > - > /* > * In cgroup mode, the pid argument is used to pass the fd > * opened to the cgroup directory in cgroupfs. The cpu argument > @@ -10501,6 +10487,28 @@ SYSCALL_DEFINE5(perf_event_open, > goto err_cred; > } > > + if (!attr.exclude_kernel) { > + if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN)) { > + err = -EACCES;
I would separate this combined permissions check into a function e.g. static bool perf_test_pmu_paranoid(const struct pmu *pmu, int *err) to avoid code duplication.
> + goto err_alloc; > + } > + } > + > + /* Only privileged users can get physical addresses */ > + if ((attr.sample_type & PERF_SAMPLE_PHYS_ADDR) && > + perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN)) { > + err = -EACCES; > + goto err_alloc; > + } > + > + /* privileged levels capture (kernel, hv): check permissions */ > + if ((attr.sample_type & PERF_SAMPLE_BRANCH_STACK) && > + (attr.branch_sample_type & PERF_SAMPLE_BRANCH_PERM_PLM) && > + perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN)) { > + err = -EACCES; > + goto err_alloc; > + } > + > if (is_sampling_event(event)) { > if (event->pmu->capabilities & PERF_PMU_CAP_NO_INTERRUPT) { > err = -EOPNOTSUPP; >
| |