linux-kernel - Re: [PATCH] perf/core: Improve arguments checking of inherited per-task counters when sampling.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aKRRSsEJxb1LZDV1@J2N7QTR9R3>
Date: Tue, 19 Aug 2025 11:26:18 +0100
From: Mark Rutland <mark.rutland@....com>
To: Chengdong Li <chengdongli@...imatist.com>
Cc: peterz@...radead.org, mingo@...hat.com, acme@...nel.org,
	namhyung@...nel.org, alexander.shishkin@...ux.intel.com,
	jolsa@...nel.org, irogers@...gle.com, adrian.hunter@...el.com,
	kan.liang@...ux.intel.com, linux-perf-users@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] perf/core: Improve arguments checking of inherited
 per-task counters when sampling.

On Thu, Aug 14, 2025 at 07:06:25PM +0800, Chengdong Li wrote:
> It's not allowed to mmap() of inherited per-task counters with CPU ==
> -1, this would create a performance issue. But it is not friendly to
> developers as current implementation postponed the arguments checking to
> perf_mmap(), developer can get an -EINVAL from mmap() but without
> any previous error returned from perf_event_open().
> 
> This patch improves it by moving the arguments checking from perf_mmap()
> to perf_event_open().

Why is that an improvement?

IIUC before this patch, it would be possible to read() the event,
whereas now the event cannot be opened at all.

AFAICT this is removing functionality people could legitimately use, so
this doesn't seem like an improvement.

Mark.

> 
> Signed-off-by: Chengdong Li <chengdongli@...imatist.com>
> ---
>  kernel/events/core.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 8060c2857bb2..f102adb395ec 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -6941,14 +6941,6 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
>  	int ret, flags = 0;
>  	mapped_f mapped;
>  
> -	/*
> -	 * Don't allow mmap() of inherited per-task counters. This would
> -	 * create a performance issue due to all children writing to the
> -	 * same rb.
> -	 */
> -	if (event->cpu == -1 && event->attr.inherit)
> -		return -EINVAL;
> -
>  	if (!(vma->vm_flags & VM_SHARED))
>  		return -EINVAL;
>  
> @@ -13392,6 +13384,18 @@ SYSCALL_DEFINE5(perf_event_open,
>  			return -EACCES;
>  	}
>  
> +	/*
> +	 * Don't allow perf_event_open() of inherited per-task counters
> +	 * with cpu == 1 when sampling. Otherwise, this would create a
> +	 * performance issue due to all children writing to the same mmap()
> +	 * created ring buffer.
> +	 *
> +	 * We recommend to call perf_event_open() for all cpus when sampling on
> +	 * inherited per-task counters.
> +	 */
> +	if (attr.sample_freq && attr.inherit && cpu == -1)
> +		return -EINVAL;
> +
>  	if (attr.freq) {
>  		if (attr.sample_freq > sysctl_perf_event_sample_rate)
>  			return -EINVAL;
> -- 
> 2.43.0
>