Re: perf: fuzzer triggered warning in intel_pmu_drain_pebs_nhm()

[Stephane Eranian]

Peter,

On Fri, Jul 3, 2015 at 3:13 PM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> 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.
>
Where do you see that we use cpuc->pebs_enabled after clearing it
in pebs_disable() to check for overflow or active in drain_pebs()?
I only see it used in get_next_pebs_record_by_bit()?

> 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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/