From: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> To: Igor Lubashev <ilubashe@akamai.com> Cc: linux-kernel@vger.kernel.org, Jiri Olsa <jolsa@redhat.com>, Alexey Budankov <alexey.budankov@linux.intel.com>, Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>, Mathieu Poirier <mathieu.poirier@linaro.org>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Namhyung Kim <namhyung@kernel.org>, Suzuki K Poulose <suzuki.poulose@arm.com>, linux-arm-kernel@lists.infradead.org, James Morris <jmorris@namei.org> Subject: Re: [PATCH v3 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks Date: Mon, 12 Aug 2019 17:01:34 -0300 [thread overview] Message-ID: <20190812200134.GE9280@kernel.org> (raw) In-Reply-To: <ad56df5452eeafb99dda9fc3d30f0f487aace503.1565188228.git.ilubashe@akamai.com> Em Wed, Aug 07, 2019 at 10:44:15AM -0400, Igor Lubashev escreveu: > +++ b/tools/perf/util/evsel.c > @@ -279,7 +279,7 @@ struct 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); > } While looking at your changes I think the pre-existing code is wrong, i.e. the check in sys_perf_event_open(), in the kernel is: if (!attr.exclude_kernel) { if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN)) return -EACCES; } And: static inline bool perf_paranoid_kernel(void) { return sysctl_perf_event_paranoid > 1; } So we have to change that perf_event_paranoit_check(-1) to pass 1 instead? bool perf_event_paranoid_check(int max_level) { return perf_cap__capable(CAP_SYS_ADMIN) || perf_event_paranoid() <= max_level; } Also you defined perf_cap__capable(anything) as: #ifdef HAVE_LIBCAP_SUPPORT #include <sys/capability.h> bool perf_cap__capable(cap_value_t cap); #else static inline bool perf_cap__capable(int cap __maybe_unused) { return false; } #endif /* HAVE_LIBCAP_SUPPORT */ I think we should have: #else static inline bool perf_cap__capable(int cap __maybe_unused) { return geteuid() == 0; } #endif /* HAVE_LIBCAP_SUPPORT */ Right? So I am removing the introduction of perf_cap__capable() from the first patch you sent, leaving it with _only_ the feature detection part, using that feature detection to do anything is then moved to a separate patch, after we finish this discussion about what we should fallback to when libcap-devel isn't available, i.e. we should use the previous checks, etc. - Arnaldo
WARNING: multiple messages have this Message-ID (diff)
From: Arnaldo Carvalho de Melo <acme@kernel.org> To: Igor Lubashev <ilubashe@akamai.com> Cc: Mathieu Poirier <mathieu.poirier@linaro.org>, Suzuki K Poulose <suzuki.poulose@arm.com>, Peter Zijlstra <peterz@infradead.org>, Alexey Budankov <alexey.budankov@linux.intel.com>, linux-kernel@vger.kernel.org, James Morris <jmorris@namei.org>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Ingo Molnar <mingo@redhat.com>, Namhyung Kim <namhyung@kernel.org>, Jiri Olsa <jolsa@redhat.com>, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v3 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks Date: Mon, 12 Aug 2019 17:01:34 -0300 [thread overview] Message-ID: <20190812200134.GE9280@kernel.org> (raw) In-Reply-To: <ad56df5452eeafb99dda9fc3d30f0f487aace503.1565188228.git.ilubashe@akamai.com> Em Wed, Aug 07, 2019 at 10:44:15AM -0400, Igor Lubashev escreveu: > +++ b/tools/perf/util/evsel.c > @@ -279,7 +279,7 @@ struct 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); > } While looking at your changes I think the pre-existing code is wrong, i.e. the check in sys_perf_event_open(), in the kernel is: if (!attr.exclude_kernel) { if (perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN)) return -EACCES; } And: static inline bool perf_paranoid_kernel(void) { return sysctl_perf_event_paranoid > 1; } So we have to change that perf_event_paranoit_check(-1) to pass 1 instead? bool perf_event_paranoid_check(int max_level) { return perf_cap__capable(CAP_SYS_ADMIN) || perf_event_paranoid() <= max_level; } Also you defined perf_cap__capable(anything) as: #ifdef HAVE_LIBCAP_SUPPORT #include <sys/capability.h> bool perf_cap__capable(cap_value_t cap); #else static inline bool perf_cap__capable(int cap __maybe_unused) { return false; } #endif /* HAVE_LIBCAP_SUPPORT */ I think we should have: #else static inline bool perf_cap__capable(int cap __maybe_unused) { return geteuid() == 0; } #endif /* HAVE_LIBCAP_SUPPORT */ Right? So I am removing the introduction of perf_cap__capable() from the first patch you sent, leaving it with _only_ the feature detection part, using that feature detection to do anything is then moved to a separate patch, after we finish this discussion about what we should fallback to when libcap-devel isn't available, i.e. we should use the previous checks, etc. - Arnaldo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-08-12 20:01 UTC|newest] Thread overview: 63+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-08-07 14:44 [PATCH v3 0/4] perf: Use capabilities instead of uid and euid Igor Lubashev 2019-08-07 14:44 ` Igor Lubashev 2019-08-07 14:44 ` [PATCH v3 1/4] perf: Add capability-related utilities Igor Lubashev 2019-08-07 14:44 ` Igor Lubashev 2019-08-12 19:43 ` Arnaldo Carvalho de Melo 2019-08-12 19:43 ` Arnaldo Carvalho de Melo 2019-08-15 9:24 ` [tip:perf/core] tools build: Add capability-related feature detection tip-bot for Igor Lubashev 2019-08-15 9:25 ` [tip:perf/core] perf tools: Add helpers to use capabilities if present tip-bot for Igor Lubashev 2019-08-07 14:44 ` [PATCH v3 2/4] perf: Use CAP_SYS_ADMIN with perf_event_paranoid checks Igor Lubashev 2019-08-07 14:44 ` Igor Lubashev 2019-08-12 20:01 ` Arnaldo Carvalho de Melo [this message] 2019-08-12 20:01 ` Arnaldo Carvalho de Melo 2019-08-12 20:15 ` Arnaldo Carvalho de Melo 2019-08-12 20:15 ` Arnaldo Carvalho de Melo 2019-08-12 22:33 ` Lubashev, Igor 2019-08-12 22:33 ` Lubashev, Igor 2019-08-13 13:20 ` Arnaldo Carvalho de Melo 2019-08-13 13:20 ` Arnaldo Carvalho de Melo 2019-08-07 14:44 ` [PATCH v3 3/4] perf: Use CAP_SYSLOG with kptr_restrict checks Igor Lubashev 2019-08-07 14:44 ` Igor Lubashev 2019-08-14 18:04 ` Mathieu Poirier 2019-08-14 18:04 ` Mathieu Poirier 2019-08-14 18:48 ` Arnaldo Carvalho de Melo 2019-08-14 18:48 ` Arnaldo Carvalho de Melo 2019-08-14 18:52 ` Arnaldo Carvalho de Melo 2019-08-14 18:52 ` Arnaldo Carvalho de Melo 2019-08-14 20:02 ` Lubashev, Igor 2019-08-14 20:02 ` Lubashev, Igor 2019-08-15 15:01 ` Mathieu Poirier 2019-08-15 15:01 ` Mathieu Poirier 2019-08-15 20:16 ` Mathieu Poirier 2019-08-15 20:16 ` Mathieu Poirier 2019-08-15 21:42 ` Arnaldo Carvalho de Melo 2019-08-15 21:42 ` Arnaldo Carvalho de Melo 2019-08-19 16:51 ` Mathieu Poirier 2019-08-19 16:51 ` Mathieu Poirier 2019-08-19 22:22 ` Lubashev, Igor 2019-08-19 22:22 ` Lubashev, Igor 2019-08-20 16:57 ` Mathieu Poirier 2019-08-20 16:57 ` Mathieu Poirier 2019-08-20 17:13 ` Arnaldo Carvalho de Melo 2019-08-20 17:13 ` Arnaldo Carvalho de Melo 2019-08-27 1:58 ` Lubashev, Igor 2019-08-27 1:58 ` Lubashev, Igor 2019-08-15 22:27 ` Lubashev, Igor 2019-08-15 22:27 ` Lubashev, Igor 2019-08-07 14:44 ` [PATCH v3 4/4] perf: Use CAP_SYS_ADMIN instead of euid==0 with ftrace Igor Lubashev 2019-08-07 14:44 ` Igor Lubashev 2019-08-12 20:22 ` Arnaldo Carvalho de Melo 2019-08-12 20:22 ` Arnaldo Carvalho de Melo 2019-08-12 20:27 ` Arnaldo Carvalho de Melo 2019-08-12 20:27 ` Arnaldo Carvalho de Melo 2019-08-12 20:29 ` Arnaldo Carvalho de Melo 2019-08-12 20:29 ` Arnaldo Carvalho de Melo 2019-08-12 21:42 ` Mathieu Poirier 2019-08-12 21:42 ` Mathieu Poirier 2019-08-13 13:23 ` Arnaldo Carvalho de Melo 2019-08-13 13:23 ` Arnaldo Carvalho de Melo 2019-08-13 16:35 ` Mathieu Poirier 2019-08-13 16:35 ` Mathieu Poirier 2019-08-15 9:27 ` [tip:perf/core] perf ftrace: Use CAP_SYS_ADMIN instead of euid==0 tip-bot for Igor Lubashev 2019-08-12 9:13 ` [PATCH v3 0/4] perf: Use capabilities instead of uid and euid Jiri Olsa 2019-08-12 9:13 ` Jiri Olsa
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190812200134.GE9280@kernel.org \ --to=arnaldo.melo@gmail.com \ --cc=alexander.shishkin@linux.intel.com \ --cc=alexey.budankov@linux.intel.com \ --cc=ilubashe@akamai.com \ --cc=jmorris@namei.org \ --cc=jolsa@redhat.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mathieu.poirier@linaro.org \ --cc=mingo@redhat.com \ --cc=namhyung@kernel.org \ --cc=peterz@infradead.org \ --cc=suzuki.poulose@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.