[PATCH testsuite 0/3] perf_event: fixes and cleanups

[PATCH testsuite 0/3] perf_event: fixes and cleanups

[Ondrej Mosnacek]

See individual patches for details.

Ondrej Mosnacek (3):
  perf_event: measure CPU 0 rather than CPU 1
  perf_event: fix wrong use of perf_event_open(2) API
  perf_event: fix CAP_SYS_ADMIN references

 tests/perf_event/perf_event.c | 18 +++++++++++-----
 tests/perf_event/test         | 39 ++++++++++++++++++++++-------------
 2 files changed, 38 insertions(+), 19 deletions(-)

-- 
2.29.2

[PATCH testsuite 1/3] perf_event: measure CPU 0 rather than CPU 1

[Ondrej Mosnacek]

Change the "cpu" argument of perf_event_open(2) to 0, so that the test
doesn't fail on single-core machines.

This makes the perf_event pass in the GH Actions CI.

Fixes: b9e1eb2db689 ("selinux-testsuite: Add perf_event tests")
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 tests/perf_event/perf_event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/perf_event/perf_event.c b/tests/perf_event/perf_event.c
index 8983f02..1557303 100644
--- a/tests/perf_event/perf_event.c
+++ b/tests/perf_event/perf_event.c
@@ -82,7 +82,7 @@ int main(int argc, char **argv)
 	pe_attr.disabled = 1;
 	pe_attr.exclude_hv = 1;
 
-	fd = perf_event_open(&pe_attr, -1, 1, -1, 0);
+	fd = perf_event_open(&pe_attr, -1, 0, -1, 0);
 	if (fd < 0) {
 		fprintf(stderr, "Failed perf_event_open(): %s\n",
 			strerror(errno));
-- 
2.29.2

[PATCH testsuite 2/3] perf_event: fix wrong use of perf_event_open(2) API

[Ondrej Mosnacek]

1. attr.type is not a bit field, but an enum. Use PERF_TYPE_TRACEPOINT,
   which is equivalent to current code (PERF_TYPE_HARDWARE == 0).
2. attr.config for PERF_TYPE_TRACEPOINT is the ID of the trace event to
   monitor; PERF_COUNT_HW_INSTRUCTIONS is meaningful only for
   PERF_TYPE_HARDWARE. Replace it by the ID of "ftrace:function"
   obtained from tracefs. "ftrace:function" usually has the ID of 1,
   which is the numeric value of PERF_COUNT_HW_INSTRUCTIONS, so this
   should be functionally equivalent.

Fixes: b9e1eb2db689 ("selinux-testsuite: Add perf_event tests")
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 tests/perf_event/perf_event.c | 16 ++++++++++++----
 tests/perf_event/test         | 29 ++++++++++++++++++++---------
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/tests/perf_event/perf_event.c b/tests/perf_event/perf_event.c
index 1557303..453aa91 100644
--- a/tests/perf_event/perf_event.c
+++ b/tests/perf_event/perf_event.c
@@ -20,8 +20,9 @@ enum {
 static void print_usage(char *progname)
 {
 	fprintf(stderr,
-		"usage:  %s [-f|-m] [-v]\n"
+		"usage:  %s [-f|-m] [-v] EVENT_ID\n"
 		"Where:\n\t"
+		"EVENT_ID  target ftrace event ID\n\n\t"
 		"-f  Read perf_event info using read(2)\n\t"
 		"-m  Read perf_event info using mmap(2)\n\t"
 		"    Default is to use read(2) and mmap(2)\n\t"
@@ -39,7 +40,7 @@ static long perf_event_open(struct perf_event_attr *hw_event, pid_t pid,
 int main(int argc, char **argv)
 {
 	int opt, result, page_size, mmap_size, fd;
-	long long count;
+	long long count, event_id;
 	bool verbose = false;
 	char *context;
 	void *base;
@@ -64,6 +65,13 @@ int main(int argc, char **argv)
 		}
 	}
 
+	if ((argc - optind) != 1)
+		print_usage(argv[0]);
+
+	event_id = atoll(argv[optind]);
+	if (event_id < 0)
+		print_usage(argv[0]);
+
 	if (verbose) {
 		result = getcon(&context);
 		if (result < 0) {
@@ -76,9 +84,9 @@ int main(int argc, char **argv)
 
 	/* Test perf_event { open cpu kernel tracepoint } */
 	memset(&pe_attr, 0, sizeof(struct perf_event_attr));
-	pe_attr.type = PERF_TYPE_HARDWARE | PERF_TYPE_TRACEPOINT;
+	pe_attr.type = PERF_TYPE_TRACEPOINT;
 	pe_attr.size = sizeof(struct perf_event_attr);
-	pe_attr.config = PERF_COUNT_HW_INSTRUCTIONS;
+	pe_attr.config = event_id;
 	pe_attr.disabled = 1;
 	pe_attr.exclude_hv = 1;
 
diff --git a/tests/perf_event/test b/tests/perf_event/test
index 1d337e9..561a634 100755
--- a/tests/perf_event/test
+++ b/tests/perf_event/test
@@ -48,45 +48,56 @@ BEGIN {
     plan tests => $test_count;
 }
 
+$event_id = `cat /sys/kernel/debug/tracing/events/ftrace/function/id`;
+chomp($event_id);
+
 # perf_event { open cpu kernel tracepoint read write };
 print "Test perf_event\n";
-$result = system "runcon -t test_perf_t $basedir/perf_event $v";
+$result = system "runcon -t test_perf_t $basedir/perf_event $v $event_id";
 ok( $result eq 0 );
 
 if ($sys_admin) {
 
     # Deny capability { sys_admin } - EACCES perf_event_open(2)
-    $result = system "runcon -t test_perf_no_cap_t $basedir/perf_event $v 2>&1";
+    $result = system
+      "runcon -t test_perf_no_cap_t $basedir/perf_event $v $event_id 2>&1";
     ok( $result >> 8 eq 1 );
 }
 
 # Deny perf_event { open } - EACCES perf_event_open(2)
-$result = system "runcon -t test_perf_no_open_t $basedir/perf_event $v 2>&1";
+$result =
+  system "runcon -t test_perf_no_open_t $basedir/perf_event $v $event_id 2>&1";
 ok( $result >> 8 eq 1 );
 
 # Deny perf_event { cpu } - EACCES perf_event_open(2)
-$result = system "runcon -t test_perf_no_cpu_t $basedir/perf_event $v 2>&1";
+$result =
+  system "runcon -t test_perf_no_cpu_t $basedir/perf_event $v $event_id 2>&1";
 ok( $result >> 8 eq 1 );
 
 # Deny perf_event { kernel } - EACCES perf_event_open(2)
-$result = system "runcon -t test_perf_no_kernel_t $basedir/perf_event $v 2>&1";
+$result = system
+  "runcon -t test_perf_no_kernel_t $basedir/perf_event $v $event_id 2>&1";
 ok( $result >> 8 eq 1 );
 
 # Deny perf_event { tracepoint } - EACCES perf_event_open(2)
 $result =
-  system "runcon -t test_perf_no_tracepoint_t $basedir/perf_event $v 2>&1";
+  system
+  "runcon -t test_perf_no_tracepoint_t $basedir/perf_event $v $event_id 2>&1";
 ok( $result >> 8 eq 1 );
 
 # Deny perf_event { read } - EACCES mmap(2)
-$result = system "runcon -t test_perf_no_read_t $basedir/perf_event -m $v 2>&1";
+$result = system
+  "runcon -t test_perf_no_read_t $basedir/perf_event -m $v $event_id 2>&1";
 ok( $result >> 8 eq 6 );
 
 # Deny perf_event { read } - EACCES read(2)
-$result = system "runcon -t test_perf_no_read_t $basedir/perf_event -f $v 2>&1";
+$result = system
+  "runcon -t test_perf_no_read_t $basedir/perf_event -f $v $event_id 2>&1";
 ok( $result >> 8 eq 7 );
 
 # Deny perf_event { write } - EACCES ioctl(2) write
-$result = system "runcon -t test_perf_no_write_t $basedir/perf_event $v 2>&1";
+$result =
+  system "runcon -t test_perf_no_write_t $basedir/perf_event $v $event_id 2>&1";
 ok( $result >> 8 eq 2 );
 
 exit;
-- 
2.29.2

[PATCH testsuite 3/3] perf_event: fix CAP_SYS_ADMIN references

[Ondrej Mosnacek]

Fix the references to CAP_SYS_ADMIN in comments and variable names to
refer to CAP_PERFMON or just "capability" to avoid confusion. As of
commit 259751492b58 ("perf_event: update perf_event test for the new
CAP_PERFMON capability"), CAP_PERFMON is tested as the main capability
that gates access to perf events, rather than CAP_SYS_ADMIN, which is
now used only as a compatibility fallback.

Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
---
 tests/perf_event/test | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/perf_event/test b/tests/perf_event/test
index 561a634..380d9cc 100755
--- a/tests/perf_event/test
+++ b/tests/perf_event/test
@@ -6,7 +6,7 @@ BEGIN {
     $basedir =~ s|(.*)/[^/]*|$1|;
 
     $test_count = 8;
-    $sys_admin  = 0;
+    $capability = 0;
 
     # allow info to be shown during tests
     $v = $ARGV[0];
@@ -21,9 +21,9 @@ BEGIN {
 
     $level = `cat /proc/sys/kernel/perf_event_paranoid`;
     chomp($level);
-    if ( $level >= 2 ) {    # These tests require CAP_SYS_ADMIN
+    if ( $level >= 2 ) {    # These tests require CAP_PERFMON
         $test_count += 1;
-        $sys_admin = 1;
+        $capability = 1;
     }
 
     if ( $v eq "-v" ) {
@@ -56,9 +56,9 @@ print "Test perf_event\n";
 $result = system "runcon -t test_perf_t $basedir/perf_event $v $event_id";
 ok( $result eq 0 );
 
-if ($sys_admin) {
+if ($capability) {
 
-    # Deny capability { sys_admin } - EACCES perf_event_open(2)
+    # Deny capability { perfmon } - EACCES perf_event_open(2)
     $result = system
       "runcon -t test_perf_no_cap_t $basedir/perf_event $v $event_id 2>&1";
     ok( $result >> 8 eq 1 );
-- 
2.29.2

Re: [PATCH testsuite 0/3] perf_event: fixes and cleanups

[Ondrej Mosnacek]

On Tue, Mar 2, 2021 at 6:21 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
>
> See individual patches for details.
>
> Ondrej Mosnacek (3):
>   perf_event: measure CPU 0 rather than CPU 1
>   perf_event: fix wrong use of perf_event_open(2) API
>   perf_event: fix CAP_SYS_ADMIN references
>
>  tests/perf_event/perf_event.c | 18 +++++++++++-----
>  tests/perf_event/test         | 39 ++++++++++++++++++++++-------------
>  2 files changed, 38 insertions(+), 19 deletions(-)
>
> --
> 2.29.2

These are now merged as well, and we have a green CI again \o/  (At
least in my fork; CI on upstream master is still running.)

-- 
Ondrej Mosnacek
Software Engineer, Linux Security - SELinux kernel
Red Hat, Inc.

Re: [PATCH testsuite 0/3] perf_event: fixes and cleanups

[Paul Moore]

On Thu, Mar 4, 2021 at 5:35 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> On Tue, Mar 2, 2021 at 6:21 PM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> >
> > See individual patches for details.
> >
> > Ondrej Mosnacek (3):
> >   perf_event: measure CPU 0 rather than CPU 1
> >   perf_event: fix wrong use of perf_event_open(2) API
> >   perf_event: fix CAP_SYS_ADMIN references
> >
> >  tests/perf_event/perf_event.c | 18 +++++++++++-----
> >  tests/perf_event/test         | 39 ++++++++++++++++++++++-------------
> >  2 files changed, 38 insertions(+), 19 deletions(-)
> >
> > --
> > 2.29.2
>
> These are now merged as well, and we have a green CI again \o/  (At
> least in my fork; CI on upstream master is still running.)

Things are looking good on both my x86_64 and aarch64 test systems too - thanks!

-- 
paul moore
www.paul-moore.com