linux-kernel - Re: [PATCH 01/12] x86/mce: Fix hw MCE injection feature detection

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: <532fd5c3-dddb-4503-9b81-31c3d07a7119@intel.com>
Date:   Fri, 13 Oct 2023 08:05:54 -0700
From:   Dave Hansen <dave.hansen@...el.com>
To:     isaku.yamahata@...el.com, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     isaku.yamahata@...il.com, Michael Roth <michael.roth@....com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <seanjc@...gle.com>,
        linux-coco@...ts.linux.dev,
        Chao Peng <chao.p.peng@...ux.intel.com>,
        Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>,
        Borislav Petkov <bp@...e.de>
Subject: Re: [PATCH 01/12] x86/mce: Fix hw MCE injection feature detection

Isaku, when you report a bug, it would be great to include the folks who
authored and worked on the original patch that introduced the bug.  I've
gone ahead and done that for you here.

On 10/10/23 01:35, isaku.yamahata@...el.com wrote:
> From: Isaku Yamahata <isaku.yamahata@...el.com>
> 
> When initializing x86 MCE injection framework, it checks if hardware mce
> injection is available or not.  When it's not available on AMD, set the
> boolean variable to false to not use it.  The variable is on by default and
> the feature is AMD specific based on the code.
> 
> Because the variable is default on, it is true on Intel platform (probably
> on other non-AMD x86 platform).  It results in unchecked msr access of
> MSR_K7_HWCR=0xc0010015 when injecting MCE on Intel platform.  (Probably on
> other x86 platform.)
> 
> Make the variable of by default, and set the variable on when the hardware
> feature is usable.

Gah, I'm finding that changelog impenetrable.  Here's what's missing:

  * The entirety of check_hw_inj_possible() is for AMD hardware:
    X86_FEATURE_SMCA, the MSRs, hw_injection_possible, everything.
  * Only AMD systems with SMCA support hardware error injection
    (anything other than "echo sw > /sys/kernel/debug/mce-inject/flags")
  * That AMD-only restriction is enforced by 'hw_injection_possible'
  * 'hw_injection_possible' is true by default and only set to false in
    check_hw_inj_possible() ... the AMD-only code

The end result is that everyone except SMCA-enabled AMD systems (Intel
included) leaves hw_injection_possible=true.  They are free to try and
inject hardware errors.  If they do, they'll get errors when writing to
the MSRs.

To fix this, make disable hw_injection_possible by default.  Only enable
it on SMCA hardware that actually succeeds in ... whatever:

                wrmsrl_safe(mca_msr_reg(bank, MCA_STATUS), status);
                rdmsrl_safe(mca_msr_reg(bank, MCA_STATUS), &status);

is doing.

... and don't do it at the top of the function.  Why bother setting it
to true only to disable it a moment later?

Do something like the following instead.
View attachment "amdmce.patch" of type "text/x-patch" (734 bytes)