fix

Author: 6eanut

executor/common_kvm_amd64.h

@@ -1126,7 +1126,7 @@ static void install_syzos_code(void* host_mem, size_t mem_size)
 {
 	size_t size = (char*)&__stop_guest - (char*)&__start_guest;
 	if (size > mem_size)
-		fail("SyzOS size exceeds guest memory");
+		fail("SYZOS size exceeds guest memory");
 	memcpy(host_mem, &__start_guest, size);
 }
 

executor/common_kvm_amd64_syzos.h

@@ -6,11 +6,12 @@
 
 // This file provides guest code running inside the AMD64 KVM.
 
-#include "common_kvm_syzos.h"
-#include "kvm.h"
 #include <linux/kvm.h>
 #include <stdbool.h>
 
+#include "common_kvm_syzos.h"
+#include "kvm.h"
+
 // There are no particular rules to assign numbers here, but changing them will
 // result in losing some existing reproducers. Therefore, we try to leave spaces
 // between unrelated IDs.
@@ -43,16 +44,6 @@ typedef enum {
 	SYZOS_API_STOP, // Must be the last one
 } syzos_api_id;
 
-struct api_call_header {
-	uint64 call;
-	uint64 size;
-};
-
-struct api_call_uexit {
-	struct api_call_header header;
-	uint64 exit_code;
-};
-
 struct api_call_code {
 	struct api_call_header header;
 	uint8 insns[];
@@ -70,26 +61,6 @@ struct api_call_cpuid {
 	uint32 ecx;
 };
 
-struct api_call_1 {
-	struct api_call_header header;
-	uint64 arg;
-};
-
-struct api_call_2 {
-	struct api_call_header header;
-	uint64 args[2];
-};
-
-struct api_call_3 {
-	struct api_call_header header;
-	uint64 args[3];
-};
-
-struct api_call_5 {
-	struct api_call_header header;
-	uint64 args[5];
-};
-
 // This struct must match the push/pop order in nested_vm_exit_handler_intel_asm().
 struct l2_guest_regs {
 	uint64 rax, rbx, rcx, rdx, rsi, rdi, rbp;
@@ -165,8 +136,8 @@ __attribute__((naked)) GUEST_CODE static void uexit_irq_handler()
 // TODO(glider): executor/style_test.go insists that single-line compound statements should not
 // be used e.g. in the following case:
 //   if (call == SYZOS_API_UEXIT) {
-//     struct api_call_uexit* ucmd = (struct api_call_uexit*)cmd;
-//     guest_uexit(ucmd->exit_code);
+//     struct api_call_1* ccmd = (struct api_call_1*)cmd;
+//     guest_uexit(ucmd->arg);
 //   } else if (call == SYZOS_API_WR_CRN) {
 //     guest_handle_wr_crn((struct api_call_2*)cmd);  // Style check fails here
 //   }
@@ -188,8 +159,8 @@ guest_main(uint64 size, uint64 cpu)
 		volatile uint64 call = cmd->call;
 		if (call == SYZOS_API_UEXIT) {
 			// Issue a user exit.
-			struct api_call_uexit* ucmd = (struct api_call_uexit*)cmd;
-			guest_uexit(ucmd->exit_code);
+			struct api_call_1* ccmd = (struct api_call_1*)cmd;
+			guest_uexit(ccmd->arg);
 		} else if (call == SYZOS_API_CODE) {
 			// Execute an instruction blob.
 			struct api_call_code* ccmd = (struct api_call_code*)cmd;

executor/common_kvm_arm64.h

@@ -70,8 +70,8 @@ static void vm_set_user_memory_region(int vmfd, uint32 slot, uint32 flags, uint6
 #define ADRP_OPCODE 0x90000000
 #define ADRP_OPCODE_MASK 0x9f000000
 
-// Code loading SyzOS into guest memory does not handle data relocations (see
-// https://github.com/google/syzkaller/issues/5565), so SyzOS will crash soon after encountering an
+// Code loading SYZOS into guest memory does not handle data relocations (see
+// https://github.com/google/syzkaller/issues/5565), so SYZOS will crash soon after encountering an
 // ADRP instruction. Detect these instructions to catch regressions early.
 // The most common reason for using data relocaions is accessing global variables and constants.
 // Sometimes the compiler may choose to emit a read-only constant to zero-initialize a structure
@@ -81,15 +81,15 @@ static void validate_guest_code(void* mem, size_t size)
 	uint32* insns = (uint32*)mem;
 	for (size_t i = 0; i < size / 4; i++) {
 		if ((insns[i] & ADRP_OPCODE_MASK) == ADRP_OPCODE)
-			fail("ADRP instruction detected in SyzOS, exiting");
+			fail("ADRP instruction detected in SYZOS, exiting");
 	}
 }
 
 static void install_syzos_code(void* host_mem, size_t mem_size)
 {
 	size_t size = (char*)&__stop_guest - (char*)&__start_guest;
 	if (size > mem_size)
-		fail("SyzOS size exceeds guest memory");
+		fail("SYZOS size exceeds guest memory");
 	memcpy(host_mem, &__start_guest, size);
 	validate_guest_code(host_mem, size);
 }

executor/common_kvm_arm64_syzos.h

@@ -6,11 +6,12 @@
 
 // This file provides guest code running inside the ARM64 KVM.
 
-#include "common_kvm_syzos.h"
-#include "kvm.h"
 #include <linux/kvm.h>
 #include <stdbool.h>
 
+#include "common_kvm_syzos.h"
+#include "kvm.h"
+
 // Compilers will eagerly try to transform the switch statement in guest_main()
 // into a jump table, unless the cases are sparse enough.
 // We use prime numbers multiplied by 10 to prevent this behavior.
@@ -31,31 +32,6 @@ typedef enum {
 	SYZOS_API_STOP, // Must be the last one
 } syzos_api_id;
 
-struct api_call_header {
-	uint64 call;
-	uint64 size;
-};
-
-struct api_call_uexit {
-	struct api_call_header header;
-	uint64 exit_code;
-};
-
-struct api_call_1 {
-	struct api_call_header header;
-	uint64 arg;
-};
-
-struct api_call_2 {
-	struct api_call_header header;
-	uint64 args[2];
-};
-
-struct api_call_3 {
-	struct api_call_header header;
-	uint64 args[3];
-};
-
 struct api_call_code {
 	struct api_call_header header;
 	uint32 insns[];
@@ -127,8 +103,8 @@ guest_main(uint64 size, uint64 cpu)
 			return;
 		switch (cmd->call) {
 		case SYZOS_API_UEXIT: {
-			struct api_call_uexit* ucmd = (struct api_call_uexit*)cmd;
-			guest_uexit(ucmd->exit_code);
+			struct api_call_1* ccmd = (struct api_call_1*)cmd;
+			guest_uexit(ccmd->arg);
 			break;
 		}
 		case SYZOS_API_CODE: {
@@ -1219,7 +1195,7 @@ GUEST_CODE static void its_send_sync_cmd(uint64 cmdq_base, uint32 vcpu_id)
 }
 
 // This function is carefully written in a way that prevents jump table emission.
-// SyzOS cannot reference global constants, and compilers are very eager to generate a jump table
+// SYZOS cannot reference global constants, and compilers are very eager to generate a jump table
 // for a switch over GITS commands.
 // To work around that, we replace the switch statement with a series of if statements.
 // In addition, cmd->type is stored in a volatile variable, so that it is read on each if statement,

executor/common_kvm_riscv64.h

@@ -8,12 +8,13 @@
 
 // Implementation of syz_kvm_setup_cpu pseudo-syscall.
 
-#include "common_kvm.h"
-#include "kvm.h"
 #include <stdint.h>
 #include <string.h>
 #include <sys/ioctl.h>
 
+#include "common_kvm.h"
+#include "kvm.h"
+
 #if SYZ_EXECUTOR || __NR_syz_kvm_setup_syzos_vm || __NR_syz_kvm_add_vcpu
 #include "common_kvm_riscv64_syzos.h"
 #endif
@@ -101,6 +102,10 @@ struct kvm_text {
 #endif
 
 #if SYZ_EXECUTOR || __NR_syz_kvm_setup_cpu || __NR_syz_kvm_setup_syzos_vm || __NR_syz_kvm_add_vcpu
+// The exception vector table setup and SBI invocation here follow the
+// implementation in Linux kselftest KVM RISC-V tests.
+// See https://elixir.bootlin.com/linux/v6.19-rc5/source/tools/testing/selftests/kvm/lib/riscv/processor.c#L337 .
+
 #define KVM_RISCV_SELFTESTS_SBI_EXT 0x08FFFFFF
 #define KVM_RISCV_SELFTESTS_SBI_UNEXP 1
 
@@ -116,14 +121,14 @@ struct sbiret sbi_ecall(unsigned long arg0, unsigned long arg1,
 {
 	struct sbiret ret;
 
-	register uintptr_t a0 asm("a0") = (uintptr_t)(arg0);
-	register uintptr_t a1 asm("a1") = (uintptr_t)(arg1);
-	register uintptr_t a2 asm("a2") = (uintptr_t)(arg2);
-	register uintptr_t a3 asm("a3") = (uintptr_t)(arg3);
-	register uintptr_t a4 asm("a4") = (uintptr_t)(arg4);
-	register uintptr_t a5 asm("a5") = (uintptr_t)(arg5);
-	register uintptr_t a6 asm("a6") = (uintptr_t)(fid);
-	register uintptr_t a7 asm("a7") = (uintptr_t)(ext);
+	register unsigned long a0 asm("a0") = arg0;
+	register unsigned long a1 asm("a1") = arg1;
+	register unsigned long a2 asm("a2") = arg2;
+	register unsigned long a3 asm("a3") = arg3;
+	register unsigned long a4 asm("a4") = arg4;
+	register unsigned long a5 asm("a5") = arg5;
+	register unsigned long a6 asm("a6") = fid;
+	register unsigned long a7 asm("a7") = ext;
 	asm volatile("ecall"
 		     : "+r"(a0), "+r"(a1)
 		     : "r"(a2), "r"(a3), "r"(a4), "r"(a5), "r"(a6), "r"(a7)
@@ -143,8 +148,6 @@ __attribute__((used)) __attribute((__aligned__(16))) static void guest_unexp_tra
 #endif
 
 #if SYZ_EXECUTOR || __NR_syz_kvm_setup_cpu
-// Define the starting physical address for the guest code.
-#define CODE_START 0x80000000ULL
 
 // syz_kvm_setup_cpu$riscv64(fd fd_kvmvm, cpufd fd_kvmcpu, usermem vma[24], text ptr[in, array[kvm_text_riscv64, 1]], ntext len[text], flags const[0], opts ptr[in, array[kvm_setup_opt_riscv64, 1]], nopt len[opts])
 static volatile long syz_kvm_setup_cpu(volatile long a0, volatile long a1, volatile long a2, volatile long a3, volatile long a4, volatile long a5, volatile long a6, volatile long a7)
@@ -163,7 +166,7 @@ static volatile long syz_kvm_setup_cpu(volatile long a0, volatile long a1, volat
 		struct kvm_userspace_memory_region mem = {
 		    .slot = (unsigned int)i,
 		    .flags = 0,
-		    .guest_phys_addr = CODE_START + i * page_size,
+		    .guest_phys_addr = RISCV64_ADDR_USER_CODE + i * page_size,
 		    .memory_size = page_size,
 		    .userspace_addr =
 			(uintptr_t)(host_mem + i * page_size),
@@ -185,10 +188,10 @@ static volatile long syz_kvm_setup_cpu(volatile long a0, volatile long a1, volat
 
 	// Initialize VCPU registers.
 	// Set PC (program counter) to start of code.
-	if (kvm_set_reg(cpufd, RISCV_CORE_REG(CORE_PC), CODE_START))
+	if (kvm_set_reg(cpufd, RISCV_CORE_REG(CORE_PC), RISCV64_ADDR_USER_CODE))
 		return -1;
 	// Set SP (stack pointer) at end of memory, reserving space for stack.
-	unsigned long stack_top = CODE_START + guest_mem_size - page_size;
+	unsigned long stack_top = RISCV64_ADDR_USER_CODE + guest_mem_size - page_size;
 	if (kvm_set_reg(cpufd, RISCV_CORE_REG(CORE_SP), stack_top))
 		return -1;
 	// Set privilege mode to S-mode.
@@ -199,7 +202,7 @@ static volatile long syz_kvm_setup_cpu(volatile long a0, volatile long a1, volat
 	if (kvm_set_reg(cpufd, RISCV_CSR_REG(CSR_SSTATUS), sstatus))
 		return -1;
 	// Set STVEC.
-	unsigned long stvec = CODE_START + page_size;
+	unsigned long stvec = RISCV64_ADDR_USER_CODE + page_size;
 	if (kvm_set_reg(cpufd, RISCV_CSR_REG(CSR_STVEC), stvec))
 		return -1;
 	// Set GP.
@@ -276,8 +279,8 @@ static void vm_set_user_memory_region(int vmfd, uint32 slot, uint32 flags, uint6
 #define AUIPC_OPCODE 0x17
 #define AUIPC_OPCODE_MASK 0x7f
 
-// Code loading SyzOS into guest memory does not handle data relocations (see
-// https://github.com/google/syzkaller/issues/5565), so SyzOS will crash soon after encountering an
+// Code loading SYZOS into guest memory does not handle data relocations (see
+// https://github.com/google/syzkaller/issues/5565), so SYZOS will crash soon after encountering an
 // AUIPC instruction. Detect these instructions to catch regressions early.
 // The most common reason for using data relocaions is accessing global variables and constants.
 // Sometimes the compiler may choose to emit a read-only constant to zero-initialize a structure
@@ -287,15 +290,15 @@ static void validate_guest_code(void* mem, size_t size)
 	uint32* insns = (uint32*)mem;
 	for (size_t i = 0; i < size / 4; i++) {
 		if ((insns[i] & AUIPC_OPCODE_MASK) == AUIPC_OPCODE)
-			fail("AUIPC instruction detected in SyzOS, exiting");
+			fail("AUIPC instruction detected in SYZOS, exiting");
 	}
 }
 
 static void install_syzos_code(void* host_mem, size_t mem_size)
 {
 	size_t size = (char*)&__stop_guest - (char*)&__start_guest;
 	if (size > mem_size)
-		fail("SyzOS size exceeds guest memory");
+		fail("SYZOS size exceeds guest memory");
 	memcpy(host_mem, &__start_guest, size);
 	validate_guest_code(host_mem, size);
 }