| 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
| ||
|
Date: Thu, 16 Jul 2015 08:02:03 +0200
From: Stephane Eranian <eranian@...glemail.com>
To: Peter Zijlstra <peterz@...radead.org>
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 Wed, Jul 15, 2015 at 2:35 PM, Peter Zijlstra <peterz@...radead.org> wrote:
> On Wed, Jul 15, 2015 at 08:42:50AM +0200, Stephane Eranian wrote:
>> On Fri, Jul 3, 2015 at 9:49 PM, Vince Weaver <vincent.weaver@...ne.edu> wrote:
>> > On Fri, 3 Jul 2015, Peter Zijlstra wrote:
>> >
>> >> That said, its far too warm and I might just not be making sense.
>> >
>> > you need to come visit Maine! Although I am not sure the cooler weather
>> > necessarily improves my kernel debugging skills.
>> >
>> > I managed to lock the machine (again this is with the patch applied).
>> >
>> I can reproduce the problem on my HSW running the fuzzer.
>>
>> I can see why this could be happening if you are mixing PEBS and non PEBS events
>> in the bottom 4 counters. I suspect:
>> for (bit = 0; bit < x86_pmu.max_pebs_events; bit++) {
>> if ((counts[bit] == 0) && (error[bit] == 0))
>> continue;
>>
>> This test is not correct when you have non-PEBS events mixed with PEBS
>> events and
>> they overflow at the same time. They will have counts[i] != 0 but
>> error[i] == 0, and thus
>> you fall thru the loop and hit the assert. Or it is something along those lines.
>>
>
> The only way I can make this work is if ->status only has !PEBS events
> set, because if it has both set we'll take that slow path which masks
> out the !PEBS bits.
>
> After masking there are 3 options:
>
> - there is one bit set, and its @bit, we increment counts[bit].
> - there are multiple bits set, we increment error[] for each set bit,
> we do not increment counts[].
> - there are no bits set, we do nothing.
>
> The intent was to never increment counts[] for !PEBS events.
>
> Now if we start out with only a single !PEBS event set, we'll pass the
> test and increment counts[] for a !PEBS and hit the warn.
>
> The below patch modifies the code such that it can deal with that
> particular issue. Can you try?
>
Been running it for a couple of hours, so far so good. I will let it
run all night.
Testing it on HSW and NHM, my SNB is broken at the moment.
Don't know if the fuzzer has already reproduced the test case.
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;
> - if (!test_bit(bit, cpuc->active_mask))
> - continue;
> +
> /*
> * The PEBS hardware does not deal well with the situation
> * when events happen near to each other and multiple bits
> @@ -1172,27 +1175,21 @@ static void intel_pmu_drain_pebs_nhm(struct pt_regs *iregs)
> * one, and it's not possible to reconstruct all events
> * that caused the PEBS record. It's called collision.
> * If collision happened, the record will be dropped.
> - *
> */
> - if (p->status != (1 << bit)) {
> - u64 pebs_status;
> -
> - /* slow path */
> - pebs_status = p->status & cpuc->pebs_enabled;
> - pebs_status &= (1ULL << MAX_PEBS_EVENTS) - 1;
> - if (pebs_status != (1 << bit)) {
> - for_each_set_bit(i, (unsigned long *)&pebs_status,
> - MAX_PEBS_EVENTS)
> - error[i]++;
> - continue;
> - }
> + if (p->status != (1ULL << bit)) {
> + for_each_set_bit(i, (unsigned long *)&pebs_status,
> + x86_pmu.max_pebs_events)
> + error[i]++;
> + continue;
> }
> +
> counts[bit]++;
> }
>
> for (bit = 0; bit < x86_pmu.max_pebs_events; bit++) {
> if ((counts[bit] == 0) && (error[bit] == 0))
> continue;
> +
> event = cpuc->events[bit];
> WARN_ON_ONCE(!event);
> WARN_ON_ONCE(!event->attr.precise_ip);
--
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/
Powered by blists - more mailing lists