linux-kernel - Re: [PATCH 1/8] perf/x86/intel: Fix memory corruption

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: <CABPqkBSZbAbU5h=t2f=oMre080ezSq78LaNQO5xEdRV1nczuew@mail.gmail.com>
Date:   Thu, 21 Mar 2019 10:23:50 -0700
From:   Stephane Eranian <eranian@...gle.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>, Jiri Olsa <jolsa@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>, tonyj@...e.com,
        nelson.dsouza@...el.com
Subject: Re: [PATCH 1/8] perf/x86/intel: Fix memory corruption

On Thu, Mar 21, 2019 at 9:45 AM Thomas Gleixner <tglx@...utronix.de> wrote:
>
> On Thu, 21 Mar 2019, Peter Zijlstra wrote:
> > Subject: perf/x86/intel: Initialize TFA MSR
> >
> > Stephane reported that we don't initialize the TFA MSR, which could lead
> > to trouble if the RESET value is not 0 or on kexec.
>
> That sentence doesn't parse.
>
>   Stephane reported that the TFA MSR is not initialized by the kernel, but
>   the TFA bit could set by firmware or as a leftover from a kexec, which
>   makes the state inconsistent.
>
Correct. This is what I meant.
The issue is what does the kernel guarantee when it boots?

I see:
static bool allow_tsx_force_abort = true;

Therefore you must ensure the MSR is set to reflect that state on boot.
So you have to force it to that value to be in sync which is what your
new patch is doing.

> > Reported-by: Stephane Eranian <eranian@...gle.com>
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> > ---
> >  arch/x86/events/intel/core.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
> > index 8baa441d8000..2d3caf2d1384 100644
> > --- a/arch/x86/events/intel/core.c
> > +++ b/arch/x86/events/intel/core.c
> > @@ -3575,6 +3575,12 @@ static void intel_pmu_cpu_starting(int cpu)
> >
> >       cpuc->lbr_sel = NULL;
> >
> > +     if (x86_pmu.flags & PMU_FL_TFA) {
> > +             WARN_ON_ONCE(cpuc->tfa_shadow);
>
> Hmm. I wouldn't warn here as this is a legit state for kexec/kdump and I
> don't think we can figure out whether this comes directly from the
> firmware.
>
> Thanks,
>
>         tglx