linux-kernel - Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split lock

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: <ba2c0aab-1d7c-5cfd-0054-ac2c266c1df3@redhat.com>
Date:   Wed, 16 Oct 2019 17:37:41 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Thomas Gleixner <tglx@...utronix.de>,
        Xiaoyao Li <xiaoyao.li@...el.com>
Cc:     Sean Christopherson <sean.j.christopherson@...el.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        H Peter Anvin <hpa@...or.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Dave Hansen <dave.hansen@...el.com>,
        Radim Krcmar <rkrcmar@...hat.com>,
        Ashok Raj <ashok.raj@...el.com>,
        Tony Luck <tony.luck@...el.com>,
        Dan Williams <dan.j.williams@...el.com>,
        Sai Praneeth Prakhya <sai.praneeth.prakhya@...el.com>,
        Ravi V Shankar <ravi.v.shankar@...el.com>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        x86 <x86@...nel.org>, kvm@...r.kernel.org
Subject: Re: [PATCH v9 09/17] x86/split_lock: Handle #AC exception for split
 lock

On 16/10/19 16:43, Thomas Gleixner wrote:
> 
> N | #AC       | #AC enabled | SMT | Ctrl    | Guest | Action
> R | available | on host     |     | exposed | #AC   |
> --|-----------|-------------|-----|---------|-------|---------------------
>   |           |             |     |         |       |
> 0 | N         |     x       |  x  |   N     |   x   | None
>   |           |             |     |         |       |
> 1 | Y         |     N       |  x  |   N     |   x   | None

So far so good.

> 2 | Y         |     Y       |  x  |   Y     |   Y   | Forward to guest
>
> 3 | Y         |     Y       |  N  |   Y     |   N   | A) Store in vCPU and
>   |           |             |     |         |       |    toggle on VMENTER/EXIT
>   |           |             |     |         |       |
>   |           |             |     |         |       | B) SIGBUS or KVM exit code

(2) is problematic for the SMT=y case, because of what happens when #AC 
is disabled on the host---safe guests can start to be susceptible to 
DoS.

For (3), which is the SMT=n case,, the behavior is the same independent of
guest #AC.

So I would change these two lines to:

  2 | Y         |     Y       |  Y  |   N     |   x   | On first guest #AC,
    |           |             |     |         |       | disable globally on host.
    |           |             |     |         |       |
  3 | Y         |     Y       |  N  |   Y     |   x   | Switch MSR_TEST_CTRL on
    |           |             |     |         |       | enter/exit, plus:
    |           |             |     |         |       | A) #AC forwarded to guest.
    |           |             |     |         |       | B) SIGBUS or KVM exit code

> 4 | Y         |     Y       |  Y  |   Y     |   N   | A) Disable globally on
>   |           |             |     |         |       |    host. Store in vCPU/guest
>   |           |             |     |         |       |    state and evtl. reenable
>   |           |             |     |         |       |    when guest goes away.
>   |           |             |     |         |       |
>   |           |             |     |         |       | B) SIGBUS or KVM exit code

Also okay.  And finally:

  5 | Y         |     Y       |  Y  |   Y     |   Y   | Forward to guest

> Now there are a two possible state transitions:

>  #AC enabled on host during runtime
> 
>    Existing guests are not notified. Nothing changes.

Switches from (1) to (2) or (4) and (5).  Ugly for (2) and (4A), in that
split-lock detection might end up being forcibly disabled on the host, but
guests do not notice anything.  Okay for (4B) and (5).

>  #AC disabled on host during runtime

Switches from (2), (4) and (5) to (1).  Bad for (4A) and (5), in that
guests might miss #ACs from userspace.  No problem for (2), okay for (4B)
since the host admin decision affects KVM userspace but not KVM guests.

Because (4A) and (5) are problematic, and (4B) can cause guests to halt
irrecoverably on guest #AC, I'd prefer the control not to be
exposed by default.  In KVM API terms:

- KVM_GET_SUPPORTED_CPUID should *not* return the new CPUID bit and
KVM_GET_MSR_INDEX_LIST should not return MSR_TEST_CTRL.  A separate
capability can be queried with KVM_CHECK_EXTENSION to determine whether
KVM supports split-lock detection is available.  The default behavior will
be (2).

- we only need to pick one of (3A)/(4A) and (3B)/(4B).  (3A) should definitely
be the default, probably (4A) too.  But if both are implemented, the
aforementioned capability can be used with KVM_ENABLE_CAP to switch from
one behavior to the other.

Paolo