[22.04 FEAT] KVM: Enable storage key checking for intercepted instruction handled by userspace

Bug #1933179 reported by bugproxy
20
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ubuntu on IBM z Systems
Fix Released
High
Skipper Bug Screeners
linux (Ubuntu)
Fix Released
High
Canonical Kernel Team

Bug Description

KVM uses lazy storage key enablement as Linux does no longer make use of the storage keys. When the guest enters keyed mode, then KVM will save/restore the key during paging, provide change/reference tracking for guest and host and for all interpreted instructions will do key protection.
If an instruction is intercepted and passed along to userspace (like QEMU) no storage key protection is checked, though. This is in violation of the architecture and it can result in misbehaving guests that rely on key protection for all instructions.
This item will improve the MEMOP ioctl to also add key checking. In case of a key protection the right fault is injected in the guest.

Value: Interoperability

bugproxy (bugproxy)
tags: added: architecture-s39064 bugnameltc-193314 severity-high targetmilestone-inin2110
Changed in ubuntu:
assignee: nobody → Skipper Bug Screeners (skipper-screen-team)
affects: ubuntu → linux (Ubuntu)
Frank Heimes (fheimes)
Changed in ubuntu-z-systems:
importance: Undecided → High
Changed in linux (Ubuntu):
status: New → Incomplete
Changed in ubuntu-z-systems:
status: New → Incomplete
assignee: nobody → Skipper Bug Screeners (skipper-screen-team)
Revision history for this message
bugproxy (bugproxy) wrote : Comment bridged from LTC Bugzilla

------- Comment From <email address hidden> 2021-09-09 04:57 EDT-------
Feature doesn't make it into impish / 21.10, hence moving to 22.04
Changing IBM BZ Target Milestone:21.10->22.04

tags: added: targetmilestone-inin2204
removed: targetmilestone-inin2110
Frank Heimes (fheimes)
summary: - [21.10 FEAT] KVM: Enable storage key checking for intercepted
+ [22.04 FEAT] KVM: Enable storage key checking for intercepted
instruction handled by userspace
Revision history for this message
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2022-02-04 08:01 EDT-------
*** Bug 193313 has been marked as a duplicate of this bug. ***

Revision history for this message
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2022-02-15 02:13 EDT-------
The code has been committed to the Christians kvms390 next branch https://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git/log/?h=next

The commits are:

5e35d0eb472b KVM: s390: Update api documentation for memop ioctl
d004079edc16 KVM: s390: Add capability for storage key extension of MEM_OP IOCTL
0e1234c02b77 KVM: s390: Rename existing vcpu memop functions
ef11c9463ae0 KVM: s390: Add vm IOCTL for key checked guest absolute memory access
e9e9feebcbc1 KVM: s390: Add optional storage key checking to MEMOP IOCTL
c7ef9ebbed20 KVM: s390: selftests: Test TEST PROTECTION emulation
61380a7adfce KVM: s390: handle_tprot: Honor storage keys
e613d83454d7 KVM: s390: Honor storage keys when accessing guest memory
1a82f6ab2365 s390/uaccess: Add copy_from/to_user_key functions

Frank Heimes (fheimes)
Changed in linux (Ubuntu):
status: Incomplete → New
Changed in ubuntu-z-systems:
status: Incomplete → New
Revision history for this message
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2022-02-15 10:18 EDT-------
(In reply to comment #9)
> The code has been committed to the Christians kvms390 next branch
> https://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git/log/?h=next
>
> The commits are:
>
> 5e35d0eb472b KVM: s390: Update api documentation for memop ioctl
> d004079edc16 KVM: s390: Add capability for storage key extension of MEM_OP
> IOCTL
> 0e1234c02b77 KVM: s390: Rename existing vcpu memop functions
> ef11c9463ae0 KVM: s390: Add vm IOCTL for key checked guest absolute memory
> access
> e9e9feebcbc1 KVM: s390: Add optional storage key checking to MEMOP IOCTL
> c7ef9ebbed20 KVM: s390: selftests: Test TEST PROTECTION emulation
> 61380a7adfce KVM: s390: handle_tprot: Honor storage keys
> e613d83454d7 KVM: s390: Honor storage keys when accessing guest memory
> 1a82f6ab2365 s390/uaccess: Add copy_from/to_user_key functions

It is now also part of linux-next as of Feb 15

Frank Heimes (fheimes)
Changed in linux (Ubuntu):
importance: Undecided → High
Revision history for this message
Frank Heimes (fheimes) wrote :

Thx and yes, they are all in (linux-next).
Will work on a PR for the submission to the kernel team soon - after package feature freeze (that is this Thursday).

Revision history for this message
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2022-02-22 07:42 EDT-------
Two more patches should be picked on top of the ones listed above: a bug fix and a doc clarification.

3d9042f8b923 KVM: s390: Add missing vm MEM_OP size check
cbf9b8109d32 KVM: s390: Clarify key argument for MEM_OP in api docs

Revision history for this message
Frank Heimes (fheimes) wrote :

Ok ... but these are not in linux-next, yet.
Need to wait for them to land at least in linux-next, I guess you are working on that...

(Note that the FF this Thu is for packages, means if these two commits land in linux-next in the next days, we should be still able to pick them ...)

Changed in linux (Ubuntu):
status: New → Incomplete
Changed in ubuntu-z-systems:
status: New → Incomplete
Revision history for this message
Frank Heimes (fheimes) wrote :

The patches:
3d9042f8b923 KVM: s390: Add missing vm MEM_OP size check
cbf9b8109d32 KVM: s390: Clarify key argument for MEM_OP in api docs
have landed now in 'linux-next'.

Changed in linux (Ubuntu):
status: Incomplete → New
Changed in ubuntu-z-systems:
status: Incomplete → New
Revision history for this message
Frank Heimes (fheimes) wrote :

Cherry-picking these commits lead to several conflicts.
(1a82f6ab2365 can be easily solved, but there is more ...)
Are there possibly pre-requisite commits missing - or maybe backports of some of the listed ones?

The changes (either cherry-picks or backports) need to apply cleanly to jammy's master-next tree to be able to accept them.

[ git clone git://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/jammy --branch master-next --single-branch jammy-master-next ]

Please double-check the patches mentioned above based on jammy-master-next.

Changed in ubuntu-z-systems:
status: New → Incomplete
Changed in linux (Ubuntu):
status: New → Incomplete
Revision history for this message
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2022-02-23 09:13 EDT-------
There are indeed missing prerequisites:
416e7f0c9d61 (KVM: s390: gaccess: Refactor gpa and length calculation)
7faa543df19b (KVM: s390: gaccess: Refactor access address range check)
bad13799e030 (KVM: s390: gaccess: Cleanup access to guest pages)
012a224e1fa3 (s390/uaccess: introduce bit field for OAC specifier)
3d787b392d16 (s390/uaccess: fix compile error)

[ git cherry-pick 416e7f0c9d61 7faa543df19b bad13799e030 012a224e1fa3 3d787b392d16 ]

This will result in two trivial conflicts, one from the TEST PROTECTION selftest, one from the introduction of the KVM_CAP_S390_MEM_OP_EXTENSION capability.

Revision history for this message
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2022-02-23 10:35 EDT-------
Another thing to consider. Paolo will merge a power change that also uses CAP 210. We will very likely have 211 for KVM_CAP_S390_MEM_OP_EXTENSION as a merge resolution. (To be considered when doing the backport).....

See patch
KVM: s390: Add capability for storage key extension of MEM_OP IOCTL

Revision history for this message
Frank Heimes (fheimes) wrote :

Hi @cborntra - knowing that it's probably a bit crystal-ball reading, but do you have a rough idea when this might happen?
Just don't want to generate updates to the jammy kernel like crazy.
So IF it's a matter of a few days (and with that well before the freeze) I would wait for it to come ...

Revision history for this message
Frank Heimes (fheimes) wrote (last edit ):

Hi @Janis, thx - yes, that's now a working set!

Revision history for this message
Frank Heimes (fheimes) wrote :

Pull request submitted to kernel team's mailing list:
https://lists.ubuntu.com/archives/kernel-team/2022-February/thread.html#128293
changing status to 'In Progress'.

A test build was done here:
https://launchpad.net/~fheimes/+archive/ubuntu/lp1933179/

Changed in linux (Ubuntu):
status: Incomplete → In Progress
Changed in ubuntu-z-systems:
status: Incomplete → In Progress
information type: Private → Public
Changed in linux (Ubuntu):
assignee: Skipper Bug Screeners (skipper-screen-team) → Canonical Kernel Team (canonical-kernel-team)
Revision history for this message
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2022-02-24 04:54 EDT-------
You only need the s390 specific lines for the two conflicts, i.e.

+/s390x/tprot

and

+#define KVM_CAP_S390_MEM_OP_EXTENSION 210

Revision history for this message
Frank Heimes (fheimes) wrote :

Yes, that's already incl. in the pull request that I've mentioned in the comment above ...
and with that also incl. in the test kernel.

Revision history for this message
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2022-02-24 07:29 EDT-------
Am I looking at the wrong thing?
https://git.launchpad.net/~fheimes/+git/lp1933179/commit/?h=master-next&id=1e2aeba3bda1e4ab7e91487ac1df29f1e3767adc

Those lines are superfluous:
+/x86_64/amx_test
+/x86_64/cpuid_test

Similarly the cap patch pulls in unrelated caps.

Revision history for this message
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2022-02-25 07:32 EDT-------
Capability number will be 211, see this merge

https://git.kernel.org/pub/scm/virt/kvm/kvm.git/commit/?h=next&id=4dfc4ec2b7f5a3a27d166ac42cf8a583fa2d3284

diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index dbc550bbd9fa3..a02bbf8fd0f67 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1140,7 +1140,8 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_VM_GPA_BITS 207
#define KVM_CAP_XSAVE2 208
#define KVM_CAP_SYS_ATTRIBUTES 209
-#define KVM_CAP_S390_MEM_OP_EXTENSION 210
+#define KVM_CAP_PPC_AIL_MODE_3 210
+#define KVM_CAP_S390_MEM_OP_EXTENSION 211

Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (67.6 KiB)

This bug was fixed in the package linux - 5.15.0-23.23

---------------
linux (5.15.0-23.23) jammy; urgency=medium

  * jammy/linux: 5.15.0-23.23 -proposed tracker (LP: #1964573)

  * Packaging resync (LP: #1786013)
    - [Packaging] resync dkms-build{,--nvidia-N} from LRMv5
    - debian/dkms-versions -- update from kernel-versions (main/master)

  * [22.04 FEAT] KVM: Enable GISA support for Secure Execution guests
    (LP: #1959977)
    - KVM: s390: pv: make use of ultravisor AIV support

  * intel_iommu breaks Intel IPU6 camera: isys port open ready failed -16
    (LP: #1958004)
    - SAUCE: iommu: intel-ipu: use IOMMU passthrough mode for Intel IPUs

  * CVE-2022-23960
    - ARM: report Spectre v2 status through sysfs
    - ARM: early traps initialisation
    - ARM: use LOADADDR() to get load address of sections
    - ARM: Spectre-BHB workaround
    - ARM: include unprivileged BPF status in Spectre V2 reporting
    - arm64: Add Neoverse-N2, Cortex-A710 CPU part definition
    - arm64: Add HWCAP for self-synchronising virtual counter
    - arm64: Add Cortex-X2 CPU part definition
    - arm64: add ID_AA64ISAR2_EL1 sys register
    - arm64: cpufeature: add HWCAP for FEAT_AFP
    - arm64: cpufeature: add HWCAP for FEAT_RPRES
    - arm64: entry.S: Add ventry overflow sanity checks
    - arm64: spectre: Rename spectre_v4_patch_fw_mitigation_conduit
    - KVM: arm64: Allow indirect vectors to be used without SPECTRE_V3A
    - arm64: entry: Make the trampoline cleanup optional
    - arm64: entry: Free up another register on kpti's tramp_exit path
    - arm64: entry: Move the trampoline data page before the text page
    - arm64: entry: Allow tramp_alias to access symbols after the 4K boundary
    - arm64: entry: Don't assume tramp_vectors is the start of the vectors
    - arm64: entry: Move trampoline macros out of ifdef'd section
    - arm64: entry: Make the kpti trampoline's kpti sequence optional
    - arm64: entry: Allow the trampoline text to occupy multiple pages
    - arm64: entry: Add non-kpti __bp_harden_el1_vectors for mitigations
    - arm64: entry: Add vectors that have the bhb mitigation sequences
    - arm64: entry: Add macro for reading symbol addresses from the trampoline
    - arm64: Add percpu vectors for EL1
    - arm64: proton-pack: Report Spectre-BHB vulnerabilities as part of Spectre-v2
    - arm64: Mitigate spectre style branch history side channels
    - KVM: arm64: Allow SMCCC_ARCH_WORKAROUND_3 to be discovered and migrated
    - arm64: Use the clearbhb instruction in mitigations
    - arm64: proton-pack: Include unprivileged eBPF status in Spectre v2
      mitigation reporting
    - ARM: fix build error when BPF_SYSCALL is disabled

  * CVE-2021-26401
    - x86/speculation: Use generic retpoline by default on AMD
    - x86/speculation: Update link to AMD speculation whitepaper
    - x86/speculation: Warn about Spectre v2 LFENCE mitigation
    - x86/speculation: Warn about eIBRS + LFENCE + Unprivileged eBPF + SMT

  * CVE-2022-0001
    - x86,bugs: Unconditionally allow spectre_v2=retpoline,amd
    - x86/speculation: Rename RETPOLINE_AMD to RETPOLINE_LFENCE
    - x86/speculation: Add eIBRS + Retpoline options
    - Document...

Changed in linux (Ubuntu):
status: In Progress → Fix Released
Frank Heimes (fheimes)
Changed in ubuntu-z-systems:
status: In Progress → Fix Released
Revision history for this message
Ubuntu Kernel Bot (ubuntu-kernel-bot) wrote :

This bug is awaiting verification that the linux-oracle-5.15/5.15.0-1006.8~20.04.1 kernel in -proposed solves the problem. Please test the kernel and update this bug with the results. If the problem is solved, change the tag 'verification-needed-focal' to 'verification-done-focal'. If the problem still exists, change the tag 'verification-needed-focal' to 'verification-failed-focal'.

If verification is not done by 5 working days from today, this fix will be dropped from the source code, and this bug will be closed.

See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you!

tags: added: verification-needed-focal
Revision history for this message
Frank Heimes (fheimes) wrote :

This bug is already Fix Released and closed and was requested for jammy only.
So it wasn't requested for focal, hence updating the tags to 'verification-done-focal' to unblock any processes.

tags: added: verification-done-focal
removed: verification-needed-focal
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.