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

On Thu, Jul 02, 2015 at 11:18:10AM -0400, Vince Weaver wrote:
> 
> So sad to say the lack of fuzzer reports was because I was out of town for 
> a bit, not due to the kernel suddenly getting amazingly better.
> 
> In any case I am running against current git and getting a lot of 
> warnings, but most of them seem to be old ones.  This following one looks 
> new though.
> 
> This is current linus-git on a Haswell machine with peterz's patch to fix 
> the aux buffer spinlock recursion (I can still crash the kernel if that 
> patch is not applied).
> 
> It corresponds to:
> 
> 	WARN_ON_ONCE(!event->attr.precise_ip);
> 
> [  584.352324] WARNING: CPU: 2 PID: 18924 at arch/x86/kernel/cpu/perf_event_intel_ds.c:1198 intel_pmu_drain_pebs_nhm+0x283/0x2e0()

I've not yet tried to reproduce, but the below could explain things.

On disabling an event we first clear our cpuc->pebs_enabled bits, only
to then check them to see if there are any set, and if so, drain the
buffer.

If we just cleared the last bit, we'll fail to drain the buffer.

If we then program another event on that counter and another PEBS event,
we can hit the above WARN with the 'stale' entries left over from the
previous event.

---
 arch/x86/kernel/cpu/perf_event_intel_ds.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 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..041a30ba5654 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_ds.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_ds.c
@@ -548,7 +548,7 @@ int intel_pmu_drain_bts_buffer(void)
 
 static inline void intel_pmu_drain_pebs_buffer(void)
 {
-	struct pt_regs regs;
+	struct pt_regs regs; /* SAMPLE_REGS_INTR must not be set for FREERUNNING */
 
 	x86_pmu.drain_pebs(&regs);
 }
@@ -755,13 +755,6 @@ void intel_pmu_pebs_disable(struct perf_event *event)
 	struct hw_perf_event *hwc = &event->hw;
 	struct debug_store *ds = cpuc->ds;
 
-	cpuc->pebs_enabled &= ~(1ULL << hwc->idx);
-
-	if (event->hw.flags & PERF_X86_EVENT_PEBS_LDLAT)
-		cpuc->pebs_enabled &= ~(1ULL << (hwc->idx + 32));
-	else if (event->hw.flags & PERF_X86_EVENT_PEBS_ST)
-		cpuc->pebs_enabled &= ~(1ULL << 63);
-
 	if (ds->pebs_interrupt_threshold >
 	    ds->pebs_buffer_base + x86_pmu.pebs_record_size) {
 		intel_pmu_drain_pebs_buffer();
@@ -769,6 +762,13 @@ void intel_pmu_pebs_disable(struct perf_event *event)
 			perf_sched_cb_dec(event->ctx->pmu);
 	}
 
+	cpuc->pebs_enabled &= ~(1ULL << hwc->idx);
+
+	if (event->hw.flags & PERF_X86_EVENT_PEBS_LDLAT)
+		cpuc->pebs_enabled &= ~(1ULL << (hwc->idx + 32));
+	else if (event->hw.flags & PERF_X86_EVENT_PEBS_ST)
+		cpuc->pebs_enabled &= ~(1ULL << 63);
+
 	if (cpuc->enabled)
 		wrmsrl(MSR_IA32_PEBS_ENABLE, cpuc->pebs_enabled);
 
--
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/