Re: perf: fuzzer triggered warning in intel_pmu_drain_pebs_nhm()

[Peter Zijlstra <peterz@infradead.org>]

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);