linux-kernel - Re: [PATCH v2 06/12] perf record: Switch user option to use BPF filter

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: <aAquj31djneyTwLG@x1>
Date: Thu, 24 Apr 2025 18:35:11 -0300
From: Arnaldo Carvalho de Melo <acme@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
	Namhyung Kim <namhyung@...nel.org>,
	Mark Rutland <mark.rutland@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Kan Liang <kan.liang@...ux.intel.com>,
	James Clark <james.clark@...aro.org>,
	Dapeng Mi <dapeng1.mi@...ux.intel.com>,
	Thomas Richter <tmricht@...ux.ibm.com>,
	Veronika Molnarova <vmolnaro@...hat.com>, Hao Ge <gehao@...inos.cn>,
	Howard Chu <howardchu95@...il.com>,
	Weilin Wang <weilin.wang@...el.com>, Levi Yun <yeoreum.yun@....com>,
	"Dr. David Alan Gilbert" <linux@...blig.org>,
	Dominique Martinet <asmadeus@...ewreck.org>,
	Xu Yang <xu.yang_2@....com>, Tengda Wu <wutengda@...weicloud.com>,
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
	bpf@...r.kernel.org
Subject: Re: [PATCH v2 06/12] perf record: Switch user option to use BPF
 filter

On Thu, Apr 10, 2025 at 10:36:25AM -0700, Ian Rogers wrote:
> Finding user processes by scanning /proc is inherently racy and
> results in perf_event_open failures. Use a BPF filter to drop samples
> where the uid doesn't match. Ensure adding the BPF filter forces
> system-wide.

Since the BPF filter is not introduced in this patch, can you please
provide, in the commit log message or in the patch itself, some
commentary as to how this is accomplished thru a BPF filter?

- Arnaldo
 
> Signed-off-by: Ian Rogers <irogers@...gle.com>
> ---
>  tools/perf/builtin-record.c | 27 ++++++++++++++++-----------
>  1 file changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index ba20bf7c011d..202c917fd122 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -173,6 +173,7 @@ struct record {
>  	bool			timestamp_boundary;
>  	bool			off_cpu;
>  	const char		*filter_action;
> +	const char		*uid_str;
>  	struct switch_output	switch_output;
>  	unsigned long long	samples;
>  	unsigned long		output_max_size;	/* = 0: unlimited */
> @@ -3460,8 +3461,7 @@ static struct option __record_options[] = {
>  		     "or ranges of time to enable events e.g. '-D 10-20,30-40'",
>  		     record__parse_event_enable_time),
>  	OPT_BOOLEAN(0, "kcore", &record.opts.kcore, "copy /proc/kcore"),
> -	OPT_STRING('u', "uid", &record.opts.target.uid_str, "user",
> -		   "user to profile"),
> +	OPT_STRING('u', "uid", &record.uid_str, "user", "user to profile"),
>  
>  	OPT_CALLBACK_NOOPT('b', "branch-any", &record.opts.branch_stack,
>  		     "branch any", "sample any taken branches",
> @@ -4196,19 +4196,24 @@ int cmd_record(int argc, const char **argv)
>  		ui__warning("%s\n", errbuf);
>  	}
>  
> -	err = target__parse_uid(&rec->opts.target);
> -	if (err) {
> -		int saved_errno = errno;
> +	if (rec->uid_str) {
> +		uid_t uid = parse_uid(rec->uid_str);
>  
> -		target__strerror(&rec->opts.target, err, errbuf, BUFSIZ);
> -		ui__error("%s", errbuf);
> +		if (uid == UINT_MAX) {
> +			ui__error("Invalid User: %s", rec->uid_str);
> +			err = -EINVAL;
> +			goto out;
> +		}
> +		err = parse_uid_filter(rec->evlist, uid);
> +		if (err)
> +			goto out;
>  
> -		err = -saved_errno;
> -		goto out;
> +		/* User ID filtering implies system wide. */
> +		rec->opts.target.system_wide = true;
>  	}
>  
> -	/* Enable ignoring missing threads when -u/-p option is defined. */
> -	rec->opts.ignore_missing_thread = rec->opts.target.uid != UINT_MAX || rec->opts.target.pid;
> +	/* Enable ignoring missing threads when -p option is defined. */
> +	rec->opts.ignore_missing_thread = rec->opts.target.pid;
>  
>  	evlist__warn_user_requested_cpus(rec->evlist, rec->opts.target.cpu_list);
>  
> -- 
> 2.49.0.604.gff1f9ca942-goog