[PATCH v7 1/2] perf: Reveal PMU type in fdinfo

[PATCH v7 1/2] perf: Reveal PMU type in fdinfo

[Chun-Tse Shao]

It gives useful info on knowing which PMUs are reserved by this process.
Also add config which would be useful.
Testing cycles:

  $ ./perf stat -e cycles &
  $ cat /proc/`pidof perf`/fdinfo/3
  pos:    0
  flags:  02000002
  mnt_id: 16
  ino:    3081
  perf_event_attr.type:   0
  perf_event_attr.config: 0x0
  perf_event_attr.config1:        0x0
  perf_event_attr.config2:        0x0
  perf_event_attr.config3:        0x0

Testing L1-dcache-load-misses:

  $ ./perf stat -e L1-dcache-load-misses &
  $ cat /proc/`pidof perf`/fdinfo/3
  pos:    0
  flags:  02000002
  mnt_id: 16
  ino:    1072
  perf_event_attr.type:   3
  perf_event_attr.config: 0x10000
  perf_event_attr.config1:        0x0
  perf_event_attr.config2:        0x0
  perf_event_attr.config3:        0x0

Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Chun-Tse Shao <ctshao@google.com>
---
v7:
  Fix a leakage found by Sashiko.
  sashiko.dev/#/patchset/20260518174159.1177549-1-ctshao@google.com?part=2

v6: lore.kernel.org/20260518174159.1177549-1-ctshao@google.com/
  Rebase.

v5: lore.kernel.org/20251121230951.3240124-1-ctshao@google.com/
  Added config1, config2 and config3 and print output in hex.

v4:
  lore.kernel.org/20250603181634.1362626-1-ctshao@google.com/
  Removed the first patch in v3 since it is merged.
  Retested to make sure the patch still work.

v3: lore.kernel.org/20241106003007.2112584-3-ctshao@google.com/

 kernel/events/core.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 6d1f8bad7e1c..d88947e9f774 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -58,6 +58,7 @@
 #include <linux/percpu-rwsem.h>
 #include <linux/unwind_deferred.h>
 #include <linux/kvm_types.h>
+#include <linux/seq_file.h>

 #include "internal.h"

@@ -7506,6 +7507,20 @@ static int perf_fasync(int fd, struct file *filp, int on)
 	return 0;
 }

+static void perf_show_fdinfo(struct seq_file *m, struct file *f)
+{
+	struct perf_event *event = f->private_data;
+
+	seq_printf(m, "perf_event_attr.type:\t%u\n", event->orig_type);
+	seq_printf(m, "perf_event_attr.config:\t0x%llx\n", (unsigned long long)event->attr.config);
+	seq_printf(m, "perf_event_attr.config1:\t0x%llx\n",
+		   (unsigned long long)event->attr.config1);
+	seq_printf(m, "perf_event_attr.config2:\t0x%llx\n",
+		   (unsigned long long)event->attr.config2);
+	seq_printf(m, "perf_event_attr.config3:\t0x%llx\n",
+		   (unsigned long long)event->attr.config3);
+}
+
 static const struct file_operations perf_fops = {
 	.release		= perf_release,
 	.read			= perf_read,
@@ -7514,6 +7529,7 @@ static const struct file_operations perf_fops = {
 	.compat_ioctl		= perf_compat_ioctl,
 	.mmap			= perf_mmap,
 	.fasync			= perf_fasync,
+	.show_fdinfo		= perf_show_fdinfo,
 };

 /*
--
2.54.0.746.g67dd491aae-goog

[PATCH v7 2/2] perf evsel: Find process with busy PMUs for EBUSY

[Chun-Tse Shao]

It parses fdinfo with PMU type, comparing with the event which failed to
open, and report the processes causing EBUSY error.

Testing cycles and intel_pt//

  $ ./perf stat -e cycles &
  [1] 55569
  $ ./perf stat -e intel_pt// &
  [2] 55683
  $ ./perf stat -e intel_pt//
  Error:
  The PMU intel_pt counters are busy and in use by another process.
  Possible processes:
  55683 ./perf stat -e intel_pt//

Only perf with intel_pt was reported.

Reviewed-by: Ian Rogers <irogers@google.com>
Signed-off-by: Chun-Tse Shao <ctshao@google.com>
---
 tools/perf/util/evsel.c | 80 ++++++++++++++++++++++++++++++-----------
 1 file changed, 59 insertions(+), 21 deletions(-)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 2ee87fd84d3e..1f120c27e5d7 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -3928,7 +3928,8 @@ static bool find_process(const char *name)
 	return ret ? false : true;
 }
 
-static int dump_perf_event_processes(char *msg, size_t size)
+static int dump_perf_event_processes(const struct perf_event_attr *failed_attr,
+				     char *msg, size_t size)
 {
 	DIR *proc_dir;
 	struct dirent *proc_entry;
@@ -3969,29 +3970,64 @@ static int dump_perf_event_processes(char *msg, size_t size)
 				continue;
 			/* Take care as readlink doesn't null terminate the string. */
 			if (!strncmp(buf, "anon_inode:[perf_event]", link_size)) {
-				int cmdline_fd;
-				ssize_t cmdline_size;
-
-				scnprintf(buf, sizeof(buf), "%s/cmdline", proc_entry->d_name);
-				cmdline_fd = openat(dirfd(proc_dir), buf, O_RDONLY);
-				if (cmdline_fd == -1)
+				int fdinfo_fd;
+				ssize_t fdinfo_size;
+				char *line;
+				u32 perf_event_type = UINT32_MAX;
+
+				/* Let's check the PMU type reserved by this process */
+				scnprintf(buf, sizeof(buf), "%s/fdinfo/%s",
+					  proc_entry->d_name, fd_entry->d_name);
+				fdinfo_fd = openat(dirfd(proc_dir), buf, O_RDONLY);
+				if (fdinfo_fd == -1)
 					continue;
-				cmdline_size = read(cmdline_fd, buf, sizeof(buf) - 1);
-				close(cmdline_fd);
-				if (cmdline_size < 0)
+				fdinfo_size = read(fdinfo_fd, buf, sizeof(buf) - 1);
+				close(fdinfo_fd);
+				if (fdinfo_size < 0)
 					continue;
-				buf[cmdline_size] = '\0';
-				for (ssize_t i = 0; i < cmdline_size; i++) {
-					if (buf[i] == '\0')
-						buf[i] = ' ';
+				buf[fdinfo_size] = '\0';
+
+				line = strtok(buf, "\n");
+				while (line != NULL) {
+					if (sscanf(line,
+						   "perf_event_attr.type:\t%u",
+						   &perf_event_type) == 1)
+						break;
+					line = strtok(NULL, "\n");
 				}
 
-				if (printed == 0)
-					printed += scnprintf(msg, size, "Possible processes:\n");
-
-				printed += scnprintf(msg + printed, size - printed,
-						"%s %s\n", proc_entry->d_name, buf);
-				break;
+				/* Report the process which reserves the conflicted PMU. */
+				/* If fdinfo does not contain PMU type, report it too. */
+				if (perf_event_type == failed_attr->type ||
+				    perf_event_type == UINT32_MAX) {
+					int cmdline_fd;
+					ssize_t cmdline_size;
+
+					scnprintf(buf, sizeof(buf),
+						  "%s/cmdline",
+						  proc_entry->d_name);
+					cmdline_fd = openat(dirfd(proc_dir), buf, O_RDONLY);
+					if (cmdline_fd == -1)
+						continue;
+					cmdline_size = read(cmdline_fd, buf, sizeof(buf) - 1);
+					close(cmdline_fd);
+					if (cmdline_size < 0)
+						continue;
+					buf[cmdline_size] = '\0';
+					for (ssize_t i = 0; i < cmdline_size; i++) {
+						if (buf[i] == '\0')
+							buf[i] = ' ';
+					}
+
+					if (printed == 0)
+						printed += scnprintf(
+							msg, size,
+							"Possible processes:\n");
+
+					printed += scnprintf(msg + printed, size - printed,
+							"%s %s\n", proc_entry->d_name, buf);
+					break;
+				}
 			}
 		}
 		closedir(fd_dir);
@@ -4109,7 +4145,9 @@ int evsel__open_strerror(struct evsel *evsel, struct target *target,
 			msg, size,
 			"The PMU %s counters are busy and in use by another process.\n",
 			evsel->pmu ? evsel->pmu->name : "");
-		return printed + dump_perf_event_processes(msg + printed, size - printed);
+		return printed + dump_perf_event_processes(&evsel->core.attr,
+							   msg + printed,
+							   size - printed);
 		break;
 	case EINVAL:
 		if (evsel->core.attr.sample_type & PERF_SAMPLE_CODE_PAGE_SIZE && perf_missing_features.code_page_size)
-- 
2.54.0.746.g67dd491aae-goog

Re: [PATCH v7 1/2] perf: Reveal PMU type in fdinfo

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Printing `orig_type` in `fdinfo` fails to reveal the actual reserved PMU for generic event types, contrary to the commit message.
- [Medium] Data race and potential torn reads on `event->attr.config1` and `event->attr.config2` in `perf_show_fdinfo`.
- [Low] The `perf_event_attr` configuration dump in `fdinfo` is incomplete because it omits `config4`.
--

commit 2d51cc413225501aabea03532725af0cfbaf6950
Author: Chun-Tse Shao <ctshao@google.com>

perf: Reveal PMU type in fdinfo

This commit adds a show_fdinfo handler for perf event file descriptors.
It exposes the original perf event type and configuration values to
userspace via the fdinfo interface.

> It gives useful info on knowing which PMUs are reserved by this process.

Does printing the orig_type actually reveal which hardware PMU is reserved?

For generic events like PERF_TYPE_HARDWARE (0), orig_type will just be 0.
On heterogeneous systems with multiple PMUs, like hybrid architectures,
printing the requested type doesn't seem to reveal which specific physical
PMU was actually allocated by the kernel.

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c

[ ... ]

> @@ -7506,6 +7507,20 @@ static int perf_fasync(int fd, struct file *filp, int on)
> +static void perf_show_fdinfo(struct seq_file *m, struct file *f)
> +{
> +	struct perf_event *event = f->private_data;
> +
> +	seq_printf(m, "perf_event_attr.type:\t%u\n", event->orig_type);
> +	seq_printf(m, "perf_event_attr.config:\t0x%llx\n", (unsigned long long)event->attr.config);
> +	seq_printf(m, "perf_event_attr.config1:\t0x%llx\n",
> +		   (unsigned long long)event->attr.config1);
> +	seq_printf(m, "perf_event_attr.config2:\t0x%llx\n",
> +		   (unsigned long long)event->attr.config2);

Can reading event->attr.config1 and config2 locklessly lead to torn reads
on 32-bit architectures?

These fields share a union with bp_addr and bp_len, which can be modified
concurrently by ioctl(PERF_EVENT_IOC_MODIFY_ATTRIBUTES) via
perf_event_modify_breakpoint().

Because the fdinfo read path does not acquire the perf_event_ctx_lock used
during modification, could concurrent execution cause split 64-bit values to
be printed?

> +	seq_printf(m, "perf_event_attr.config3:\t0x%llx\n",
> +		   (unsigned long long)event->attr.config3);

Is it intentional to omit config4 here?

The perf_event_attr structure includes config4, so skipping it results in
an incomplete dump of the event configuration.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518224819.1247382-1-ctshao@google.com?part=1

Re: [PATCH v7 2/2] perf evsel: Find process with busy PMUs for EBUSY

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Use-After-Free and state corruption due to strtok() on a stack-allocated buffer.
- [Medium] Terminal injection vulnerability via unsanitized process command lines.
- [Low] Improper string comparison bound can lead to false positive process identification.
--

commit a4a94ff69c8d665f6d09eaea15fdbbbd89954b18
Author: Chun-Tse Shao <ctshao@google.com>

perf evsel: Find process with busy PMUs for EBUSY

This commit parses fdinfo to match the PMU type of a perf event that
failed to open with EBUSY, and reports the conflicting processes.

> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -3969,29 +3970,64 @@ static int dump_perf_event_processes(char *msg, size_t size)
>  				continue;
>  			/* Take care as readlink doesn't null terminate the string. */
>  			if (!strncmp(buf, "anon_inode:[perf_event]", link_size)) {

Is link_size the correct bound for this comparison?

If the symlink target is shorter than "anon_inode:[perf_event]" but exactly
matches the prefix (for example, if link_size is 4 and buf is "anon"), this
strncmp will return 0 and falsely identify the process. Should this also
check that link_size equals the exact length of the expected string?

[ ... ]

> +				buf[fdinfo_size] = '\0';
> +
> +				line = strtok(buf, "\n");
> +				while (line != NULL) {
> +					if (sscanf(line,
> +						   "perf_event_attr.type:\t%u",
> +						   &perf_event_type) == 1)
> +						break;
> +					line = strtok(NULL, "\n");
>  				}

buf is a stack-allocated array. Since strtok stores a pointer to the parsed
string in its internal static state, does breaking out of this loop early
leave the internal pointer pointing to popped stack memory?

If another part of the tool later calls strtok(NULL, ...) in the same thread,
could this cause a use-after-free or memory corruption as strtok writes null
bytes? Would it be safer to use strtok_r here to avoid clobbering the shared
parsing state?

[ ... ]

> +					cmdline_size = read(cmdline_fd, buf, sizeof(buf) - 1);
> +					close(cmdline_fd);
> +					if (cmdline_size < 0)
> +						continue;
> +					buf[cmdline_size] = '\0';
> +					for (ssize_t i = 0; i < cmdline_size; i++) {
> +						if (buf[i] == '\0')
> +							buf[i] = ' ';
> +					}
> +
> +					if (printed == 0)
> +						printed += scnprintf(
> +							msg, size,
> +							"Possible processes:\n");
> +
> +					printed += scnprintf(msg + printed, size - printed,
> +							"%s %s\n", proc_entry->d_name, buf);
> +					break;

Does this safely handle unprivileged process command lines?

While null bytes are replaced with spaces, other non-printable characters and
ANSI escape sequences are preserved. If a malicious user intentionally holds a
PMU and crafts a command line with escape sequences, could those sequences
execute in the terminal of the administrator running perf stat, potentially
allowing terminal output spoofing?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518224819.1247382-1-ctshao@google.com?part=2