linux-kernel - Re: perf: fuzzer triggered warning in intel_pmu_drain_pebs

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: <20150716071536.GY19282@twins.programming.kicks-ass.net>
Date:	Thu, 16 Jul 2015 09:15:36 +0200
From:	Peter Zijlstra <peterz@...radead.org>
To:	eranian@...il.com
Cc:	Vince Weaver <vincent.weaver@...ne.edu>,
	LKML <linux-kernel@...r.kernel.org>,
	Ingo Molnar <mingo@...hat.com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	kan.liang@...el.com
Subject: Re: perf: fuzzer triggered warning in intel_pmu_drain_pebs_nhm()

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.
--
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/