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: <159242406774.62212.13909672383879587787@swboyd.mtv.corp.google.com>
Date:   Wed, 17 Jun 2020 13:01:07 -0700
From:   Stephen Boyd <swboyd@...omium.org>
To:     Alexandru Elisei <alexandru.elisei@....com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Cc:     mark.rutland@....com, Julien Thierry <julien.thierry@....com>,
        Peter Zijlstra <peterz@...radead.org>, maz@...nel.org,
        Jiri Olsa <jolsa@...hat.com>,
        Will Deacon <will.deacon@....com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Ingo Molnar <mingo@...hat.com>, catalin.marinas@....com,
        Namhyung Kim <namhyung@...nel.org>, will@...nel.org,
        Julien Thierry <julien.thierry.kdev@...il.com>
Subject: Re: [PATCH v5 1/7] arm64: perf: Add missing ISB in armv8pmu_enable_event()

Quoting Alexandru Elisei (2020-06-17 04:38:45)
> Writes to the PMXEVTYPER_EL0 register are not self-synchronising. In
> armv8pmu_enable_event(), the PE can reorder configuring the event type
> after we have enabled the counter and the interrupt. This can lead to an
> interrupt being asserted because the of the previous event type that we

'because the of the' doesn't read properly.

> were counting, not the one that we've just enabled.
> 
> The same rationale applies to writes to the PMINTENSET_EL1 register. The PE
> can reorder enabling the interrupt at any point in the future after we have
> enabled the event.
> 
> Prevent both situations from happening by adding an ISB just before we
> enable the event counter.
> 
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index 4d7879484cec..ee180b2a5b39 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -605,6 +605,7 @@ static void armv8pmu_enable_event(struct perf_event *event)
>          * Enable interrupt for this counter
>          */
>         armv8pmu_enable_event_irq(event);
> +       isb();

Please add a comment before the isb() explaining the situation. Nobody
knows what this is for when reading the code and they don't want to do
git archaeology to figure it out.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ