[PATCH v4 00/10] KVM: s390: Do storage key checking

From: Janis Schoetterl-Glausch
Date: Fri Feb 11 2022 - 13:24:56 EST


Check keys when emulating instructions and let user space do key checked
accesses.
User space can do so via an extension of the MEMOP IOCTL:
* allow optional key checking
* allow MEMOP on vm fd, so key checked accesses on absolute memory
become possible

I haven't finished the memop selftest rewrite, but decided to send out a
new version anyway, since the functional patches are (hopefully) final
and the memop selftest patch works. I'll reply to it with the
rewritten version.

v3: https://lore.kernel.org/kvm/20220209170422.1910690-1-scgl@xxxxxxxxxxxxx/
v2: https://lore.kernel.org/kvm/20220207165930.1608621-1-scgl@xxxxxxxxxxxxx/

v3 -> v4
* rebase
* ignore key in memop if skey flag not specified
* fix nits in documentation
* pick up tags

v2 -> v3
* get rid of reserved bytes check in vm,vcpu memop
* minor documentation changes
* moved memop selftest patches to end of series and squashed them,
currently working on making the test pretty

v1 -> v2
* rebase
* storage key variants of _?copy_from/to_user instead of
__copy_from/to_user_key, with long key arg instead of char
* refactor protection override checks
* u8 instead of char for key argument in s390 KVM code
* add comments
* pass ar (access register) to trans_exec in access_guest_with_key
* check reserved/unused fields (backwards compatible)
* move key arg of MEMOP out of flags
* rename new MEMOP capability to KVM_CAP_S390_MEM_OP_EXTENSION
* minor changes

Janis Schoetterl-Glausch (10):
s390/uaccess: Add copy_from/to_user_key functions
KVM: s390: Honor storage keys when accessing guest memory
KVM: s390: handle_tprot: Honor storage keys
KVM: s390: selftests: Test TEST PROTECTION emulation
KVM: s390: Add optional storage key checking to MEMOP IOCTL
KVM: s390: Add vm IOCTL for key checked guest absolute memory access
KVM: s390: Rename existing vcpu memop functions
KVM: s390: Add capability for storage key extension of MEM_OP IOCTL
KVM: s390: Update api documentation for memop ioctl
KVM: s390: selftests: Test memops with storage keys

Documentation/virt/kvm/api.rst | 112 ++++-
arch/s390/include/asm/ctl_reg.h | 2 +
arch/s390/include/asm/page.h | 2 +
arch/s390/include/asm/uaccess.h | 22 +
arch/s390/kvm/gaccess.c | 250 +++++++++-
arch/s390/kvm/gaccess.h | 84 +++-
arch/s390/kvm/intercept.c | 12 +-
arch/s390/kvm/kvm-s390.c | 132 ++++-
arch/s390/kvm/priv.c | 66 +--
arch/s390/lib/uaccess.c | 81 +++-
include/uapi/linux/kvm.h | 11 +-
tools/testing/selftests/kvm/.gitignore | 1 +
tools/testing/selftests/kvm/Makefile | 1 +
tools/testing/selftests/kvm/s390x/memop.c | 558 +++++++++++++++++++---
tools/testing/selftests/kvm/s390x/tprot.c | 227 +++++++++
15 files changed, 1375 insertions(+), 186 deletions(-)
create mode 100644 tools/testing/selftests/kvm/s390x/tprot.c

Range-diff against v3:
1: 0049c4412978 = 1: 313eb689b715 s390/uaccess: Add copy_from/to_user_key functions
2: 296096b9a7b9 = 2: 192fe30b1863 KVM: s390: Honor storage keys when accessing guest memory
3: a5976cb3a147 = 3: 19bd017ae5a4 KVM: s390: handle_tprot: Honor storage keys
4: 5f5e056e66df = 4: d20fad8d501b KVM: s390: selftests: Test TEST PROTECTION emulation
5: 64fa17a83b26 ! 5: bdee09b4a15e KVM: s390: Add optional storage key checking to MEMOP IOCTL
@@ Commit message
CPU would, or pass another key if necessary.

Signed-off-by: Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx>
- Acked-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
Reviewed-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>
+ Reviewed-by: Christian Borntraeger <borntraeger@xxxxxxxxxxxxx>
+ Reviewed-by: Janosch Frank <frankja@xxxxxxxxxxxxx>

## arch/s390/kvm/kvm-s390.c ##
-@@
- #include <linux/sched/signal.h>
- #include <linux/string.h>
- #include <linux/pgtable.h>
-+#include <linux/bitfield.h>
-
- #include <asm/asm-offsets.h>
- #include <asm/lowcore.h>
@@ arch/s390/kvm/kvm-s390.c: static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
return r;
}
@@ arch/s390/kvm/kvm-s390.c: static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcp
+ if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
+ if (access_key_invalid(mop->key))
+ return -EINVAL;
++ } else {
++ mop->key = 0;
+ }
if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
tmpbuf = vmalloc(mop->size);
6: 57e3ad332677 ! 6: e207a2f9af8a KVM: s390: Add vm IOCTL for key checked guest absolute memory access
@@ Commit message
accesses and so are not applied as they are when using the vcpu memop.

Signed-off-by: Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx>
- Acked-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
+ Reviewed-by: Christian Borntraeger <borntraeger@xxxxxxxxxxxxx>

## arch/s390/kvm/gaccess.c ##
@@ arch/s390/kvm/gaccess.c: static int low_address_protection_enabled(struct kvm_vcpu *vcpu,
@@ arch/s390/kvm/kvm-s390.c: static bool access_key_invalid(u8 access_key)
+ if (mop->flags & KVM_S390_MEMOP_F_SKEY_PROTECTION) {
+ if (access_key_invalid(mop->key))
+ return -EINVAL;
++ } else {
++ mop->key = 0;
+ }
+ if (!(mop->flags & KVM_S390_MEMOP_F_CHECK_ONLY)) {
+ tmpbuf = vmalloc(mop->size);
7: 1615f5ab6e30 = 7: 52adbceebe41 KVM: s390: Rename existing vcpu memop functions
8: a8420e0f1b7f = 8: 43280a2db282 KVM: s390: Add capability for storage key extension of MEM_OP IOCTL
9: c59952ee362b ! 9: 9389cd2f4d23 KVM: s390: Update api documentation for memop ioctl
@@ Commit message
as well as the existing SIDA operations.

Signed-off-by: Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx>
+ Reviewed-by: Janosch Frank <frankja@xxxxxxxxxxxxx>

## Documentation/virt/kvm/api.rst ##
@@ Documentation/virt/kvm/api.rst: The fields in each entry are defined as follows:
@@ Documentation/virt/kvm/api.rst: Parameters are specified via the following struc
+the access. "ar" designates the access register number to be used; the valid
+range is 0..15.
+Logical accesses are permitted for the VCPU ioctl only.
-+Logical accesses are permitted for non secure guests only.
++Logical accesses are permitted for non-protected guests only.
+
+Supported flags:
+ * ``KVM_S390_MEMOP_F_CHECK_ONLY``
@@ Documentation/virt/kvm/api.rst: Parameters are specified via the following struc
+ * ``KVM_S390_MEMOP_F_SKEY_PROTECTION``
+
+The KVM_S390_MEMOP_F_CHECK_ONLY flag can be set to check whether the
-+corresponding memory access would cause an access exception, without touching
-+the data in memory at the destination.
++corresponding memory access would cause an access exception; however,
++no actual access to the data in memory at the destination is performed.
+In this case, "buf" is unused and can be NULL.
+
+In case an access exception occurred during the access (or would occur
@@ Documentation/virt/kvm/api.rst: Parameters are specified via the following struc
+Absolute accesses are permitted for the VM ioctl if KVM_CAP_S390_MEM_OP_EXTENSION
+is > 0.
+Currently absolute accesses are not permitted for VCPU ioctls.
-+Absolute accesses are permitted for non secure guests only.
++Absolute accesses are permitted for non-protected guests only.
+
+Supported flags:
+ * ``KVM_S390_MEMOP_F_CHECK_ONLY``
@@ Documentation/virt/kvm/api.rst: Parameters are specified via the following struc
+^^^^^^^^^^^^^^^^
+
+Access the secure instruction data area which contains memory operands necessary
-+for instruction emulation for secure guests.
++for instruction emulation for protected guests.
+SIDA accesses are available if the KVM_CAP_S390_PROTECTED capability is available.
+SIDA accesses are permitted for the VCPU ioctl only.
-+SIDA accesses are permitted for secure guests only.
++SIDA accesses are permitted for protected guests only.

-The "reserved" field is meant for future extensions. It is not used by
-KVM with the currently defined set of flags.
10: 68752e1eca95 = 10: af33593d63a4 KVM: s390: selftests: Test memops with storage keys

base-commit: f1baf68e1383f6ed93eb9cff2866d46562607a43
--
2.32.0