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-next>] [day] [month] [year] [list]
Message-Id: <20220503060205.2823727-1-oupton@google.com>
Date:   Tue,  3 May 2022 06:01:58 +0000
From:   Oliver Upton <oupton@...gle.com>
To:     kvmarm@...ts.cs.columbia.edu
Cc:     kvm@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org, maz@...nel.org, james.morse@....com,
        alexandru.elisei@....com, suzuki.poulose@....com,
        reijiw@...gle.com, ricarkol@...gle.com,
        Oliver Upton <oupton@...gle.com>
Subject: [PATCH v4 0/7] KVM: arm64: Limit feature register reads from AArch32

KVM/arm64 does not restrict the guest's view of the AArch32 feature
registers when read from AArch32. HCR_EL2.TID3 is cleared for AArch32
guests, meaning that register reads come straight from hardware. This is
problematic as KVM relies on read_sanitised_ftr_reg() to expose a set of
features consistent for a particular system.

Appropriate handlers must first be put in place for CP10 and CP15 ID
register accesses before setting TID3. Rather than exhaustively
enumerating each of the encodings for CP10 and CP15 registers, take the
lazy route and aim the register accesses at the AArch64 system register
table.

Patches 1-2 are small cleanups to how we handle register emulation
failure. No functional change for current KVM, but required to do
register emulation correctly in this series.

Patch 3 reroutes the CP15 registers into the AArch64 table, taking care
to immediately RAZ undefined ranges of registers. This is done to avoid
possibly conflicting with encodings for future AArch64 registers.

Patch 4 installs an exit handler for the CP10 ID registers and also
relies on the general AArch64 register handler to implement reads.

Patch 5 actually sets TID3 for AArch32 guests, providing known-safe
values for feature register accesses.

Patch 6 makes KVM UNDEF the guest on an unsupported PMU reg access.
Finally, patch 7 drops the intermediate fix of returning early from
register accesses if the PMU is disabled.

There is an argument that the series is in fact a bug fix for running
AArch32 VMs on heterogeneous systems. To that end, it could be
blamed/backported to when we first knew better:

  93390c0a1b20 ("arm64: KVM: Hide unsupported AArch64 CPU features from
  guests")

But I left that tag off as in the aforementioned change skipping
AArch32 was intentional. Up to you, Marc, if you want to call it a
bugfix ;-)

Applies cleanly to 5.18-rc5.

Tested with AArch32 kvm-unit-tests and booting an AArch32 debian guest
on a Raspberry Pi 4. Additionally, I tested AArch32 kvm-unit-tests w/
pmu={on,off} and saw no splat, as Alex had discovered [1]. The test
correctly skips with the PMU feature bit disabled now.

Furthermore, I hacked up the test even more to ignore ID_DFR0.PerfMon to
verify that the guest UNDEFs when the PMU is disabled (and doesn't blow
up the host kernel).

[1]: https://lore.kernel.org/r/20220425145530.723858-1-alexandru.elisei@arm.com

v1: https://lore.kernel.org/kvmarm/20220329011301.1166265-1-oupton@google.com/
v2: https://lore.kernel.org/r/20220401010832.3425787-1-oupton@google.com
v3: https://lore.kernel.org/kvmarm/20220425235342.3210912-1-oupton@google.com

v3 -> v4:
  - Grab Alex's patch for using pmu_visibility() to hide PMU regs
  - Revert Alex's intermediate fix of silently sinking PMU reg
    read/writes

v2 -> v3:
  - Collect R-b from Reiji (thanks!)
  - Adopt Marc's suggestion for CP15 register handling
  - Avoid writing to Rt when emulation fails (Marc)
  - Print some debug info on an unexpected CP10 register access (Reiji)

v1 -> v2:
  - Actually set TID3! Oops.
  - Refactor kvm_emulate_cp15_id_reg() to check preconditions before
    proceeding to emulation (Reiji)
  - Tighten up comment on kvm_is_cp15_id_reg() to indicate that the only
    other trapped ID register (CTR) is already handled in the cp15

Alexandru Elisei (1):
  KVM/arm64: Hide AArch32 PMU registers when not available

Oliver Upton (6):
  KVM: arm64: Return a bool from emulate_cp()
  KVM: arm64: Don't write to Rt unless sys_reg emulation succeeds
  KVM: arm64: Wire up CP15 feature registers to their AArch64
    equivalents
  KVM: arm64: Plumb cp10 ID traps through the AArch64 sysreg handler
  KVM: arm64: Start trapping ID registers for 32 bit guests
  Revert "KVM/arm64: Don't emulate a PMU for 32-bit guests if feature
    not set"

 arch/arm64/include/asm/kvm_arm.h     |   3 +-
 arch/arm64/include/asm/kvm_emulate.h |   7 -
 arch/arm64/include/asm/kvm_host.h    |   1 +
 arch/arm64/kvm/handle_exit.c         |   1 +
 arch/arm64/kvm/pmu-emul.c            |  23 +--
 arch/arm64/kvm/sys_regs.c            | 257 +++++++++++++++++++++------
 arch/arm64/kvm/sys_regs.h            |   9 +-
 7 files changed, 211 insertions(+), 90 deletions(-)

-- 
2.36.0.464.gb9c8b46e94-goog

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ