linux-kernel - Re: [PATCH v16] x86/split_lock: Enable split lock detection by kernel

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: <CALCETrXT9zo2yFN+iz-1ijayOKNNz-717pEJggU1kC79ZVf34g@mail.gmail.com>
Date:   Sat, 25 Jan 2020 16:34:29 -0800
From:   Andy Lutomirski <luto@...nel.org>
To:     "Luck, Tony" <tony.luck@...el.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Arvind Sankar <nivedita@...m.mit.edu>,
        "Christopherson, Sean J" <sean.j.christopherson@...el.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        "Yu, Fenghua" <fenghua.yu@...el.com>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        H Peter Anvin <hpa@...or.com>,
        "Raj, Ashok" <ashok.raj@...el.com>,
        "Shankar, Ravi V" <ravi.v.shankar@...el.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        x86 <x86@...nel.org>, Andrew Cooper <andrew.cooper3@...rix.com>
Subject: Re: [PATCH v16] x86/split_lock: Enable split lock detection by kernel

On Sat, Jan 25, 2020 at 2:07 PM Luck, Tony <tony.luck@...el.com> wrote:
>
> From: "Peter Zijlstra (Intel)" <peterz@...radead.org>
>

> +void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c)
> +{
> +       u64 ia32_core_caps = 0;
> +
> +       if (c->x86_vendor != X86_VENDOR_INTEL)
> +               return;
> +       if (cpu_has(c, X86_FEATURE_CORE_CAPABILITIES)) {
> +               /* Enumerate features reported in IA32_CORE_CAPABILITIES MSR. */
> +               rdmsrl(MSR_IA32_CORE_CAPS, ia32_core_caps);
> +       } else if (!boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
> +               /* Enumerate split lock detection by family and model. */
> +               if (x86_match_cpu(split_lock_cpu_ids))
> +                       ia32_core_caps |= MSR_IA32_CORE_CAPS_SPLIT_LOCK_DETECT;
> +       }

I was chatting with Andrew Cooper, and apparently there are a ton of
hypervisors bugs in this space, and the bugs take two forms.  Some
hypervisors might #GP the read, and some might allow the read but
silently swallow writes.  This isn't *that* likely given that the
hypervisor bit is the default, but we could improve this like (sorry
for awful whitespace);

static bool have_split_lock_detect(void) {
      unsigned long tmp;

      if (cpu_has(c, X86_FEATURE_CORE_CAPABILITIES) {
              /* Enumerate features reported in IA32_CORE_CAPABILITIES MSR. */
               rdmsrl(MSR_IA32_CORE_CAPS, tmp);
               if (tmp & MSR_IA32_CORE_CAPS_SPLIT_LOCK_DETECT)
                  return true;
      }

      if (cpu_has(c, X86_FEATURE_HYPERVISOR))
            return false;

       if (rdmsrl_safe(MSR_TEST_CTRL, &tmp))
               return false;

      if (wrmsrl_safe(MSR_TEST_CTRL, tmp ^ MSR_TEST_CTRL_SPLIT_LOCK_DETECT))
              return false;

      wrmsrl(MSR_TEST_CTRL, tmp);
}

Although I suppose the pile of wrmsrl_safes() in the existing patch
might be sufficient.

All this being said, the current code appears wrong if a CPU is in the
list but does have X86_FEATURE_CORE_CAPABILITIES.  Are there such
CPUs?  I think either the logic should be changed or a comment should
be added.