| Message ID | a66d5648-2b8e-577e-e1f2-1d56c017ab5e@linux.intel.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Introduce CAP_PERFMON to secure system performance monitoring and observability | expand |
On Thu, Apr 02, 2020 at 11:47:35AM +0300, Alexey Budankov wrote: > > Extend error messages to mention CAP_PERFMON capability as an option > to substitute CAP_SYS_ADMIN capability for secure system performance > monitoring and observability operations. Make perf_event_paranoid_check() > and __cmd_ftrace() to be aware of CAP_PERFMON capability. > > CAP_PERFMON implements the principal of least privilege for performance > monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 > principle of least privilege: A security design principle that states > that a process or program be granted only those privileges (e.g., > capabilities) necessary to accomplish its legitimate function, and only > for the time that such privileges are actually required) > > For backward compatibility reasons access to perf_events subsystem remains > open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for > secure perf_events monitoring is discouraged with respect to CAP_PERFMON > capability. > > Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com> > Reviewed-by: James Morris <jamorris@linux.microsoft.com> Acked-by: Jiri Olsa <jolsa@redhat.com> thanks, jirka > --- > tools/perf/builtin-ftrace.c | 5 +++-- > tools/perf/design.txt | 3 ++- > tools/perf/util/cap.h | 4 ++++ > tools/perf/util/evsel.c | 10 +++++----- > tools/perf/util/util.c | 1 + > 5 files changed, 15 insertions(+), 8 deletions(-) > > diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c > index d5adc417a4ca..55eda54240fb 100644 > --- a/tools/perf/builtin-ftrace.c > +++ b/tools/perf/builtin-ftrace.c > @@ -284,10 +284,11 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv) > .events = POLLIN, > }; > > - if (!perf_cap__capable(CAP_SYS_ADMIN)) { > + if (!(perf_cap__capable(CAP_PERFMON) || > + perf_cap__capable(CAP_SYS_ADMIN))) { > pr_err("ftrace only works for %s!\n", > #ifdef HAVE_LIBCAP_SUPPORT > - "users with the SYS_ADMIN capability" > + "users with the CAP_PERFMON or CAP_SYS_ADMIN capability" > #else > "root" > #endif > diff --git a/tools/perf/design.txt b/tools/perf/design.txt > index 0453ba26cdbd..a42fab308ff6 100644 > --- a/tools/perf/design.txt > +++ b/tools/perf/design.txt > @@ -258,7 +258,8 @@ gets schedule to. Per task counters can be created by any user, for > their own tasks. > > A 'pid == -1' and 'cpu == x' counter is a per CPU counter that counts > -all events on CPU-x. Per CPU counters need CAP_SYS_ADMIN privilege. > +all events on CPU-x. Per CPU counters need CAP_PERFMON or CAP_SYS_ADMIN > +privilege. > > The 'flags' parameter is currently unused and must be zero. > > diff --git a/tools/perf/util/cap.h b/tools/perf/util/cap.h > index 051dc590ceee..ae52878c0b2e 100644 > --- a/tools/perf/util/cap.h > +++ b/tools/perf/util/cap.h > @@ -29,4 +29,8 @@ static inline bool perf_cap__capable(int cap __maybe_unused) > #define CAP_SYSLOG 34 > #endif > > +#ifndef CAP_PERFMON > +#define CAP_PERFMON 38 > +#endif > + > #endif /* __PERF_CAP_H */ > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > index 816d930d774e..2696922f06bc 100644 > --- a/tools/perf/util/evsel.c > +++ b/tools/perf/util/evsel.c > @@ -2507,14 +2507,14 @@ int perf_evsel__open_strerror(struct evsel *evsel, struct target *target, > "You may not have permission to collect %sstats.\n\n" > "Consider tweaking /proc/sys/kernel/perf_event_paranoid,\n" > "which controls use of the performance events system by\n" > - "unprivileged users (without CAP_SYS_ADMIN).\n\n" > + "unprivileged users (without CAP_PERFMON or CAP_SYS_ADMIN).\n\n" > "The current value is %d:\n\n" > " -1: Allow use of (almost) all events by all users\n" > " Ignore mlock limit after perf_event_mlock_kb without CAP_IPC_LOCK\n" > - ">= 0: Disallow ftrace function tracepoint by users without CAP_SYS_ADMIN\n" > - " Disallow raw tracepoint access by users without CAP_SYS_ADMIN\n" > - ">= 1: Disallow CPU event access by users without CAP_SYS_ADMIN\n" > - ">= 2: Disallow kernel profiling by users without CAP_SYS_ADMIN\n\n" > + ">= 0: Disallow ftrace function tracepoint by users without CAP_PERFMON or CAP_SYS_ADMIN\n" > + " Disallow raw tracepoint access by users without CAP_SYS_PERFMON or CAP_SYS_ADMIN\n" > + ">= 1: Disallow CPU event access by users without CAP_PERFMON or CAP_SYS_ADMIN\n" > + ">= 2: Disallow kernel profiling by users without CAP_PERFMON or CAP_SYS_ADMIN\n\n" > "To make this setting permanent, edit /etc/sysctl.conf too, e.g.:\n\n" > " kernel.perf_event_paranoid = -1\n" , > target->system_wide ? "system-wide " : "", > diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c > index d707c9624dd9..37a9492edb3e 100644 > --- a/tools/perf/util/util.c > +++ b/tools/perf/util/util.c > @@ -290,6 +290,7 @@ int perf_event_paranoid(void) > bool perf_event_paranoid_check(int max_level) > { > return perf_cap__capable(CAP_SYS_ADMIN) || > + perf_cap__capable(CAP_PERFMON) || > perf_event_paranoid() <= max_level; > } > > -- > 2.24.1 >
On 03.04.2020 14:08, Jiri Olsa wrote: > On Thu, Apr 02, 2020 at 11:47:35AM +0300, Alexey Budankov wrote: >> >> Extend error messages to mention CAP_PERFMON capability as an option >> to substitute CAP_SYS_ADMIN capability for secure system performance >> monitoring and observability operations. Make perf_event_paranoid_check() >> and __cmd_ftrace() to be aware of CAP_PERFMON capability. >> >> CAP_PERFMON implements the principal of least privilege for performance >> monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 >> principle of least privilege: A security design principle that states >> that a process or program be granted only those privileges (e.g., >> capabilities) necessary to accomplish its legitimate function, and only >> for the time that such privileges are actually required) >> >> For backward compatibility reasons access to perf_events subsystem remains >> open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for >> secure perf_events monitoring is discouraged with respect to CAP_PERFMON >> capability. >> >> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com> >> Reviewed-by: James Morris <jamorris@linux.microsoft.com> > > Acked-by: Jiri Olsa <jolsa@redhat.com> Thanks! I appreciate you support. ~Alexey > > thanks, > jirka > >> --- >> tools/perf/builtin-ftrace.c | 5 +++-- >> tools/perf/design.txt | 3 ++- >> tools/perf/util/cap.h | 4 ++++ >> tools/perf/util/evsel.c | 10 +++++----- >> tools/perf/util/util.c | 1 + >> 5 files changed, 15 insertions(+), 8 deletions(-) >> >> diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c >> index d5adc417a4ca..55eda54240fb 100644 >> --- a/tools/perf/builtin-ftrace.c >> +++ b/tools/perf/builtin-ftrace.c >> @@ -284,10 +284,11 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv) >> .events = POLLIN, >> }; >> >> - if (!perf_cap__capable(CAP_SYS_ADMIN)) { >> + if (!(perf_cap__capable(CAP_PERFMON) || >> + perf_cap__capable(CAP_SYS_ADMIN))) { >> pr_err("ftrace only works for %s!\n", >> #ifdef HAVE_LIBCAP_SUPPORT >> - "users with the SYS_ADMIN capability" >> + "users with the CAP_PERFMON or CAP_SYS_ADMIN capability" >> #else >> "root" >> #endif >> diff --git a/tools/perf/design.txt b/tools/perf/design.txt >> index 0453ba26cdbd..a42fab308ff6 100644 >> --- a/tools/perf/design.txt >> +++ b/tools/perf/design.txt >> @@ -258,7 +258,8 @@ gets schedule to. Per task counters can be created by any user, for >> their own tasks. >> >> A 'pid == -1' and 'cpu == x' counter is a per CPU counter that counts >> -all events on CPU-x. Per CPU counters need CAP_SYS_ADMIN privilege. >> +all events on CPU-x. Per CPU counters need CAP_PERFMON or CAP_SYS_ADMIN >> +privilege. >> >> The 'flags' parameter is currently unused and must be zero. >> >> diff --git a/tools/perf/util/cap.h b/tools/perf/util/cap.h >> index 051dc590ceee..ae52878c0b2e 100644 >> --- a/tools/perf/util/cap.h >> +++ b/tools/perf/util/cap.h >> @@ -29,4 +29,8 @@ static inline bool perf_cap__capable(int cap __maybe_unused) >> #define CAP_SYSLOG 34 >> #endif >> >> +#ifndef CAP_PERFMON >> +#define CAP_PERFMON 38 >> +#endif >> + >> #endif /* __PERF_CAP_H */ >> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c >> index 816d930d774e..2696922f06bc 100644 >> --- a/tools/perf/util/evsel.c >> +++ b/tools/perf/util/evsel.c >> @@ -2507,14 +2507,14 @@ int perf_evsel__open_strerror(struct evsel *evsel, struct target *target, >> "You may not have permission to collect %sstats.\n\n" >> "Consider tweaking /proc/sys/kernel/perf_event_paranoid,\n" >> "which controls use of the performance events system by\n" >> - "unprivileged users (without CAP_SYS_ADMIN).\n\n" >> + "unprivileged users (without CAP_PERFMON or CAP_SYS_ADMIN).\n\n" >> "The current value is %d:\n\n" >> " -1: Allow use of (almost) all events by all users\n" >> " Ignore mlock limit after perf_event_mlock_kb without CAP_IPC_LOCK\n" >> - ">= 0: Disallow ftrace function tracepoint by users without CAP_SYS_ADMIN\n" >> - " Disallow raw tracepoint access by users without CAP_SYS_ADMIN\n" >> - ">= 1: Disallow CPU event access by users without CAP_SYS_ADMIN\n" >> - ">= 2: Disallow kernel profiling by users without CAP_SYS_ADMIN\n\n" >> + ">= 0: Disallow ftrace function tracepoint by users without CAP_PERFMON or CAP_SYS_ADMIN\n" >> + " Disallow raw tracepoint access by users without CAP_SYS_PERFMON or CAP_SYS_ADMIN\n" >> + ">= 1: Disallow CPU event access by users without CAP_PERFMON or CAP_SYS_ADMIN\n" >> + ">= 2: Disallow kernel profiling by users without CAP_PERFMON or CAP_SYS_ADMIN\n\n" >> "To make this setting permanent, edit /etc/sysctl.conf too, e.g.:\n\n" >> " kernel.perf_event_paranoid = -1\n" , >> target->system_wide ? "system-wide " : "", >> diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c >> index d707c9624dd9..37a9492edb3e 100644 >> --- a/tools/perf/util/util.c >> +++ b/tools/perf/util/util.c >> @@ -290,6 +290,7 @@ int perf_event_paranoid(void) >> bool perf_event_paranoid_check(int max_level) >> { >> return perf_cap__capable(CAP_SYS_ADMIN) || >> + perf_cap__capable(CAP_PERFMON) || >> perf_event_paranoid() <= max_level; >> } >> >> -- >> 2.24.1 >> >
Hello, On Thu, Apr 2, 2020 at 5:47 PM Alexey Budankov <alexey.budankov@linux.intel.com> wrote: > > > Extend error messages to mention CAP_PERFMON capability as an option > to substitute CAP_SYS_ADMIN capability for secure system performance > monitoring and observability operations. Make perf_event_paranoid_check() > and __cmd_ftrace() to be aware of CAP_PERFMON capability. > > CAP_PERFMON implements the principal of least privilege for performance > monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 > principle of least privilege: A security design principle that states > that a process or program be granted only those privileges (e.g., > capabilities) necessary to accomplish its legitimate function, and only > for the time that such privileges are actually required) > > For backward compatibility reasons access to perf_events subsystem remains > open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for > secure perf_events monitoring is discouraged with respect to CAP_PERFMON > capability. > > Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com> > Reviewed-by: James Morris <jamorris@linux.microsoft.com> Acked-by: Namhyung Kim <namhyung@kernel.org> Thanks Namhyung
Hi Namhyung, On 04.04.2020 5:18, Namhyung Kim wrote: > Hello, > > On Thu, Apr 2, 2020 at 5:47 PM Alexey Budankov > <alexey.budankov@linux.intel.com> wrote: >> >> >> Extend error messages to mention CAP_PERFMON capability as an option >> to substitute CAP_SYS_ADMIN capability for secure system performance >> monitoring and observability operations. Make perf_event_paranoid_check() >> and __cmd_ftrace() to be aware of CAP_PERFMON capability. >> >> CAP_PERFMON implements the principal of least privilege for performance >> monitoring and observability operations (POSIX IEEE 1003.1e 2.2.2.39 >> principle of least privilege: A security design principle that states >> that a process or program be granted only those privileges (e.g., >> capabilities) necessary to accomplish its legitimate function, and only >> for the time that such privileges are actually required) >> >> For backward compatibility reasons access to perf_events subsystem remains >> open for CAP_SYS_ADMIN privileged processes but CAP_SYS_ADMIN usage for >> secure perf_events monitoring is discouraged with respect to CAP_PERFMON >> capability. >> >> Signed-off-by: Alexey Budankov <alexey.budankov@linux.intel.com> >> Reviewed-by: James Morris <jamorris@linux.microsoft.com> > > Acked-by: Namhyung Kim <namhyung@kernel.org> Thanks! I appreciate you involvement and effort. ~Alexey > > Thanks > Namhyung >
diff --git a/tools/perf/builtin-ftrace.c b/tools/perf/builtin-ftrace.c index d5adc417a4ca..55eda54240fb 100644 --- a/tools/perf/builtin-ftrace.c +++ b/tools/perf/builtin-ftrace.c @@ -284,10 +284,11 @@ static int __cmd_ftrace(struct perf_ftrace *ftrace, int argc, const char **argv) .events = POLLIN, }; - if (!perf_cap__capable(CAP_SYS_ADMIN)) { + if (!(perf_cap__capable(CAP_PERFMON) || + perf_cap__capable(CAP_SYS_ADMIN))) { pr_err("ftrace only works for %s!\n", #ifdef HAVE_LIBCAP_SUPPORT - "users with the SYS_ADMIN capability" + "users with the CAP_PERFMON or CAP_SYS_ADMIN capability" #else "root" #endif diff --git a/tools/perf/design.txt b/tools/perf/design.txt index 0453ba26cdbd..a42fab308ff6 100644 --- a/tools/perf/design.txt +++ b/tools/perf/design.txt @@ -258,7 +258,8 @@ gets schedule to. Per task counters can be created by any user, for their own tasks. A 'pid == -1' and 'cpu == x' counter is a per CPU counter that counts -all events on CPU-x. Per CPU counters need CAP_SYS_ADMIN privilege. +all events on CPU-x. Per CPU counters need CAP_PERFMON or CAP_SYS_ADMIN +privilege. The 'flags' parameter is currently unused and must be zero. diff --git a/tools/perf/util/cap.h b/tools/perf/util/cap.h index 051dc590ceee..ae52878c0b2e 100644 --- a/tools/perf/util/cap.h +++ b/tools/perf/util/cap.h @@ -29,4 +29,8 @@ static inline bool perf_cap__capable(int cap __maybe_unused) #define CAP_SYSLOG 34 #endif +#ifndef CAP_PERFMON +#define CAP_PERFMON 38 +#endif + #endif /* __PERF_CAP_H */ diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index 816d930d774e..2696922f06bc 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -2507,14 +2507,14 @@ int perf_evsel__open_strerror(struct evsel *evsel, struct target *target, "You may not have permission to collect %sstats.\n\n" "Consider tweaking /proc/sys/kernel/perf_event_paranoid,\n" "which controls use of the performance events system by\n" - "unprivileged users (without CAP_SYS_ADMIN).\n\n" + "unprivileged users (without CAP_PERFMON or CAP_SYS_ADMIN).\n\n" "The current value is %d:\n\n" " -1: Allow use of (almost) all events by all users\n" " Ignore mlock limit after perf_event_mlock_kb without CAP_IPC_LOCK\n" - ">= 0: Disallow ftrace function tracepoint by users without CAP_SYS_ADMIN\n" - " Disallow raw tracepoint access by users without CAP_SYS_ADMIN\n" - ">= 1: Disallow CPU event access by users without CAP_SYS_ADMIN\n" - ">= 2: Disallow kernel profiling by users without CAP_SYS_ADMIN\n\n" + ">= 0: Disallow ftrace function tracepoint by users without CAP_PERFMON or CAP_SYS_ADMIN\n" + " Disallow raw tracepoint access by users without CAP_SYS_PERFMON or CAP_SYS_ADMIN\n" + ">= 1: Disallow CPU event access by users without CAP_PERFMON or CAP_SYS_ADMIN\n" + ">= 2: Disallow kernel profiling by users without CAP_PERFMON or CAP_SYS_ADMIN\n\n" "To make this setting permanent, edit /etc/sysctl.conf too, e.g.:\n\n" " kernel.perf_event_paranoid = -1\n" , target->system_wide ? "system-wide " : "", diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c index d707c9624dd9..37a9492edb3e 100644 --- a/tools/perf/util/util.c +++ b/tools/perf/util/util.c @@ -290,6 +290,7 @@ int perf_event_paranoid(void) bool perf_event_paranoid_check(int max_level) { return perf_cap__capable(CAP_SYS_ADMIN) || + perf_cap__capable(CAP_PERFMON) || perf_event_paranoid() <= max_level; }