Re: perf: fuzzer triggered warning in intel_pmu_drain_pebs_nhm()

[Peter Zijlstra <peterz@infradead.org>]

On Thu, Jul 16, 2015 at 08:02:03AM +0200, Stephane Eranian wrote:
> Been running it for a couple of hours, so far so good. I will let it
> run all night.

Thanks!

> > ---
> >  arch/x86/kernel/cpu/perf_event_intel_ds.c | 29 +++++++++++++----------------
> >  1 file changed, 13 insertions(+), 16 deletions(-)
> >
> > diff --git a/arch/x86/kernel/cpu/perf_event_intel_ds.c b/arch/x86/kernel/cpu/perf_event_intel_ds.c
> > index 71fc40238843..68d0ced1d229 100644
> > --- a/arch/x86/kernel/cpu/perf_event_intel_ds.c
> > +++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c
> > @@ -1142,6 +1142,7 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
> >
> >         for (at = base; at < top; at += x86_pmu.pebs_record_size) {
> >                 struct pebs_record_nhm *p = at;
> > +               u64 pebs_status;
> >
> >                 /* PEBS v3 has accurate status bits */
> >                 if (x86_pmu.intel_cap.pebs_format >= 3) {
> > @@ -1152,12 +1153,14 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
> >                         continue;
> >                 }
> >
> > -               bit = find_first_bit((unsigned long *)&p->status,
> > +               pebs_status = p->status & cpuc->pebs_enabled;
> > +               pebs_status &= (1ULL << x86_pmu.max_pebs_events) - 1;
> > +
> > +               bit = find_first_bit((unsigned long *)&pebs_status,
> >                                         x86_pmu.max_pebs_events);
> >                 if (bit >= x86_pmu.max_pebs_events)
> >                         continue;

Maybe we should WARN in this case? A PEBS entry without any PEBS bits
set in the status field would be 'weird', right?

Maybe something like:

		if (WARN(bit >= x86_pmu.max_pebs_events,
			 "PEBS record without PEBS event! status=%Lx pebs_enabled=%Lx active_mask=%Lx",
			 p->status, cpuc->pebs_enabled, cpuc->active_mask))
			continue;

If that triggers we at least get more info.