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

[sashiko-bot@kernel.org]

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