linux-kernel - Re: [PATCH v2 01/11] perf,x86: Fix event/group validation

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 for Android: free password hash cracker in your pocket

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

Date:	Tue, 26 May 2015 05:25:59 -0700
From:	Stephane Eranian <eranian@...gle.com>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	Ingo Molnar <mingo@...nel.org>,
	Vince Weaver <vincent.weaver@...ne.edu>,
	Jiri Olsa <jolsa@...hat.com>,
	"Liang, Kan" <kan.liang@...el.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Andrew Hunter <ahh@...gle.com>,
	Maria Dimakopoulou <maria.n.dimakopoulou@...il.com>
Subject: Re: [PATCH v2 01/11] perf,x86: Fix event/group validation

On Tue, May 26, 2015 at 5:16 AM, Peter Zijlstra <peterz@...radead.org> wrote:
> On Tue, May 26, 2015 at 04:46:07AM -0700, Stephane Eranian wrote:
>> On Tue, May 26, 2015 at 3:12 AM, Peter Zijlstra <peterz@...radead.org> wrote:
>> > On Tue, May 26, 2015 at 02:24:38AM -0700, Stephane Eranian wrote:
>> >> On Fri, May 22, 2015 at 6:40 AM, Peter Zijlstra <peterz@...radead.org> wrote:
>> >> > On Fri, May 22, 2015 at 03:29:06PM +0200, Peter Zijlstra wrote:
>> >> >> @@ -788,9 +788,9 @@ int x86_schedule_events(struct cpu_hw_ev
>> >> >>               x86_pmu.start_scheduling(cpuc);
>> >> >>
>> >> >>       for (i = 0, wmin = X86_PMC_IDX_MAX, wmax = 0; i < n; i++) {
>> >> >> +             cpuc->event_constraint[i] = NULL;
>> >> >
>
>> But where is the code that says: skip reinstalling the constraint
>> in intel_get_event_constraints() because there is already a (stale)
>> one? I don't see where that is.
>
> IIRC the problem was that the copy from c2 into c1:
>
>         if (c1 && (c1->flags & PERF_X86_EVENT_DYNAMIC)) {
>                 bitmap_copy(c1->idxmsk, c2->idxmsk, X86_PMC_IDX_MAX);
>                 c1->weight = c2->weight;
>                 c2 = c1;
>         }
>
> is incomplete. For instance, flags is not copied, and some code down the
> line might check that and get wrong flags.
>
Ok, now I remember this code. It has to do with incremental scheduling.
Suppose E1, E2, E3 events where E1 is exclusive. The first call is
for scheduling E1. It gets to get_event_constraint() which "allocates" a
dynamic constraint. The second call  tries to schedule E1, E2. But the
second time for E1, you already have the dynamic constraint allocated, so
this code is reusing the constraint storage and just updates the bitmask
and weight.

Now, that the storage is not actually dynamic (kmalloc'd), but taken from a
fixed size array in cpuc, I believe we can simplify this and "re-allocate"
the constraint for each incremental call to intel_get_event_constraints().
Do you agree?


> I'm not entirely sure I saw misbehaviour, but I figured I'd better close
> that hole and rule out this is contributing to fail.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/