| Message ID | 1562112605-6235-3-git-send-email-ilubashe@akamai.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | perf: Use capabilities instead of uid and euid | expand |
On Tue, Jul 02, 2019 at 08:10:04PM -0400, Igor Lubashev wrote: > The kernel is using CAP_SYS_ADMIN instead of euid==0 to override > perf_event_paranoid check. Make perf do the same. I see another geteuid check in __cmd_ftrace, perhaps we should cover this one as well jirka
I could add another patch to the series for that. Any suggestion for what capability to check for here? (There is always an alternative to not check for anything and let the kernel refuse to perform actions that the user does not have permissions to perform.) - Igor -----Original Message----- From: Jiri Olsa <jolsa@redhat.com> Sent: Tuesday, July 16, 2019 4:48 AM Subject: Re: [PATCH 2/3] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks On Tue, Jul 02, 2019 at 08:10:04PM -0400, Igor Lubashev wrote: > The kernel is using CAP_SYS_ADMIN instead of euid==0 to override > perf_event_paranoid check. Make perf do the same. I see another geteuid check in __cmd_ftrace, perhaps we should cover this one as well jirka
On Tue, Jul 16, 2019 at 05:01:26PM +0000, Lubashev, Igor wrote: > I could add another patch to the series for that. Any suggestion for what capability to check for here? it's: if (geteuid() != 0) { pr_err("ftrace only works for root!\n"); return -1 } so I think check for CAP_SYS_ADMIN should be fine in here jirka > > (There is always an alternative to not check for anything and let the kernel refuse to perform actions that the user does not have permissions to perform.) > > - Igor > > -----Original Message----- > From: Jiri Olsa <jolsa@redhat.com> > Sent: Tuesday, July 16, 2019 4:48 AM > Subject: Re: [PATCH 2/3] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks > > On Tue, Jul 02, 2019 at 08:10:04PM -0400, Igor Lubashev wrote: > > The kernel is using CAP_SYS_ADMIN instead of euid==0 to override > > perf_event_paranoid check. Make perf do the same. > > I see another geteuid check in __cmd_ftrace, > perhaps we should cover this one as well > > jirka
> On Wednesday, July 17, 2019 3:10 AM Jiri Olsa wrote: > On Tue, Jul 16, 2019 at 05:01:26PM +0000, Lubashev, Igor wrote: > > I could add another patch to the series for that. Any suggestion for what > capability to check for here? > > it's: > > if (geteuid() != 0) { > pr_err("ftrace only works for root!\n"); > return -1 > } > > so I think check for CAP_SYS_ADMIN should be fine in here Thanks. Added the [PATCH 4/3] to this series (https://lore.kernel.org/lkml/1563387359-27694-1-git-send-email-ilubashe@akamai.com/). Let me know if you'd rather I reroll a V2 of this series. - Igor > > jirka > > > > > (There is always an alternative to not check for anything and let the kernel > refuse to perform actions that the user does not have permissions to perform.) > > > > - Igor > > > > -----Original Message----- > > From: Jiri Olsa <jolsa@redhat.com> > > Sent: Tuesday, July 16, 2019 4:48 AM > > Subject: Re: [PATCH 2/3] perf: Use CAP_SYS_ADMIN with perf_event_paranoid > checks > > > > On Tue, Jul 02, 2019 at 08:10:04PM -0400, Igor Lubashev wrote: > > > The kernel is using CAP_SYS_ADMIN instead of euid==0 to override > > > perf_event_paranoid check. Make perf do the same. > > > > I see another geteuid check in __cmd_ftrace, > > perhaps we should cover this one as well > > > > jirka
diff --git a/tools/perf/arch/arm/util/cs-etm.c b/tools/perf/arch/arm/util/cs-etm.c index 911426721170..e004ba7ad957 100644 --- a/tools/perf/arch/arm/util/cs-etm.c +++ b/tools/perf/arch/arm/util/cs-etm.c @@ -17,6 +17,7 @@ #include "../../perf.h" #include "../../util/auxtrace.h" #include "../../util/cpumap.h" +#include "../../util/event.h" #include "../../util/evlist.h" #include "../../util/evsel.h" #include "../../util/pmu.h" @@ -106,7 +107,7 @@ static int cs_etm_recording_options(struct auxtrace_record *itr, struct perf_pmu *cs_etm_pmu = ptr->cs_etm_pmu; struct perf_evsel *evsel, *cs_etm_evsel = NULL; const struct cpu_map *cpus = evlist->cpus; - bool privileged = (geteuid() == 0 || perf_event_paranoid() < 0); + bool privileged = perf_event_paranoid_check(-1); ptr->evlist = evlist; ptr->snapshot_mode = opts->auxtrace_snapshot_mode; diff --git a/tools/perf/arch/arm64/util/arm-spe.c b/tools/perf/arch/arm64/util/arm-spe.c index 5ccfce87e693..f5ec6953c69c 100644 --- a/tools/perf/arch/arm64/util/arm-spe.c +++ b/tools/perf/arch/arm64/util/arm-spe.c @@ -11,6 +11,7 @@ #include <time.h> #include "../../util/cpumap.h" +#include "../../util/event.h" #include "../../util/evsel.h" #include "../../util/evlist.h" #include "../../util/session.h" @@ -65,7 +66,7 @@ static int arm_spe_recording_options(struct auxtrace_record *itr, container_of(itr, struct arm_spe_recording, itr); struct perf_pmu *arm_spe_pmu = sper->arm_spe_pmu; struct perf_evsel *evsel, *arm_spe_evsel = NULL; - bool privileged = geteuid() == 0 || perf_event_paranoid() < 0; + bool privileged = perf_event_paranoid_check(-1); struct perf_evsel *tracking_evsel; int err; diff --git a/tools/perf/arch/x86/util/intel-bts.c b/tools/perf/arch/x86/util/intel-bts.c index e6d4d9591c79..fe7cecdb494d 100644 --- a/tools/perf/arch/x86/util/intel-bts.c +++ b/tools/perf/arch/x86/util/intel-bts.c @@ -11,6 +11,7 @@ #include <linux/log2.h> #include "../../util/cpumap.h" +#include "../../util/event.h" #include "../../util/evsel.h" #include "../../util/evlist.h" #include "../../util/session.h" @@ -107,7 +108,7 @@ static int intel_bts_recording_options(struct auxtrace_record *itr, struct perf_pmu *intel_bts_pmu = btsr->intel_bts_pmu; struct perf_evsel *evsel, *intel_bts_evsel = NULL; const struct cpu_map *cpus = evlist->cpus; - bool privileged = geteuid() == 0 || perf_event_paranoid() < 0; + bool privileged = perf_event_paranoid_check(-1); btsr->evlist = evlist; btsr->snapshot_mode = opts->auxtrace_snapshot_mode; diff --git a/tools/perf/arch/x86/util/intel-pt.c b/tools/perf/arch/x86/util/intel-pt.c index 1869f62a10cd..44d2194fdab3 100644 --- a/tools/perf/arch/x86/util/intel-pt.c +++ b/tools/perf/arch/x86/util/intel-pt.c @@ -557,7 +557,7 @@ static int intel_pt_recording_options(struct auxtrace_record *itr, bool have_timing_info, need_immediate = false; struct perf_evsel *evsel, *intel_pt_evsel = NULL; const struct cpu_map *cpus = evlist->cpus; - bool privileged = geteuid() == 0 || perf_event_paranoid() < 0; + bool privileged = perf_event_paranoid_check(-1); u64 tsc_bit; int err; diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index 4a5947625c5c..ce28d890d6bf 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -277,7 +277,7 @@ struct perf_evsel *perf_evsel__new_idx(struct perf_event_attr *attr, int idx) static bool perf_event_can_profile_kernel(void) { - return geteuid() == 0 || perf_event_paranoid() == -1; + return perf_event_paranoid_check(-1); } struct perf_evsel *perf_evsel__new_cycles(bool precise)
The kernel is using CAP_SYS_ADMIN instead of euid==0 to override perf_event_paranoid check. Make perf do the same. Signed-off-by: Igor Lubashev <ilubashe@akamai.com> --- tools/perf/arch/arm/util/cs-etm.c | 3 ++- tools/perf/arch/arm64/util/arm-spe.c | 3 ++- tools/perf/arch/x86/util/intel-bts.c | 3 ++- tools/perf/arch/x86/util/intel-pt.c | 2 +- tools/perf/util/evsel.c | 2 +- 5 files changed, 8 insertions(+), 5 deletions(-)