linux-kernel - Re: [PATCH V6 07/14] perf/x86/intel: Generic support for hardware TopDown metrics

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200721142505.GK119549@hirez.programming.kicks-ass.net>
Date:   Tue, 21 Jul 2020 16:25:05 +0200
From:   peterz@...radead.org
To:     "Liang, Kan" <kan.liang@...ux.intel.com>
Cc:     acme@...hat.com, mingo@...nel.org, linux-kernel@...r.kernel.org,
        jolsa@...nel.org, eranian@...gle.com,
        alexander.shishkin@...ux.intel.com, ak@...ux.intel.com
Subject: Re: [PATCH V6 07/14] perf/x86/intel: Generic support for hardware
 TopDown metrics

On Tue, Jul 21, 2020 at 10:05:55AM -0400, Liang, Kan wrote:
> On 7/21/2020 5:43 AM, Peter Zijlstra wrote:
> > @@ -1098,37 +1105,20 @@ static int collect_events(struct cpu_hw_
> >   		cpuc->pebs_output = is_pebs_pt(leader) + 1;
> >   	}
> > -	if (x86_pmu.intel_cap.perf_metrics &&
> > -	    add_nr_metric_event(cpuc, leader, &max_count, false))
> > +	if (is_x86_event(leader) && collect_event(cpuc, leader, max_count, n))
> >   		return -EINVAL;
> > +	n++;
> 
> If a leader is not an x86 event, n will be mistakenly increased.
> But is it possible that a leader is not an x86 event here?

You're right, and yes that can happen, see the move_group=1 case in
sys_perf_event_open().

	if (is_x86_event(leader)) {
		if (collect_event(cpuc, leader, max_count, n))
			return -EINVAL;
		n++;
	}

it is then..