| Message ID | 20220526210817.3428868-1-seanjc@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | KVM: x86: Emulator _regs fixes and cleanups | expand |
It >DID< build ok with this patch, I applied it and built last night,
I'm presently running that kernel with a KVM/Qemu Windows guest on my
workstation:
Linux nanook 5.18.0 #1 SMP PREEMPT Wed May 25 18:14:43 PDT 2022 x86_64 x86_64
x86_64 GNU/Linux
The Windows guest starts and runs but networking isn't working (I am
using a bridge for the guest hosts and a static IP for both the main machine
and guest), it all works including networking with 5.17.9.
-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-
Eskimo North Linux Friendly Internet Access, Shell Accounts, and Hosting.
Knowledgeable human assistance, not telephone trees or script readers.
See our web site: http://www.eskimo.com/ (206) 812-0051 or (800) 246-6874.
On Thu, 26 May 2022, Sean Christopherson wrote:
> Date: Thu, 26 May 2022 21:08:09 +0000
> From: Sean Christopherson <seanjc@google.com>
> To: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Sean Christopherson <seanjc@google.com>,
> Vitaly Kuznetsov <vkuznets@redhat.com>,
> Wanpeng Li <wanpengli@tencent.com>, Jim Mattson <jmattson@google.com>,
> Joerg Roedel <joro@8bytes.org>, kvm@vger.kernel.org,
> linux-kernel@vger.kernel.org, Kees Cook <keescook@chromium.org>,
> Robert Dinse <nanook@eskimo.com>
> Subject: [PATCH v2 0/8] KVM: x86: Emulator _regs fixes and cleanups
>
> Clean up and harden the use of the x86_emulate_ctxt._regs, which is
> surrounded by a fair bit of magic. This series was prompted by bug reports
> by Kees and Robert where GCC-12 flags an out-of-bounds _regs access. The
> warning is a false positive due to a now-known GCC bug, but it's cheap and
> easy to harden the _regs usage, and doing so minimizing the risk of more
> precisely handling 32-bit vs. 64-bit GPRs.
>
> I didn't tag patch 2 with Fixes or Cc: stable@. It does remedy the
> GCC-12 warning, but AIUI the GCC-12 bug affects other KVM paths that
> already have explicit guardrails, i.e. fixing this one case doesn't
> guarantee happiness when building with CONFIG_KVM_WERROR=y, let alone
> CONFIG_WERROR=y. That said, it might be worth sending to the v5.18 stable
> tree[*] as it does appear to make some configs/setups happy.
>
> [*] KVM hasn't changed, but the warning=>error was introduced in v5.18 by
> commit e6148767825c ("Makefile: Enable -Warray-bounds").
>
> v2:
> - Collect reviews and tests. [Vitaly, Kees, Robert]
> - Tweak patch 1's changelog to explicitly call out that dirty_regs is a
> 4 byte field. [Vitaly]
> - Add Reported-by for Kees and Robert since this does technically fix a
> build breakage.
> - Use a raw literal for NR_EMULATOR_GPRS instead of VCPU_REGS_R15+1 to
> play nice with 32-bit builds. [kernel test robot]
> - Reduce the number of emulated GPRs to 8 for 32-bit builds.
> - Add and use KVM_EMULATOR_BUG_ON() to bug/kill the VM when an emulator
> bug is detected. [Vitaly]
>
> v1: https://lore.kernel.org/all/20220525222604.2810054-1-seanjc@google.com
>
> Sean Christopherson (8):
> KVM: x86: Grab regs_dirty in local 'unsigned long'
> KVM: x86: Harden _regs accesses to guard against buggy input
> KVM: x86: Omit VCPU_REGS_RIP from emulator's _regs array
> KVM: x86: Use 16-bit fields to track dirty/valid emulator GPRs
> KVM: x86: Reduce the number of emulator GPRs to '8' for 32-bit KVM
> KVM: x86: Bug the VM if the emulator accesses a non-existent GPR
> KVM: x86: Bug the VM if the emulator generates a bogus exception
> vector
> KVM: x86: Bug the VM on an out-of-bounds data read
>
> arch/x86/kvm/emulate.c | 26 ++++++++++++++++++++------
> arch/x86/kvm/kvm_emulate.h | 28 +++++++++++++++++++++++++---
> arch/x86/kvm/x86.c | 9 +++++++++
> 3 files changed, 54 insertions(+), 9 deletions(-)
>
>
> base-commit: 90bde5bea810d766e7046bf5884f2ccf76dd78e9
> --
> 2.36.1.255.ge46751e96f-goog
>
On 5/26/22 23:08, Sean Christopherson wrote: > Clean up and harden the use of the x86_emulate_ctxt._regs, which is > surrounded by a fair bit of magic. This series was prompted by bug reports > by Kees and Robert where GCC-12 flags an out-of-bounds _regs access. The > warning is a false positive due to a now-known GCC bug, but it's cheap and > easy to harden the _regs usage, and doing so minimizing the risk of more > precisely handling 32-bit vs. 64-bit GPRs. > > I didn't tag patch 2 with Fixes or Cc: stable@. It does remedy the > GCC-12 warning, but AIUI the GCC-12 bug affects other KVM paths that > already have explicit guardrails, i.e. fixing this one case doesn't > guarantee happiness when building with CONFIG_KVM_WERROR=y, let alone > CONFIG_WERROR=y. That said, it might be worth sending to the v5.18 stable > tree[*] as it does appear to make some configs/setups happy. > > [*] KVM hasn't changed, but the warning=>error was introduced in v5.18 by > commit e6148767825c ("Makefile: Enable -Warray-bounds"). > > v2: > - Collect reviews and tests. [Vitaly, Kees, Robert] > - Tweak patch 1's changelog to explicitly call out that dirty_regs is a > 4 byte field. [Vitaly] > - Add Reported-by for Kees and Robert since this does technically fix a > build breakage. > - Use a raw literal for NR_EMULATOR_GPRS instead of VCPU_REGS_R15+1 to > play nice with 32-bit builds. [kernel test robot] > - Reduce the number of emulated GPRs to 8 for 32-bit builds. > - Add and use KVM_EMULATOR_BUG_ON() to bug/kill the VM when an emulator > bug is detected. [Vitaly] > > v1: https://lore.kernel.org/all/20220525222604.2810054-1-seanjc@google.com > > Sean Christopherson (8): > KVM: x86: Grab regs_dirty in local 'unsigned long' > KVM: x86: Harden _regs accesses to guard against buggy input > KVM: x86: Omit VCPU_REGS_RIP from emulator's _regs array > KVM: x86: Use 16-bit fields to track dirty/valid emulator GPRs > KVM: x86: Reduce the number of emulator GPRs to '8' for 32-bit KVM > KVM: x86: Bug the VM if the emulator accesses a non-existent GPR > KVM: x86: Bug the VM if the emulator generates a bogus exception > vector > KVM: x86: Bug the VM on an out-of-bounds data read > > arch/x86/kvm/emulate.c | 26 ++++++++++++++++++++------ > arch/x86/kvm/kvm_emulate.h | 28 +++++++++++++++++++++++++--- > arch/x86/kvm/x86.c | 9 +++++++++ > 3 files changed, 54 insertions(+), 9 deletions(-) > > > base-commit: 90bde5bea810d766e7046bf5884f2ccf76dd78e9 Queued, thanks. Paolo
Clean up and harden the use of the x86_emulate_ctxt._regs, which is surrounded by a fair bit of magic. This series was prompted by bug reports by Kees and Robert where GCC-12 flags an out-of-bounds _regs access. The warning is a false positive due to a now-known GCC bug, but it's cheap and easy to harden the _regs usage, and doing so minimizing the risk of more precisely handling 32-bit vs. 64-bit GPRs. I didn't tag patch 2 with Fixes or Cc: stable@. It does remedy the GCC-12 warning, but AIUI the GCC-12 bug affects other KVM paths that already have explicit guardrails, i.e. fixing this one case doesn't guarantee happiness when building with CONFIG_KVM_WERROR=y, let alone CONFIG_WERROR=y. That said, it might be worth sending to the v5.18 stable tree[*] as it does appear to make some configs/setups happy. [*] KVM hasn't changed, but the warning=>error was introduced in v5.18 by commit e6148767825c ("Makefile: Enable -Warray-bounds"). v2: - Collect reviews and tests. [Vitaly, Kees, Robert] - Tweak patch 1's changelog to explicitly call out that dirty_regs is a 4 byte field. [Vitaly] - Add Reported-by for Kees and Robert since this does technically fix a build breakage. - Use a raw literal for NR_EMULATOR_GPRS instead of VCPU_REGS_R15+1 to play nice with 32-bit builds. [kernel test robot] - Reduce the number of emulated GPRs to 8 for 32-bit builds. - Add and use KVM_EMULATOR_BUG_ON() to bug/kill the VM when an emulator bug is detected. [Vitaly] v1: https://lore.kernel.org/all/20220525222604.2810054-1-seanjc@google.com Sean Christopherson (8): KVM: x86: Grab regs_dirty in local 'unsigned long' KVM: x86: Harden _regs accesses to guard against buggy input KVM: x86: Omit VCPU_REGS_RIP from emulator's _regs array KVM: x86: Use 16-bit fields to track dirty/valid emulator GPRs KVM: x86: Reduce the number of emulator GPRs to '8' for 32-bit KVM KVM: x86: Bug the VM if the emulator accesses a non-existent GPR KVM: x86: Bug the VM if the emulator generates a bogus exception vector KVM: x86: Bug the VM on an out-of-bounds data read arch/x86/kvm/emulate.c | 26 ++++++++++++++++++++------ arch/x86/kvm/kvm_emulate.h | 28 +++++++++++++++++++++++++--- arch/x86/kvm/x86.c | 9 +++++++++ 3 files changed, 54 insertions(+), 9 deletions(-) base-commit: 90bde5bea810d766e7046bf5884f2ccf76dd78e9