[PATCH v3 2/4] KVM: s390: vsie: Fix length of facility list shadowed

Nina Schoetterl-Glausch posted 4 patches 2 years, 1 month ago
There is a newer version of this series
[PATCH v3 2/4] KVM: s390: vsie: Fix length of facility list shadowed
Posted by Nina Schoetterl-Glausch 2 years, 1 month ago
The length of the facility list accessed when interpretively executing
STFLE is the same as the hosts facility list (in case of format-0)
When shadowing, copy only those bytes.
The memory following the facility list need not be accessible, in which
case we'd wrongly inject a validity intercept.

Acked-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
---
 arch/s390/include/asm/facility.h |  6 ++++++
 arch/s390/kernel/Makefile        |  2 +-
 arch/s390/kernel/facility.c      | 21 +++++++++++++++++++++
 arch/s390/kvm/vsie.c             | 12 +++++++++++-
 4 files changed, 39 insertions(+), 2 deletions(-)
 create mode 100644 arch/s390/kernel/facility.c

diff --git a/arch/s390/include/asm/facility.h b/arch/s390/include/asm/facility.h
index 94b6919026df..796007125dff 100644
--- a/arch/s390/include/asm/facility.h
+++ b/arch/s390/include/asm/facility.h
@@ -111,4 +111,10 @@ static inline void stfle(u64 *stfle_fac_list, int size)
 	preempt_enable();
 }
 
+/**
+ * stfle_size - Actual size of the facility list as specified by stfle
+ * (number of double words)
+ */
+unsigned int stfle_size(void);
+
 #endif /* __ASM_FACILITY_H */
diff --git a/arch/s390/kernel/Makefile b/arch/s390/kernel/Makefile
index 0df2b88cc0da..0daa81439478 100644
--- a/arch/s390/kernel/Makefile
+++ b/arch/s390/kernel/Makefile
@@ -41,7 +41,7 @@ obj-y	+= sysinfo.o lgr.o os_info.o
 obj-y	+= runtime_instr.o cache.o fpu.o dumpstack.o guarded_storage.o sthyi.o
 obj-y	+= entry.o reipl.o kdebugfs.o alternative.o
 obj-y	+= nospec-branch.o ipl_vmparm.o machine_kexec_reloc.o unwind_bc.o
-obj-y	+= smp.o text_amode31.o stacktrace.o abs_lowcore.o
+obj-y	+= smp.o text_amode31.o stacktrace.o abs_lowcore.o facility.o
 
 extra-y				+= vmlinux.lds
 
diff --git a/arch/s390/kernel/facility.c b/arch/s390/kernel/facility.c
new file mode 100644
index 000000000000..f02127219a27
--- /dev/null
+++ b/arch/s390/kernel/facility.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright IBM Corp. 2023
+ */
+
+#include <asm/facility.h>
+
+unsigned int stfle_size(void)
+{
+	static unsigned int size;
+	unsigned int r;
+	u64 dummy;
+
+	r = READ_ONCE(size);
+	if (!r) {
+		r = __stfle_asm(&dummy, 1) + 1;
+		WRITE_ONCE(size, r);
+	}
+	return r;
+}
+EXPORT_SYMBOL(stfle_size);
diff --git a/arch/s390/kvm/vsie.c b/arch/s390/kvm/vsie.c
index d989772fe211..c168e88c64da 100644
--- a/arch/s390/kvm/vsie.c
+++ b/arch/s390/kvm/vsie.c
@@ -19,6 +19,7 @@
 #include <asm/nmi.h>
 #include <asm/dis.h>
 #include <asm/fpu/api.h>
+#include <asm/facility.h>
 #include "kvm-s390.h"
 #include "gaccess.h"
 
@@ -990,11 +991,20 @@ static int handle_stfle(struct kvm_vcpu *vcpu, struct vsie_page *vsie_page)
 	struct kvm_s390_sie_block *scb_s = &vsie_page->scb_s;
 	__u32 fac = READ_ONCE(vsie_page->scb_o->fac);
 
+	/*
+	 * Alternate-STFLE-Interpretive-Execution facilities are not supported
+	 * -> format-0 flcb
+	 */
 	if (fac && test_kvm_facility(vcpu->kvm, 7)) {
 		fac = fac & 0x7ffffff8U;
 		retry_vsie_icpt(vsie_page);
+		/*
+		 * format-0 -> size of nested guest's facility list == guest's size
+		 * guest's size == host's size, since STFLE is interpretatively executed
+		 * using a format-0 for the guest, too.
+		 */
 		if (read_guest_real(vcpu, fac, &vsie_page->fac,
-				    sizeof(vsie_page->fac)))
+				    stfle_size() * sizeof(u64)))
 			return set_validity_icpt(scb_s, 0x1090U);
 		scb_s->fac = (__u32)(__u64) &vsie_page->fac;
 	}
-- 
2.39.2
Re: [PATCH v3 2/4] KVM: s390: vsie: Fix length of facility list shadowed
Posted by Janosch Frank 2 years ago
On 11/8/23 18:12, Nina Schoetterl-Glausch wrote:
> The length of the facility list accessed when interpretively executing
> STFLE is the same as the hosts facility list (in case of format-0)
> When shadowing, copy only those bytes.
> The memory following the facility list need not be accessible, in which

...doesn't need to be accessible but the current implementation is 
possibly checking for it to be accessible.

Let's fix that by checking the length that stfle returns to KVM instead 
of a fixed value.

> 
> Acked-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
[...]
Re: [PATCH v3 2/4] KVM: s390: vsie: Fix length of facility list shadowed
Posted by Heiko Carstens 2 years, 1 month ago
On Wed, Nov 08, 2023 at 06:12:27PM +0100, Nina Schoetterl-Glausch wrote:
> The length of the facility list accessed when interpretively executing
> STFLE is the same as the hosts facility list (in case of format-0)
> When shadowing, copy only those bytes.
> The memory following the facility list need not be accessible, in which
> case we'd wrongly inject a validity intercept.
> 
> Acked-by: David Hildenbrand <david@redhat.com>
> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> Signed-off-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> ---
>  arch/s390/include/asm/facility.h |  6 ++++++
>  arch/s390/kernel/Makefile        |  2 +-
>  arch/s390/kernel/facility.c      | 21 +++++++++++++++++++++
>  arch/s390/kvm/vsie.c             | 12 +++++++++++-
>  4 files changed, 39 insertions(+), 2 deletions(-)
>  create mode 100644 arch/s390/kernel/facility.c

For the non-KVM part:
Acked-by: Heiko Carstens <hca@linux.ibm.com>