[LTP] [PATCH 0/7] Two AMD SVM vulnerability tests

[LTP] [PATCH 0/7] Two AMD SVM vulnerability tests

[Martin Doucha]

Here are two more kernel vulnerability tests for AMD SVM:

kvm_svm02 checks that the host kernel intercepts VMLOAD and VMSAVE
instructions in nested VMs and translates guest addresses to the correct
physical address. Without the translation, nested SVM guest can read and
write part of an arbitrary physical memory page chosen by the parent VM.

The test tries to detect the CVE bug harmlessly at first by using VMLOAD
to read a buffer full of zeroes. If it finds any non-zero bytes in the VMCB
fields accessed by VMLOAD/VMSAVE, it'll fail because they must have come
from host memory due to missing address translation.

Since the harmless approach can produce false negatives, it'll also try
a destructive approach and write into memory using VMSAVE. If the bug
is present, the byte pattern in the destination buffer will not change
and the test will reliably fail.

kvm_svm03 checks that the host kernel correctly handles global interrupt
flag in nested VMs. Otherwise a malicious VM could lock up all CPUs
assigned to it, causing a limited denial of service attack.

The test needs synchronization between the VM and another host thread so
the patchset includes the necessary helper functions.

Martin Doucha (7):
  KVM: Add VMSAVE/VMLOAD intercept constants
  Add test for CVE 2021-3656
  lib: Add safe functions for pthread_kill() and mutexes
  KVM: Add async communication helper functions
  KVM: Allow expected KVM_RUN errors in tst_kvm_run_instance()
  KVM: Add STGI/CLGI intercept constants
  Add KVM test for CPU lockup through malicous SVM guest

 doc/kvm-test-api.txt                       |   9 +-
 include/tst_safe_pthread.h                 |  52 +++++++
 lib/safe_pthread.c                         | 161 ++++++++++++++++++++
 runtest/kvm                                |   2 +
 testcases/kernel/kvm/.gitignore            |   2 +
 testcases/kernel/kvm/Makefile              |   3 +
 testcases/kernel/kvm/include/kvm_common.h  |   8 +
 testcases/kernel/kvm/include/kvm_guest.h   |  14 ++
 testcases/kernel/kvm/include/kvm_host.h    |  21 ++-
 testcases/kernel/kvm/include/kvm_x86_svm.h |   7 +
 testcases/kernel/kvm/kvm_svm02.c           | 159 ++++++++++++++++++++
 testcases/kernel/kvm/kvm_svm03.c           | 164 +++++++++++++++++++++
 testcases/kernel/kvm/lib_guest.c           |  16 ++
 testcases/kernel/kvm/lib_host.c            |  46 +++++-
 14 files changed, 656 insertions(+), 8 deletions(-)
 create mode 100644 testcases/kernel/kvm/kvm_svm02.c
 create mode 100644 testcases/kernel/kvm/kvm_svm03.c

-- 
2.40.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 1/7] KVM: Add VMSAVE/VMLOAD intercept constants

[Martin Doucha]

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 testcases/kernel/kvm/include/kvm_x86_svm.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/testcases/kernel/kvm/include/kvm_x86_svm.h b/testcases/kernel/kvm/include/kvm_x86_svm.h
index 965d1e716..3eb832849 100644
--- a/testcases/kernel/kvm/include/kvm_x86_svm.h
+++ b/testcases/kernel/kvm/include/kvm_x86_svm.h
@@ -37,10 +37,15 @@
 /* SVM event intercept IDs */
 #define SVM_INTERCEPT_HLT 0x78
 #define SVM_INTERCEPT_VMRUN 0x80
+#define SVM_INTERCEPT_VMLOAD 0x82
+#define SVM_INTERCEPT_VMSAVE 0x83
 #define SVM_INTERCEPT_MAX 0x95
 
 /* SVM vmrun exit codes */
 #define SVM_EXIT_HLT 0x78
+#define SVM_EXIT_VMRUN 0x80
+#define SVM_EXIT_VMLOAD 0x82
+#define SVM_EXIT_VMSAVE 0x83
 #define SVM_EXIT_AVIC_NOACCEL 0x402
 #define SVM_EXIT_INVALID ((uint64_t)-1)
 
-- 
2.40.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 2/7] Add test for CVE 2021-3656

[Martin Doucha]

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---

The reproducer was verified on vulnerable SLE kernels.

 runtest/kvm                      |   1 +
 testcases/kernel/kvm/.gitignore  |   1 +
 testcases/kernel/kvm/kvm_svm02.c | 159 +++++++++++++++++++++++++++++++
 3 files changed, 161 insertions(+)
 create mode 100644 testcases/kernel/kvm/kvm_svm02.c

diff --git a/runtest/kvm b/runtest/kvm
index 726d72f0a..59e410beb 100644
--- a/runtest/kvm
+++ b/runtest/kvm
@@ -1,2 +1,3 @@
 kvm_pagefault01 kvm_pagefault01
 kvm_svm01 kvm_svm01
+kvm_svm02 kvm_svm02
diff --git a/testcases/kernel/kvm/.gitignore b/testcases/kernel/kvm/.gitignore
index b284b9528..c757cd3f4 100644
--- a/testcases/kernel/kvm/.gitignore
+++ b/testcases/kernel/kvm/.gitignore
@@ -1,2 +1,3 @@
 /kvm_pagefault01
 /kvm_svm01
+/kvm_svm02
diff --git a/testcases/kernel/kvm/kvm_svm02.c b/testcases/kernel/kvm/kvm_svm02.c
new file mode 100644
index 000000000..b89622f47
--- /dev/null
+++ b/testcases/kernel/kvm/kvm_svm02.c
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2023 SUSE LLC
+ * Author: Nicolai Stange <nstange@suse.de>
+ * LTP port: Martin Doucha <mdoucha@suse.cz>
+ */
+
+/*\
+ * CVE 2021-3656
+ *
+ * Check that KVM correctly intercepts VMSAVE and VMLOAD instructions
+ * in a nested virtual machine even when the parent guest disables
+ * intercepting either instruction. If KVM does not override the disabled
+ * intercepts, it'll give the nested VM read/write access to a few bytes
+ * of an arbitrary physical memory page. Unauthorized memory access fixed in:
+ *
+ *  commit c7dfa4009965a9b2d7b329ee970eb8da0d32f0bc
+ *  Author: Maxim Levitsky <mlevitsk@redhat.com>
+ *  Date:   Mon Jul 19 16:05:00 2021 +0300
+ *
+ *  KVM: nSVM: always intercept VMLOAD/VMSAVE when nested (CVE-2021-3656)
+ */
+
+#include "kvm_test.h"
+
+#ifdef COMPILE_PAYLOAD
+#if defined(__i386__) || defined(__x86_64__)
+
+#include "kvm_x86_svm.h"
+
+#define AVIC_REG_ADDR 0x280
+#define AVIC_TEST_VAL 0xec
+#define AVIC_READ_FAIL 0x12ead
+
+#define AVIC_INFO_MASK ((1ULL << 32) | 0xff0)
+#define AVIC_INFO_EXP ((1ULL << 32) | AVIC_REG_ADDR)
+
+static void *vmsave_buf;
+
+/* Load FS, GS, TR and LDTR state from vmsave_buf */
+static int guest_vmload(void)
+{
+	asm (
+		"vmload\n"
+		:
+		: "a" (vmsave_buf)
+	);
+	return 0;
+}
+
+/* Save current FS, GS, TR and LDTR state to vmsave_buf */
+static int guest_vmsave(void)
+{
+	asm (
+		"vmsave\n"
+		:
+		: "a" (vmsave_buf)
+	);
+	return 0;
+}
+
+static int cmp_descriptor(const struct kvm_vmcb_descriptor *a,
+	const struct kvm_vmcb_descriptor *b)
+{
+	int ret;
+
+	ret = a->selector != b->selector;
+	ret = ret || a->attrib != b->attrib;
+	ret = ret || a->limit != b->limit;
+	ret = ret || a->base != b->base;
+	return ret;
+}
+
+/* Return non-zero if the VMCB fields touched by vmsave/vmload differ */
+static int cmp_vmcb(const struct kvm_vmcb *a, const struct kvm_vmcb *b)
+{
+	int ret;
+
+	ret = cmp_descriptor(&a->fs, &b->fs);
+	ret = ret || cmp_descriptor(&a->gs, &b->gs);
+	ret = ret || cmp_descriptor(&a->tr, &b->tr);
+	ret = ret || cmp_descriptor(&a->ldtr, &b->ldtr);
+	ret = ret || a->kernel_gs_base != b->kernel_gs_base;
+	ret = ret || a->star != b->star;
+	ret = ret || a->lstar != b->lstar;
+	ret = ret || a->cstar != b->cstar;
+	ret = ret || a->sfmask != b->sfmask;
+	ret = ret || a->sysenter_cs != b->sysenter_cs;
+	ret = ret || a->sysenter_esp != b->sysenter_esp;
+	ret = ret || a->sysenter_eip != b->sysenter_eip;
+	return ret;
+}
+
+void main(void)
+{
+	uint16_t ss;
+	uint64_t rsp;
+	struct kvm_svm_vcpu *vcpu;
+
+	kvm_init_svm();
+	vcpu = kvm_create_svm_vcpu(guest_vmload, 1);
+	kvm_vmcb_set_intercept(vcpu->vmcb, SVM_INTERCEPT_VMLOAD, 0);
+	vmsave_buf = kvm_alloc_vmcb();
+
+	/* Save allocated stack for later VM reinit */
+	ss = vcpu->vmcb->ss.selector;
+	rsp = vcpu->vmcb->rsp;
+
+	/* Load partial state from vmsave_buf and save it to vcpu->vmcb */
+	kvm_svm_vmrun(vcpu);
+
+	if (vcpu->vmcb->exitcode != SVM_EXIT_HLT)
+		tst_brk(TBROK, "Nested VM exited unexpectedly");
+
+	if (cmp_vmcb(vcpu->vmcb, vmsave_buf)) {
+		tst_res(TFAIL, "Nested VM can read host memory");
+		return;
+	}
+
+	/* Load state from vcpu->vmcb and save it to vmsave_buf */
+	memset(vmsave_buf, 0xaa, sizeof(struct kvm_vmcb));
+	kvm_init_guest_vmcb(vcpu->vmcb, 1, ss, (void *)rsp, guest_vmsave);
+	kvm_vmcb_set_intercept(vcpu->vmcb, SVM_INTERCEPT_VMSAVE, 0);
+	kvm_svm_vmrun(vcpu);
+
+	if (vcpu->vmcb->exitcode != SVM_EXIT_HLT)
+		tst_brk(TBROK, "Nested VM exited unexpectedly");
+
+	if (cmp_vmcb(vcpu->vmcb, vmsave_buf)) {
+		tst_res(TFAIL, "Nested VM can overwrite host memory");
+		return;
+	}
+
+	tst_res(TPASS, "VMLOAD and VMSAVE were intercepted by kernel");
+}
+
+#else /* defined(__i386__) || defined(__x86_64__) */
+TST_TEST_TCONF("Test supported only on x86");
+#endif /* defined(__i386__) || defined(__x86_64__) */
+
+#else /* COMPILE_PAYLOAD */
+
+static struct tst_test test = {
+	.test_all = tst_kvm_run,
+	.setup = tst_kvm_setup,
+	.cleanup = tst_kvm_cleanup,
+	.supported_archs = (const char *const []) {
+		"x86_64",
+		"x86",
+		NULL
+	},
+	.tags = (struct tst_tag[]){
+		{"linux-git", "c7dfa4009965"},
+		{"CVE", "2021-3656"},
+		{}
+	}
+};
+
+#endif /* COMPILE_PAYLOAD */
-- 
2.40.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 3/7] lib: Add safe functions for pthread_kill() and mutexes

[Martin Doucha]

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 include/tst_safe_pthread.h |  52 ++++++++++++
 lib/safe_pthread.c         | 161 +++++++++++++++++++++++++++++++++++++
 2 files changed, 213 insertions(+)

diff --git a/include/tst_safe_pthread.h b/include/tst_safe_pthread.h
index 8fb553929..360b561b6 100644
--- a/include/tst_safe_pthread.h
+++ b/include/tst_safe_pthread.h
@@ -52,4 +52,56 @@ int safe_pthread_cancel(const char *file, const int lineno,
 #define SAFE_PTHREAD_CANCEL(thread_id) \
 	safe_pthread_cancel(__FILE__, __LINE__, thread_id);
 
+int safe_pthread_mutexattr_init(const char *file, const int lineno,
+	pthread_mutexattr_t *attr);
+#define SAFE_PTHREAD_MUTEXATTR_INIT(attr) \
+	safe_pthread_mutexattr_init(__FILE__, __LINE__, (attr))
+
+int safe_pthread_mutexattr_destroy(const char *file, const int lineno,
+	pthread_mutexattr_t *attr);
+#define SAFE_PTHREAD_MUTEXATTR_DESTROY(attr) \
+	safe_pthread_mutexattr_destroy(__FILE__, __LINE__, (attr))
+
+int safe_pthread_mutexattr_settype(const char *file, const int lineno,
+	pthread_mutexattr_t *attr, int type);
+#define SAFE_PTHREAD_MUTEXATTR_SETTYPE(attr, type) \
+	safe_pthread_mutexattr_settype(__FILE__, __LINE__, (attr), (type))
+
+int safe_pthread_mutex_init(const char *file, const int lineno,
+	pthread_mutex_t *mutex, const pthread_mutexattr_t *attr);
+#define SAFE_PTHREAD_MUTEX_INIT(mutex, attr) \
+	safe_pthread_mutex_init(__FILE__, __LINE__, (mutex), (attr))
+
+int safe_pthread_mutex_destroy(const char *file, const int lineno,
+	pthread_mutex_t *mutex);
+#define SAFE_PTHREAD_MUTEX_DESTROY(mutex) \
+	safe_pthread_mutex_destroy(__FILE__, __LINE__, (mutex))
+
+int safe_pthread_mutex_lock(const char *file, const int lineno,
+	pthread_mutex_t *mutex);
+#define SAFE_PTHREAD_MUTEX_LOCK(mutex) \
+	safe_pthread_mutex_lock(__FILE__, __LINE__, (mutex))
+
+/* Terminates the test on any error other than EBUSY */
+int safe_pthread_mutex_trylock(const char *file, const int lineno,
+	pthread_mutex_t *mutex);
+#define SAFE_PTHREAD_MUTEX_TRYLOCK(mutex) \
+	safe_pthread_mutex_trylock(__FILE__, __LINE__, (mutex))
+
+/* Terminates the test on any error other than ETIMEDOUT */
+int safe_pthread_mutex_timedlock(const char *file, const int lineno,
+	pthread_mutex_t *mutex, const struct timespec *abstime);
+#define SAFE_PTHREAD_MUTEX_TIMEDLOCK(mutex, abstime) \
+	safe_pthread_mutex_timedlock(__FILE__, __LINE__, (mutex), (abstime))
+
+int safe_pthread_mutex_unlock(const char *file, const int lineno,
+	pthread_mutex_t *mutex);
+#define SAFE_PTHREAD_MUTEX_UNLOCK(mutex) \
+	safe_pthread_mutex_unlock(__FILE__, __LINE__, (mutex))
+
+int safe_pthread_kill(const char *file, const int lineno,
+	pthread_t thread, int sig);
+#define SAFE_PTHREAD_KILL(thread, sig) \
+	safe_pthread_kill(__FILE__, __LINE__, (thread), (sig))
+
 #endif /* TST_SAFE_PTHREAD_H__ */
diff --git a/lib/safe_pthread.c b/lib/safe_pthread.c
index d70bb8707..aeafe014c 100644
--- a/lib/safe_pthread.c
+++ b/lib/safe_pthread.c
@@ -106,3 +106,164 @@ int safe_pthread_barrier_init(const char *file, const int lineno,
 
 	return rval;
 }
+
+int safe_pthread_mutexattr_init(const char *file, const int lineno,
+	pthread_mutexattr_t *attr)
+{
+	int ret;
+
+	ret = pthread_mutexattr_init(attr);
+
+	if (ret) {
+		tst_brk_(file, lineno, TBROK,
+			"pthread_mutexattr_init(%p) failed: %s",
+			attr, tst_strerrno(ret));
+	}
+
+	return ret;
+}
+
+int safe_pthread_mutexattr_destroy(const char *file, const int lineno,
+	pthread_mutexattr_t *attr)
+{
+	int ret;
+
+	ret = pthread_mutexattr_destroy(attr);
+
+	if (ret) {
+		tst_brk_(file, lineno, TBROK,
+			"pthread_mutexattr_destroy(%p) failed: %s",
+			attr, tst_strerrno(ret));
+	}
+
+	return ret;
+}
+
+int safe_pthread_mutexattr_settype(const char *file, const int lineno,
+	pthread_mutexattr_t *attr, int type)
+{
+	int ret;
+
+	ret = pthread_mutexattr_settype(attr, type);
+
+	if (ret) {
+		tst_brk_(file, lineno, TBROK,
+			"pthread_mutexattr_settype(%p, %d) failed: %s",
+			attr, type, tst_strerrno(ret));
+	}
+
+	return ret;
+}
+
+int safe_pthread_mutex_init(const char *file, const int lineno,
+	pthread_mutex_t *mutex, const pthread_mutexattr_t *attr)
+{
+	int ret;
+
+	ret = pthread_mutex_init(mutex, attr);
+
+	if (ret) {
+		tst_brk_(file, lineno, TBROK,
+			"pthread_mutex_init(%p, %p) failed: %s",
+			mutex, attr, tst_strerrno(ret));
+	}
+
+	return ret;
+}
+
+int safe_pthread_mutex_destroy(const char *file, const int lineno,
+	pthread_mutex_t *mutex)
+{
+	int ret;
+
+	ret = pthread_mutex_destroy(mutex);
+
+	if (ret) {
+		tst_brk_(file, lineno, TBROK,
+			"pthread_mutex_destroy(%p) failed: %s",
+			mutex, tst_strerrno(ret));
+	}
+
+	return ret;
+}
+
+int safe_pthread_mutex_lock(const char *file, const int lineno,
+	pthread_mutex_t *mutex)
+{
+	int ret;
+
+	ret = pthread_mutex_lock(mutex);
+
+	if (ret) {
+		tst_brk_(file, lineno, TBROK,
+			"pthread_mutex_lock(%p) failed: %s",
+			mutex, tst_strerrno(ret));
+	}
+
+	return ret;
+}
+
+int safe_pthread_mutex_trylock(const char *file, const int lineno,
+	pthread_mutex_t *mutex)
+{
+	int ret;
+
+	ret = pthread_mutex_trylock(mutex);
+
+	if (ret && ret != EBUSY) {
+		tst_brk_(file, lineno, TBROK,
+			"pthread_mutex_trylock(%p) failed: %s",
+			mutex, tst_strerrno(ret));
+	}
+
+	return ret;
+}
+
+int safe_pthread_mutex_timedlock(const char *file, const int lineno,
+	pthread_mutex_t *mutex, const struct timespec *abstime)
+{
+	int ret;
+
+	ret = pthread_mutex_timedlock(mutex, abstime);
+
+	if (ret && ret != ETIMEDOUT) {
+		tst_brk_(file, lineno, TBROK,
+			"pthread_mutex_timedlock(%p, {%lld, %ld}) failed: %s",
+			mutex, (long long)abstime->tv_sec, abstime->tv_nsec,
+			tst_strerrno(ret));
+	}
+
+	return ret;
+}
+
+int safe_pthread_mutex_unlock(const char *file, const int lineno,
+	pthread_mutex_t *mutex)
+{
+	int ret;
+
+	ret = pthread_mutex_unlock(mutex);
+
+	if (ret) {
+		tst_brk_(file, lineno, TBROK,
+			"pthread_mutex_unlock(%p) failed: %s",
+			mutex, tst_strerrno(ret));
+	}
+
+	return ret;
+}
+
+int safe_pthread_kill(const char *file, const int lineno,
+	pthread_t thread, int sig)
+{
+	int ret;
+
+	ret = pthread_kill(thread, sig);
+
+	if (ret) {
+		tst_brk_(file, lineno, TBROK,
+			"pthread_kill(..., %d) failed: %s",
+			sig, tst_strerrno(ret));
+	}
+
+	return ret;
+}
-- 
2.40.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 4/7] KVM: Add async communication helper functions

[Martin Doucha]

Some KVM tests need to communicate with VM controller program
without stopping guest execution. Add guest helper functions
for sending asynchronous guest signal to host and waiting for host
to acknowledge the signal. Add host helper functions for waiting
for guest signal and acknowledging it.

Test programs using these functions must have at least two host threads.

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 testcases/kernel/kvm/include/kvm_common.h |  8 ++++++++
 testcases/kernel/kvm/include/kvm_guest.h  | 14 +++++++++++++
 testcases/kernel/kvm/include/kvm_host.h   | 15 ++++++++++++++
 testcases/kernel/kvm/lib_guest.c          | 16 +++++++++++++++
 testcases/kernel/kvm/lib_host.c           | 24 +++++++++++++++++++++++
 5 files changed, 77 insertions(+)

diff --git a/testcases/kernel/kvm/include/kvm_common.h b/testcases/kernel/kvm/include/kvm_common.h
index 4e81d8302..377e3f6aa 100644
--- a/testcases/kernel/kvm/include/kvm_common.h
+++ b/testcases/kernel/kvm/include/kvm_common.h
@@ -10,6 +10,14 @@
 
 #define KVM_TNONE	-1	/* "No result" status value */
 
+/*
+ * Result value for asynchronous notifications between guest and host.
+ * Do not use this value directly. Call tst_signal_host() or tst_wait_host()
+ * in guest code. The notification must be handled by another host thread
+ * and then the result value must be reset to KVM_TNONE.
+ */
+#define KVM_TSYNC	0xfe
+
 /*
  * Result value indicating end of test. If the test program exits using
  * the HLT instruction with any valid result value other than KVM_TEXIT or
diff --git a/testcases/kernel/kvm/include/kvm_guest.h b/testcases/kernel/kvm/include/kvm_guest.h
index ec13c5845..96f246155 100644
--- a/testcases/kernel/kvm/include/kvm_guest.h
+++ b/testcases/kernel/kvm/include/kvm_guest.h
@@ -64,6 +64,20 @@ void tst_brk_(const char *file, const int lineno, int result,
 	const char *message) __attribute__((noreturn));
 #define tst_brk(result, msg) tst_brk_(__FILE__, __LINE__, (result), (msg))
 
+/*
+ * Send asynchronous notification to host without stopping VM execution and
+ * return immediately. The notification must be handled by another host thread.
+ * The data argument will be passed to host in test_result->file_addr and
+ * can be used to send additional data both ways.
+ */
+void tst_signal_host(void *data);
+
+/*
+ * Call tst_signal_host(data) and wait for host to call
+ * tst_kvm_clear_guest_signal().
+ */
+void tst_wait_host(void *data);
+
 void *tst_heap_alloc_aligned(size_t size, size_t align);
 void *tst_heap_alloc(size_t size);
 
diff --git a/testcases/kernel/kvm/include/kvm_host.h b/testcases/kernel/kvm/include/kvm_host.h
index 2359944fd..cbbdadc06 100644
--- a/testcases/kernel/kvm/include/kvm_host.h
+++ b/testcases/kernel/kvm/include/kvm_host.h
@@ -134,4 +134,19 @@ void tst_kvm_run_instance(struct tst_kvm_instance *inst);
  */
 void tst_kvm_destroy_instance(struct tst_kvm_instance *inst);
 
+/*
+ * Wait for given VM to call tst_signal_host() or tst_wait_host(). Timeout
+ * value is in seconds. Zero means no wait, negative value means wait forever.
+ * Note that the actual wait time may be up to 1 second shorter due to time()
+ * granularity. Returns 0 if signal was received, KVM_TEXIT if the VM exited
+ * without sending a signal, or -1 if timeout was reached.
+ */
+int tst_kvm_wait_guest(struct tst_kvm_instance *inst, int timeout);
+
+/*
+ * Clear VM signal sent by tst_signal_host(). If the VM is waiting
+ * in tst_wait_host(), this function will signal the VM to resume execution.
+ */
+void tst_kvm_clear_guest_signal(struct tst_kvm_instance *inst);
+
 #endif /* KVM_HOST_H_ */
diff --git a/testcases/kernel/kvm/lib_guest.c b/testcases/kernel/kvm/lib_guest.c
index d3b2ac3d5..f3e21d3d6 100644
--- a/testcases/kernel/kvm/lib_guest.c
+++ b/testcases/kernel/kvm/lib_guest.c
@@ -155,6 +155,22 @@ void tst_brk_(const char *file, const int lineno, int result,
 	kvm_exit();
 }
 
+void tst_signal_host(void *data)
+{
+	test_result->file_addr = (uintptr_t)data;
+	test_result->result = KVM_TSYNC;
+}
+
+void tst_wait_host(void *data)
+{
+	volatile int32_t *vres = &test_result->result;
+
+	tst_signal_host(data);
+
+	while (*vres != KVM_TNONE)
+		;
+}
+
 void tst_handle_interrupt(struct kvm_interrupt_frame *ifrm, long vector,
 	unsigned long errcode)
 {
diff --git a/testcases/kernel/kvm/lib_host.c b/testcases/kernel/kvm/lib_host.c
index 2782e68b0..a5f05449c 100644
--- a/testcases/kernel/kvm/lib_host.c
+++ b/testcases/kernel/kvm/lib_host.c
@@ -272,6 +272,30 @@ void tst_kvm_destroy_instance(struct tst_kvm_instance *inst)
 	memset(inst->ram, 0, sizeof(inst->ram));
 }
 
+int tst_kvm_wait_guest(struct tst_kvm_instance *inst, int timeout)
+{
+	volatile struct tst_kvm_result *result = inst->result;
+	int32_t res;
+	time_t start = time(NULL);
+
+	while ((res = result->result) != KVM_TSYNC) {
+		if (res == KVM_TEXIT)
+			return res;
+
+		if (timeout >= 0 && start + timeout <= time(NULL))
+			return -1;
+
+		usleep(1000);
+	}
+
+	return 0;
+}
+
+void tst_kvm_clear_guest_signal(struct tst_kvm_instance *inst)
+{
+	inst->result->result = KVM_TNONE;
+}
+
 void tst_kvm_setup(void)
 {
 
-- 
2.40.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 5/7] KVM: Allow expected KVM_RUN errors in tst_kvm_run_instance()

[Martin Doucha]

Add a new parameter to tst_kvm_run_instance() for expected errno value,
e.g. EINTR for testing the possiblity to kill running guest. When
ioctl(KVM_RUN) fails with the expected errno, tst_kvm_run_instance()
will return -1 without terminating the test.

Default tst_kvm_run() function expects no KVM_RUN errors.

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 doc/kvm-test-api.txt                    |  9 ++++++---
 testcases/kernel/kvm/include/kvm_host.h |  6 ++++--
 testcases/kernel/kvm/lib_host.c         | 22 +++++++++++++++++++---
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/doc/kvm-test-api.txt b/doc/kvm-test-api.txt
index 812e12b38..7e537afcb 100644
--- a/doc/kvm-test-api.txt
+++ b/doc/kvm-test-api.txt
@@ -194,9 +194,12 @@ previously allocated guarded buffers.
   with at least `ram_size` bytes of memory. The VM instance info will be
   stored in `inst`.
 
-- `void tst_kvm_run_instance(struct tst_kvm_instance *inst)` – Executes
-  the program installed in KVM virtual machine `inst`. Any result messages
-  returned by the VM will be automatically printed to controller program output.
+- `int tst_kvm_run_instance(struct tst_kvm_instance *inst, int exp_errno)` –
+  Executes the program installed in KVM virtual machine `inst`. Any result
+  messages returned by the VM will be automatically printed to controller
+  program output. Returns zero. If `exp_errno` is non-zero, the VM execution
+  syscall is allowed to fail with the `exp_errno` error code and
+  `tst_kvm_run_instance()` will return -1 instead of terminating the test.
 
 - `void tst_kvm_destroy_instance(struct tst_kvm_instance *inst)` – Deletes
   the KVM virtual machine `inst`. Note that the guarded buffers assigned
diff --git a/testcases/kernel/kvm/include/kvm_host.h b/testcases/kernel/kvm/include/kvm_host.h
index cbbdadc06..3e60d4738 100644
--- a/testcases/kernel/kvm/include/kvm_host.h
+++ b/testcases/kernel/kvm/include/kvm_host.h
@@ -125,9 +125,11 @@ struct kvm_cpuid2 *tst_kvm_get_cpuid(int sysfd);
 void tst_kvm_create_instance(struct tst_kvm_instance *inst, size_t ram_size);
 
 /*
- * Execute the given KVM instance and print results.
+ * Execute the given KVM instance and print results. If ioctl(KVM_RUN) is
+ * expected to fail, pass the expected error code in exp_errno, otherwise
+ * set it to zero. Returns last value returned by ioctl(KVM_RUN).
  */
-void tst_kvm_run_instance(struct tst_kvm_instance *inst);
+int tst_kvm_run_instance(struct tst_kvm_instance *inst, int exp_errno);
 
 /*
  * Close the given KVM instance.
diff --git a/testcases/kernel/kvm/lib_host.c b/testcases/kernel/kvm/lib_host.c
index a5f05449c..e33a5f5aa 100644
--- a/testcases/kernel/kvm/lib_host.c
+++ b/testcases/kernel/kvm/lib_host.c
@@ -234,14 +234,28 @@ void tst_kvm_create_instance(struct tst_kvm_instance *inst, size_t ram_size)
 	inst->result->message[0] = '\0';
 }
 
-void tst_kvm_run_instance(struct tst_kvm_instance *inst)
+int tst_kvm_run_instance(struct tst_kvm_instance *inst, int exp_errno)
 {
 	struct kvm_regs regs;
+	int ret;
 
 	while (1) {
 		inst->result->result = KVM_TNONE;
 		inst->result->message[0] = '\0';
-		SAFE_IOCTL(inst->vcpu_fd, KVM_RUN, 0);
+		errno = 0;
+		ret = ioctl(inst->vcpu_fd, KVM_RUN, 0);
+
+		if (ret == -1) {
+			if (errno == exp_errno)
+				return ret;
+
+			tst_brk(TBROK | TERRNO, "ioctl(KVM_RUN) failed");
+		}
+
+		if (ret < 0) {
+			tst_brk(TBROK | TERRNO,
+				"Invalid ioctl(KVM_RUN) return value %d", ret);
+		}
 
 		if (inst->vcpu_info->exit_reason != KVM_EXIT_HLT) {
 			SAFE_IOCTL(inst->vcpu_fd, KVM_GET_REGS, &regs);
@@ -255,6 +269,8 @@ void tst_kvm_run_instance(struct tst_kvm_instance *inst)
 
 		tst_kvm_print_result(inst);
 	}
+
+	return ret;
 }
 
 void tst_kvm_destroy_instance(struct tst_kvm_instance *inst)
@@ -304,7 +320,7 @@ void tst_kvm_setup(void)
 void tst_kvm_run(void)
 {
 	tst_kvm_create_instance(&test_vm, DEFAULT_RAM_SIZE);
-	tst_kvm_run_instance(&test_vm);
+	tst_kvm_run_instance(&test_vm, 0);
 	tst_kvm_destroy_instance(&test_vm);
 	tst_free_all();
 }
-- 
2.40.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 6/7] KVM: Add STGI/CLGI intercept constants

[Martin Doucha]

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---
 testcases/kernel/kvm/include/kvm_x86_svm.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/testcases/kernel/kvm/include/kvm_x86_svm.h b/testcases/kernel/kvm/include/kvm_x86_svm.h
index 3eb832849..89dc9c39f 100644
--- a/testcases/kernel/kvm/include/kvm_x86_svm.h
+++ b/testcases/kernel/kvm/include/kvm_x86_svm.h
@@ -39,6 +39,8 @@
 #define SVM_INTERCEPT_VMRUN 0x80
 #define SVM_INTERCEPT_VMLOAD 0x82
 #define SVM_INTERCEPT_VMSAVE 0x83
+#define SVM_INTERCEPT_STGI 0x84
+#define SVM_INTERCEPT_CLGI 0x85
 #define SVM_INTERCEPT_MAX 0x95
 
 /* SVM vmrun exit codes */
-- 
2.40.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH 7/7] Add KVM test for CPU lockup through malicous SVM guest

[Martin Doucha]

Signed-off-by: Martin Doucha <mdoucha@suse.cz>
---

The reproducer was verified on vulnerable SLE kernels.

 runtest/kvm                      |   1 +
 testcases/kernel/kvm/.gitignore  |   1 +
 testcases/kernel/kvm/Makefile    |   3 +
 testcases/kernel/kvm/kvm_svm03.c | 164 +++++++++++++++++++++++++++++++
 4 files changed, 169 insertions(+)
 create mode 100644 testcases/kernel/kvm/kvm_svm03.c

diff --git a/runtest/kvm b/runtest/kvm
index 59e410beb..4094a21a8 100644
--- a/runtest/kvm
+++ b/runtest/kvm
@@ -1,3 +1,4 @@
 kvm_pagefault01 kvm_pagefault01
 kvm_svm01 kvm_svm01
 kvm_svm02 kvm_svm02
+kvm_svm03 kvm_svm03
diff --git a/testcases/kernel/kvm/.gitignore b/testcases/kernel/kvm/.gitignore
index c757cd3f4..9638a6fc7 100644
--- a/testcases/kernel/kvm/.gitignore
+++ b/testcases/kernel/kvm/.gitignore
@@ -1,3 +1,4 @@
 /kvm_pagefault01
 /kvm_svm01
 /kvm_svm02
+/kvm_svm03
diff --git a/testcases/kernel/kvm/Makefile b/testcases/kernel/kvm/Makefile
index e12cb4e98..03d754420 100644
--- a/testcases/kernel/kvm/Makefile
+++ b/testcases/kernel/kvm/Makefile
@@ -48,6 +48,9 @@ endif
 lib_guest.o $(ARCH_OBJ): CPPFLAGS	:= $(GUEST_CPPFLAGS)
 lib_guest.o $(ARCH_OBJ): CFLAGS		:= $(GUEST_CFLAGS)
 
+kvm_svm03: CFLAGS += -pthread
+kvm_svm03: LDLIBS += -pthread -lrt
+
 include $(top_srcdir)/include/mk/generic_leaf_target.mk
 
 %-payload.o: %.c lib_guest.o $(ARCH_OBJ)
diff --git a/testcases/kernel/kvm/kvm_svm03.c b/testcases/kernel/kvm/kvm_svm03.c
new file mode 100644
index 000000000..365df789e
--- /dev/null
+++ b/testcases/kernel/kvm/kvm_svm03.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2023 SUSE LLC
+ * Author: Nicolai Stange <nstange@suse.de>
+ * LTP port: Martin Doucha <mdoucha@suse.cz>
+ */
+
+/*\
+ * Check that KVM correctly intercepts the CLGI instruction in a nested
+ * virtual machine even when the parent guest disables intercept.
+ * If KVM does not override the disabled intercept, it'll allow the nested VM
+ * to hold the physical CPU indefinitely and potentially perform a denial
+ * of service attack against the host kernel. CPU lockup fixed in:
+ *
+ *  commit 91b7130cb6606d8c6b3b77e54426b3f3a83f48b1
+ *  Author: Paolo Bonzini <pbonzini@redhat.com>
+ *  Date:   Fri May 22 12:28:52 2020 -0400
+ *
+ *  KVM: SVM: preserve VGIF across VMCB switch
+ */
+
+#include "kvm_test.h"
+
+#ifdef COMPILE_PAYLOAD
+#if defined(__i386__) || defined(__x86_64__)
+
+#include "kvm_x86_svm.h"
+
+/* Disable global interrupts */
+static int guest_clgi(void)
+{
+	int ret, *result = (int *)KVM_RESULT_BASEADDR;
+
+	/*
+	 * Make sure that result page is present in memory. CLGI may disable
+	 * page fault handling on the current CPU. The actual value
+	 * at that address is irrelevant.
+	 */
+	ret = *result;
+
+	/* Disable global interrupts */
+	asm ("clgi");
+
+	/* Signal host to kill the VM and wait */
+	tst_wait_host(NULL);
+	return ret;
+}
+
+void main(void)
+{
+	struct kvm_svm_vcpu *vcpu;
+
+	kvm_init_svm();
+	vcpu = kvm_create_svm_vcpu(guest_clgi, 1);
+	kvm_vmcb_set_intercept(vcpu->vmcb, SVM_INTERCEPT_CLGI, 0);
+	kvm_svm_vmrun(vcpu);
+
+	if (vcpu->vmcb->exitcode != SVM_EXIT_HLT)
+		tst_brk(TBROK, "Nested VM exited unexpectedly");
+}
+
+#else /* defined(__i386__) || defined(__x86_64__) */
+TST_TEST_TCONF("Test supported only on x86");
+#endif /* defined(__i386__) || defined(__x86_64__) */
+
+#else /* COMPILE_PAYLOAD */
+
+#include <pthread.h>
+#include "tst_safe_pthread.h"
+#include "tst_safe_clocks.h"
+
+static struct tst_kvm_instance test_vm = { .vm_fd = -1 };
+static pthread_mutex_t mutex;
+
+static void sighandler(int sig LTP_ATTRIBUTE_UNUSED)
+{
+
+}
+
+static void *vm_thread(void *arg)
+{
+	SAFE_PTHREAD_MUTEX_LOCK(&mutex);
+	tst_kvm_run_instance(&test_vm, EINTR);
+	SAFE_PTHREAD_MUTEX_UNLOCK(&mutex);
+	return arg;
+}
+
+static void setup(void)
+{
+	struct sigaction sa = { .sa_handler = sighandler };
+	pthread_mutexattr_t attr;
+
+	SAFE_PTHREAD_MUTEXATTR_INIT(&attr);
+	SAFE_PTHREAD_MUTEXATTR_SETTYPE(&attr, PTHREAD_MUTEX_NORMAL);
+	SAFE_PTHREAD_MUTEX_INIT(&mutex, &attr);
+	SAFE_PTHREAD_MUTEXATTR_DESTROY(&attr);
+	SAFE_SIGACTION(SIGUSR1, &sa, NULL);
+}
+
+static void run(void)
+{
+	struct timespec timeout;
+	pthread_t tid;
+	int ret;
+
+	tst_kvm_create_instance(&test_vm, DEFAULT_RAM_SIZE);
+
+	SAFE_PTHREAD_CREATE(&tid, NULL, vm_thread, NULL);
+	ret = tst_kvm_wait_guest(&test_vm, 2);
+
+	if (ret == KVM_TEXIT) {
+		SAFE_PTHREAD_JOIN(tid, NULL);
+		tst_brk(TCONF, "Guest exited early");
+	}
+
+	if (ret)
+		tst_brk(TBROK, "Wait for guest initialization timed out");
+
+	SAFE_PTHREAD_KILL(tid, SIGUSR1);
+	SAFE_CLOCK_GETTIME(CLOCK_REALTIME, &timeout);
+	timeout.tv_sec += 2;
+
+	if (SAFE_PTHREAD_MUTEX_TIMEDLOCK(&mutex, &timeout)) {
+		tst_kvm_clear_guest_signal(&test_vm);
+		tst_res(TFAIL, "VM thread does not respond to signals");
+	} else {
+		SAFE_PTHREAD_MUTEX_UNLOCK(&mutex);
+		tst_res(TPASS, "VM thread was interrupted by signal");
+	}
+
+	SAFE_PTHREAD_JOIN(tid, NULL);
+	tst_kvm_destroy_instance(&test_vm);
+	tst_free_all();
+}
+
+static void cleanup(void)
+{
+	/* VM is likely still running, cannot clean up anything */
+	if (SAFE_PTHREAD_MUTEX_TRYLOCK(&mutex))
+		return;
+
+	if (!SAFE_PTHREAD_MUTEX_UNLOCK(&mutex))
+		SAFE_PTHREAD_MUTEX_DESTROY(&mutex);
+
+	tst_kvm_destroy_instance(&test_vm);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.min_cpus = 2,
+	.supported_archs = (const char *const []) {
+		"x86_64",
+		"x86",
+		NULL
+	},
+	.tags = (struct tst_tag[]){
+		{"linux-git", "91b7130cb660"},
+		{}
+	}
+};
+
+#endif /* COMPILE_PAYLOAD */
-- 
2.40.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/7] KVM: Add VMSAVE/VMLOAD intercept constants

[Petr Vorel]

Hi Martin,

> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
> ---
>  testcases/kernel/kvm/include/kvm_x86_svm.h | 5 +++++
>  1 file changed, 5 insertions(+)

> diff --git a/testcases/kernel/kvm/include/kvm_x86_svm.h b/testcases/kernel/kvm/include/kvm_x86_svm.h
> index 965d1e716..3eb832849 100644
> --- a/testcases/kernel/kvm/include/kvm_x86_svm.h
> +++ b/testcases/kernel/kvm/include/kvm_x86_svm.h
> @@ -37,10 +37,15 @@
>  /* SVM event intercept IDs */
>  #define SVM_INTERCEPT_HLT 0x78
>  #define SVM_INTERCEPT_VMRUN 0x80
> +#define SVM_INTERCEPT_VMLOAD 0x82
> +#define SVM_INTERCEPT_VMSAVE 0x83
>  #define SVM_INTERCEPT_MAX 0x95

>  /* SVM vmrun exit codes */
>  #define SVM_EXIT_HLT 0x78
> +#define SVM_EXIT_VMRUN 0x80
> +#define SVM_EXIT_VMLOAD 0x82
> +#define SVM_EXIT_VMSAVE 0x83
It looks like these three aren't used anywhere.

Kind regards,
Petr

>  #define SVM_EXIT_AVIC_NOACCEL 0x402
>  #define SVM_EXIT_INVALID ((uint64_t)-1)

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/7] Add test for CVE 2021-3656

[Petr Vorel]

Hi Martin,

> +++ b/testcases/kernel/kvm/kvm_svm02.c
...
> +
> +#ifdef COMPILE_PAYLOAD
> +#if defined(__i386__) || defined(__x86_64__)
> +
> +#include "kvm_x86_svm.h"
> +
> +#define AVIC_REG_ADDR 0x280
> +#define AVIC_TEST_VAL 0xec
> +#define AVIC_READ_FAIL 0x12ead
> +
> +#define AVIC_INFO_MASK ((1ULL << 32) | 0xff0)
> +#define AVIC_INFO_EXP ((1ULL << 32) | AVIC_REG_ADDR)

These five constants aren't used at all.
Maybe copy paste from kvm_svm01.c ?

Acked-by: Petr Vorel <pvorel@suse.cz>

The rest LGTM, but I'm not the right person to review.
It'd be great if Nicolai would review.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 4/7] KVM: Add async communication helper functions

[Petr Vorel]

Hi Martin,

Acked-by: Petr Vorel <pvorel@suse.cz>

>  /*
>   * Result value indicating end of test. If the test program exits using
>   * the HLT instruction with any valid result value other than KVM_TEXIT or
> diff --git a/testcases/kernel/kvm/include/kvm_guest.h b/testcases/kernel/kvm/include/kvm_guest.h
> index ec13c5845..96f246155 100644
> --- a/testcases/kernel/kvm/include/kvm_guest.h
> +++ b/testcases/kernel/kvm/include/kvm_guest.h
> @@ -64,6 +64,20 @@ void tst_brk_(const char *file, const int lineno, int result,
>  	const char *message) __attribute__((noreturn));
>  #define tst_brk(result, msg) tst_brk_(__FILE__, __LINE__, (result), (msg))

BTW (unrelated to this commit, as it was not added to this commit) tst_brk()
definition from include/tst_test.h contains also
TST_BRK_SUPPORTS_ONLY_TCONF_TBROK().

And tst_res() has also TST_RES_SUPPORTS_TCONF_TFAIL_TINFO_TPASS_TWARN().

We could rename tst_res_flags.h to tst_res.h and add there tst_brk() and
tst_res() definitions and tst_brk_() and tst_res_() signatures.

> +
>  #endif /* KVM_HOST_H_ */
> diff --git a/testcases/kernel/kvm/lib_guest.c b/testcases/kernel/kvm/lib_guest.c
> index d3b2ac3d5..f3e21d3d6 100644
> --- a/testcases/kernel/kvm/lib_guest.c
> +++ b/testcases/kernel/kvm/lib_guest.c
> @@ -155,6 +155,22 @@ void tst_brk_(const char *file, const int lineno, int result,
>  	kvm_exit();
>  }

> +void tst_signal_host(void *data)
> +{
> +	test_result->file_addr = (uintptr_t)data;
> +	test_result->result = KVM_TSYNC;
> +}
> +
> +void tst_wait_host(void *data)
> +{
> +	volatile int32_t *vres = &test_result->result;
> +
> +	tst_signal_host(data);
> +
> +	while (*vres != KVM_TNONE)
> +		;
Interesting there is no any usleep().

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/7] lib: Add safe functions for pthread_kill() and mutexes

[Petr Vorel]

Hi Martin,

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 6/7] KVM: Add STGI/CLGI intercept constants

[Petr Vorel]

Hi Martin,

> +++ b/testcases/kernel/kvm/include/kvm_x86_svm.h
> @@ -39,6 +39,8 @@
>  #define SVM_INTERCEPT_VMRUN 0x80
>  #define SVM_INTERCEPT_VMLOAD 0x82
>  #define SVM_INTERCEPT_VMSAVE 0x83
> +#define SVM_INTERCEPT_STGI 0x84
SVM_INTERCEPT_STGI is not used anywhere.

Kind regards,
Petr

> +#define SVM_INTERCEPT_CLGI 0x85

>  #define SVM_INTERCEPT_MAX 0x95

>  /* SVM vmrun exit codes */

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/7] KVM: Add VMSAVE/VMLOAD intercept constants

[Martin Doucha]

On 18. 05. 23 0:08, Petr Vorel wrote:
> Hi Martin,
> 
>> Signed-off-by: Martin Doucha <mdoucha@suse.cz>
>> ---
>>   testcases/kernel/kvm/include/kvm_x86_svm.h | 5 +++++
>>   1 file changed, 5 insertions(+)
> 
>> diff --git a/testcases/kernel/kvm/include/kvm_x86_svm.h b/testcases/kernel/kvm/include/kvm_x86_svm.h
>> index 965d1e716..3eb832849 100644
>> --- a/testcases/kernel/kvm/include/kvm_x86_svm.h
>> +++ b/testcases/kernel/kvm/include/kvm_x86_svm.h
>> @@ -37,10 +37,15 @@
>>   /* SVM event intercept IDs */
>>   #define SVM_INTERCEPT_HLT 0x78
>>   #define SVM_INTERCEPT_VMRUN 0x80
>> +#define SVM_INTERCEPT_VMLOAD 0x82
>> +#define SVM_INTERCEPT_VMSAVE 0x83
>>   #define SVM_INTERCEPT_MAX 0x95
> 
>>   /* SVM vmrun exit codes */
>>   #define SVM_EXIT_HLT 0x78
>> +#define SVM_EXIT_VMRUN 0x80
>> +#define SVM_EXIT_VMLOAD 0x82
>> +#define SVM_EXIT_VMSAVE 0x83
> It looks like these three aren't used anywhere.

Yes, the SVM_EXIT_* constants are not used. I originally wanted to use 
them in one of the checks but then decided against it. But let's keep 
them anyway since they'll be needed to handle the intercepts in future 
tests.

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/7] Add test for CVE 2021-3656

[Martin Doucha]

On 18. 05. 23 0:32, Petr Vorel wrote:
> Hi Martin,
> 
>> +++ b/testcases/kernel/kvm/kvm_svm02.c
> ...
>> +
>> +#ifdef COMPILE_PAYLOAD
>> +#if defined(__i386__) || defined(__x86_64__)
>> +
>> +#include "kvm_x86_svm.h"
>> +
>> +#define AVIC_REG_ADDR 0x280
>> +#define AVIC_TEST_VAL 0xec
>> +#define AVIC_READ_FAIL 0x12ead
>> +
>> +#define AVIC_INFO_MASK ((1ULL << 32) | 0xff0)
>> +#define AVIC_INFO_EXP ((1ULL << 32) | AVIC_REG_ADDR)
> 
> These five constants aren't used at all.
> Maybe copy paste from kvm_svm01.c ?

Yes, the AVIC constants are leftovers from copying kvm_svm01.c, sorry 
about that. Should I send a v2 after other patches get merged?

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 4/7] KVM: Add async communication helper functions

[Martin Doucha]

On 18. 05. 23 0:47, Petr Vorel wrote:
> Hi Martin,
> 
> Acked-by: Petr Vorel <pvorel@suse.cz>
> 
>>   /*
>>    * Result value indicating end of test. If the test program exits using
>>    * the HLT instruction with any valid result value other than KVM_TEXIT or
>> diff --git a/testcases/kernel/kvm/include/kvm_guest.h b/testcases/kernel/kvm/include/kvm_guest.h
>> index ec13c5845..96f246155 100644
>> --- a/testcases/kernel/kvm/include/kvm_guest.h
>> +++ b/testcases/kernel/kvm/include/kvm_guest.h
>> @@ -64,6 +64,20 @@ void tst_brk_(const char *file, const int lineno, int result,
>>   	const char *message) __attribute__((noreturn));
>>   #define tst_brk(result, msg) tst_brk_(__FILE__, __LINE__, (result), (msg))
> 
> BTW (unrelated to this commit, as it was not added to this commit) tst_brk()
> definition from include/tst_test.h contains also
> TST_BRK_SUPPORTS_ONLY_TCONF_TBROK().
> 
> And tst_res() has also TST_RES_SUPPORTS_TCONF_TFAIL_TINFO_TPASS_TWARN().

The support macros are not needed, the controller program always handles 
TBROK using tst_brk() even if the guest incorrectly called tst_res(TBROK).

> We could rename tst_res_flags.h to tst_res.h and add there tst_brk() and
> tst_res() definitions and tst_brk_() and tst_res_() signatures.

I don't think that's a good idea. The guest library currently does not 
support variadic parameters for tst_res() and tst_brk() so the function 
signatures are slightly different.

>> diff --git a/testcases/kernel/kvm/lib_guest.c b/testcases/kernel/kvm/lib_guest.c
>> index d3b2ac3d5..f3e21d3d6 100644
>> --- a/testcases/kernel/kvm/lib_guest.c
>> +++ b/testcases/kernel/kvm/lib_guest.c
>> @@ -155,6 +155,22 @@ void tst_brk_(const char *file, const int lineno, int result,
>>   	kvm_exit();
>>   }
> 
>> +void tst_signal_host(void *data)
>> +{
>> +	test_result->file_addr = (uintptr_t)data;
>> +	test_result->result = KVM_TSYNC;
>> +}
>> +
>> +void tst_wait_host(void *data)
>> +{
>> +	volatile int32_t *vres = &test_result->result;
>> +
>> +	tst_signal_host(data);
>> +
>> +	while (*vres != KVM_TNONE)
>> +		;
> Interesting there is no any usleep().

usleep() does not exist in the guest library. And even if it did, 
yielding the CPU to the host would prevent kvm_svm03 from ever 
reproducing the CLGI bug.

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 6/7] KVM: Add STGI/CLGI intercept constants

[Martin Doucha]

On 18. 05. 23 1:20, Petr Vorel wrote:
> Hi Martin,
> 
>> +++ b/testcases/kernel/kvm/include/kvm_x86_svm.h
>> @@ -39,6 +39,8 @@
>>   #define SVM_INTERCEPT_VMRUN 0x80
>>   #define SVM_INTERCEPT_VMLOAD 0x82
>>   #define SVM_INTERCEPT_VMSAVE 0x83
>> +#define SVM_INTERCEPT_STGI 0x84
> SVM_INTERCEPT_STGI is not used anywhere.

I know. Let's keep it anyway. But maybe I should also add SVM_EXIT 
constants in v2?

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 1/7] KVM: Add VMSAVE/VMLOAD intercept constants

[Petr Vorel]

...
> > >   /* SVM vmrun exit codes */
> > >   #define SVM_EXIT_HLT 0x78
> > > +#define SVM_EXIT_VMRUN 0x80
> > > +#define SVM_EXIT_VMLOAD 0x82
> > > +#define SVM_EXIT_VMSAVE 0x83
> > It looks like these three aren't used anywhere.

> Yes, the SVM_EXIT_* constants are not used. I originally wanted to use them
> in one of the checks but then decided against it. But let's keep them anyway
> since they'll be needed to handle the intercepts in future tests.

Sure, no problem.

Acked-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 6/7] KVM: Add STGI/CLGI intercept constants

[Petr Vorel]

> On 18. 05. 23 1:20, Petr Vorel wrote:
> > Hi Martin,

> > > +++ b/testcases/kernel/kvm/include/kvm_x86_svm.h
> > > @@ -39,6 +39,8 @@
> > >   #define SVM_INTERCEPT_VMRUN 0x80
> > >   #define SVM_INTERCEPT_VMLOAD 0x82
> > >   #define SVM_INTERCEPT_VMSAVE 0x83
> > > +#define SVM_INTERCEPT_STGI 0x84
> > SVM_INTERCEPT_STGI is not used anywhere.

> I know. Let's keep it anyway. But maybe I should also add SVM_EXIT constants
> in v2?

It's a detail, but yeah it'd make more sense.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 4/7] KVM: Add async communication helper functions

[Petr Vorel]

> On 18. 05. 23 0:47, Petr Vorel wrote:
> > Hi Martin,

> > Acked-by: Petr Vorel <pvorel@suse.cz>

> > >   /*
> > >    * Result value indicating end of test. If the test program exits using
> > >    * the HLT instruction with any valid result value other than KVM_TEXIT or
> > > diff --git a/testcases/kernel/kvm/include/kvm_guest.h b/testcases/kernel/kvm/include/kvm_guest.h
> > > index ec13c5845..96f246155 100644
> > > --- a/testcases/kernel/kvm/include/kvm_guest.h
> > > +++ b/testcases/kernel/kvm/include/kvm_guest.h
> > > @@ -64,6 +64,20 @@ void tst_brk_(const char *file, const int lineno, int result,
> > >   	const char *message) __attribute__((noreturn));
> > >   #define tst_brk(result, msg) tst_brk_(__FILE__, __LINE__, (result), (msg))

> > BTW (unrelated to this commit, as it was not added to this commit) tst_brk()
> > definition from include/tst_test.h contains also
> > TST_BRK_SUPPORTS_ONLY_TCONF_TBROK().

> > And tst_res() has also TST_RES_SUPPORTS_TCONF_TFAIL_TINFO_TPASS_TWARN().

> The support macros are not needed, the controller program always handles
> TBROK using tst_brk() even if the guest incorrectly called tst_res(TBROK).

ack

> > We could rename tst_res_flags.h to tst_res.h and add there tst_brk() and
> > tst_res() definitions and tst_brk_() and tst_res_() signatures.

> I don't think that's a good idea. The guest library currently does not
> support variadic parameters for tst_res() and tst_brk() so the function
> signatures are slightly different.

Ah, right.

> > > diff --git a/testcases/kernel/kvm/lib_guest.c b/testcases/kernel/kvm/lib_guest.c
> > > index d3b2ac3d5..f3e21d3d6 100644
> > > --- a/testcases/kernel/kvm/lib_guest.c
> > > +++ b/testcases/kernel/kvm/lib_guest.c
> > > @@ -155,6 +155,22 @@ void tst_brk_(const char *file, const int lineno, int result,
> > >   	kvm_exit();
> > >   }

> > > +void tst_signal_host(void *data)
> > > +{
> > > +	test_result->file_addr = (uintptr_t)data;
> > > +	test_result->result = KVM_TSYNC;
> > > +}
> > > +
> > > +void tst_wait_host(void *data)
> > > +{
> > > +	volatile int32_t *vres = &test_result->result;
> > > +
> > > +	tst_signal_host(data);
> > > +
> > > +	while (*vres != KVM_TNONE)
> > > +		;
> > Interesting there is no any usleep().

> usleep() does not exist in the guest library. And even if it did, yielding
> the CPU to the host would prevent kvm_svm03 from ever reproducing the CLGI
> bug.

Ah, I thought I'm missing something obvious, thanks for an explanation.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 2/7] Add test for CVE 2021-3656

[Petr Vorel]

> On 18. 05. 23 0:32, Petr Vorel wrote:
> > Hi Martin,

> > > +++ b/testcases/kernel/kvm/kvm_svm02.c
> > ...
> > > +
> > > +#ifdef COMPILE_PAYLOAD
> > > +#if defined(__i386__) || defined(__x86_64__)
> > > +
> > > +#include "kvm_x86_svm.h"
> > > +
> > > +#define AVIC_REG_ADDR 0x280
> > > +#define AVIC_TEST_VAL 0xec
> > > +#define AVIC_READ_FAIL 0x12ead
> > > +
> > > +#define AVIC_INFO_MASK ((1ULL << 32) | 0xff0)
> > > +#define AVIC_INFO_EXP ((1ULL << 32) | AVIC_REG_ADDR)

> > These five constants aren't used at all.
> > Maybe copy paste from kvm_svm01.c ?

> Yes, the AVIC constants are leftovers from copying kvm_svm01.c, sorry about
> that. Should I send a v2 after other patches get merged?

It'd be trivial to remove it, but if you send v2, please fix it there.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 5/7] KVM: Allow expected KVM_RUN errors in tst_kvm_run_instance()

[Petr Vorel]

Hi Martin,

> Add a new parameter to tst_kvm_run_instance() for expected errno value,
> e.g. EINTR for testing the possiblity to kill running guest. When
> ioctl(KVM_RUN) fails with the expected errno, tst_kvm_run_instance()
> will return -1 without terminating the test.

> Default tst_kvm_run() function expects no KVM_RUN errors.

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 7/7] Add KVM test for CPU lockup through malicous SVM guest

[Petr Vorel]

Hi Martin,

Acked-by: Petr Vorel <pvorel@suse.cz>

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 3/7] lib: Add safe functions for pthread_kill() and mutexes

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 4/7] KVM: Add async communication helper functions

[Cyril Hrubis]

Hi!
> +int tst_kvm_wait_guest(struct tst_kvm_instance *inst, int timeout)
> +{
> +	volatile struct tst_kvm_result *result = inst->result;
> +	int32_t res;
> +	time_t start = time(NULL);
> +
> +	while ((res = result->result) != KVM_TSYNC) {
> +		if (res == KVM_TEXIT)
> +			return res;
> +
> +		if (timeout >= 0 && start + timeout <= time(NULL))
> +			return -1;

Can we please avoid using wall clock time for timeouts? That is broken
by desing, since the time is continually adjusted by ntp daemon these
days and can jump back and forth...

This should be really:

	struct timespec start

	tst_clock_gettime(CLOCK_MONOTONIC, &start);

	while (...) {
		struct timespec now;

		tst_clock_gettime(CLOCK_MONOTONIC, &now);

		if (tst_timespec_diff_ms(now, start) >= timeout_ms)
			return -1;

> +		usleep(1000);
> +	}
> +
> +	return 0;
> +}

The rest looks good.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 4/7] KVM: Add async communication helper functions

[Martin Doucha]

On 23. 05. 23 14:13, Cyril Hrubis wrote:
> Hi!
>> +int tst_kvm_wait_guest(struct tst_kvm_instance *inst, int timeout)
>> +{
>> +	volatile struct tst_kvm_result *result = inst->result;
>> +	int32_t res;
>> +	time_t start = time(NULL);
>> +
>> +	while ((res = result->result) != KVM_TSYNC) {
>> +		if (res == KVM_TEXIT)
>> +			return res;
>> +
>> +		if (timeout >= 0 && start + timeout <= time(NULL))
>> +			return -1;
> 
> Can we please avoid using wall clock time for timeouts? That is broken
> by desing, since the time is continually adjusted by ntp daemon these
> days and can jump back and forth...

Yes, I'll change it to tst_clock_gettime() in v2. But I'll wait a bit 
for other patches to get merged before resubmitting.

-- 
Martin Doucha   mdoucha@suse.cz
QA Engineer for Software Maintenance
SUSE LINUX, s.r.o.
CORSO IIa
Krizikova 148/34
186 00 Prague 8
Czech Republic


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH 5/7] KVM: Allow expected KVM_RUN errors in tst_kvm_run_instance()

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp