The RISC-V SBI Steal-Time Accounting (STA) extension requires the shared memory physical address to be 64-byte aligned, or set to all-ones to explicitly disable steal-time accounting. KVM exposes the SBI STA shared memory configuration to userspace via KVM_SET_ONE_REG. However, the current implementation of kvm_sbi_ext_sta_set_reg() does not validate the alignment of the configured shared memory address. As a result, userspace can install a misaligned shared memory address that violates the SBI specification. Such an invalid configuration may later reach runtime code paths that assume a valid and properly aligned shared memory region. In particular, KVM_RUN can trigger the following WARN_ON in kvm_riscv_vcpu_record_steal_time(): WARNING: arch/riscv/kvm/vcpu_sbi_sta.c:49 at kvm_riscv_vcpu_record_steal_time WARN_ON paths are not expected to be reachable during normal runtime execution, and may result in a kernel panic when panic_on_warn is enabled. Fix this by validating the computed shared memory GPA at the KVM_SET_ONE_REG boundary. A temporary GPA is constructed and checked before committing it to vcpu->arch.sta.shmem. The validation allows either a 64-byte aligned GPA or INVALID_GPA (all-ones), which disables STA as defined by the SBI specification. This prevents invalid userspace state from reaching runtime code paths that assume SBI STA invariants and avoids unexpected WARN_ON behavior. Fixes: f61ce890b1f074 ("RISC-V: KVM: Add support for SBI STA registers") Signed-off-by: Jiakai Xu Signed-off-by: Jiakai Xu Reviewed-by: Andrew Jones --- V5 -> V6: Initialized new_shmem to INVALID_GPA as suggested. V4 -> V5: Added parentheses to function name in subject. V3 -> V4: Declared new_shmem at the top of kvm_sbi_ext_sta_set_reg(). Initialized new_shmem to 0 instead of vcpu->arch.sta.shmem. Added blank lines per review feedback. V2 -> V3: Added parentheses to function name in subject. V1 -> V2: Added Fixes tag. --- arch/riscv/kvm/vcpu_sbi_sta.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/arch/riscv/kvm/vcpu_sbi_sta.c b/arch/riscv/kvm/vcpu_sbi_sta.c index afa0545c3bcfc..3b834709b429f 100644 --- a/arch/riscv/kvm/vcpu_sbi_sta.c +++ b/arch/riscv/kvm/vcpu_sbi_sta.c @@ -181,6 +181,7 @@ static int kvm_sbi_ext_sta_set_reg(struct kvm_vcpu *vcpu, unsigned long reg_num, unsigned long reg_size, const void *reg_val) { unsigned long value; + gpa_t new_shmem = INVALID_GPA; if (reg_size != sizeof(unsigned long)) return -EINVAL; @@ -191,18 +192,18 @@ static int kvm_sbi_ext_sta_set_reg(struct kvm_vcpu *vcpu, unsigned long reg_num, if (IS_ENABLED(CONFIG_32BIT)) { gpa_t hi = upper_32_bits(vcpu->arch.sta.shmem); - vcpu->arch.sta.shmem = value; - vcpu->arch.sta.shmem |= hi << 32; + new_shmem = value; + new_shmem |= hi << 32; } else { - vcpu->arch.sta.shmem = value; + new_shmem = value; } break; case KVM_REG_RISCV_SBI_STA_REG(shmem_hi): if (IS_ENABLED(CONFIG_32BIT)) { gpa_t lo = lower_32_bits(vcpu->arch.sta.shmem); - vcpu->arch.sta.shmem = ((gpa_t)value << 32); - vcpu->arch.sta.shmem |= lo; + new_shmem = ((gpa_t)value << 32); + new_shmem |= lo; } else if (value != 0) { return -EINVAL; } @@ -211,6 +212,11 @@ static int kvm_sbi_ext_sta_set_reg(struct kvm_vcpu *vcpu, unsigned long reg_num, return -ENOENT; } + if (new_shmem != INVALID_GPA && !IS_ALIGNED(new_shmem, 64)) + return -EINVAL; + + vcpu->arch.sta.shmem = new_shmem; + return 0; } -- 2.34.1 Move steal time UAPI tests from steal_time_init() into a separate check_steal_time_uapi() function for better code organization and maintainability. Previously, x86 and ARM64 architectures performed UAPI validation tests within steal_time_init(), mixing initialization logic with uapi tests. Changes by architecture: x86_64: - Extract MSR reserved bits test from steal_time_init() - Move to check_steal_time_uapi() which tests that setting MSR_KVM_STEAL_TIME with KVM_STEAL_RESERVED_MASK fails ARM64: - Extract three UAPI tests from steal_time_init(): Device attribute support check Misaligned IPA rejection (EINVAL) Duplicate IPA setting rejection (EEXIST) - Move all tests to check_steal_time_uapi() RISC-V: - Add empty check_steal_time_uapi() stub for future use - No changes to steal_time_init() (had no tests to extract) The new check_steal_time_uapi() function: - Is called once before the per-VCPU test loop No functional change intended. Suggested-by: Andrew Jones Signed-off-by: Jiakai Xu Signed-off-by: Jiakai Xu --- V8 -> V9: Created a temporary VM with one vCPU in check_steal_time_uapi() instead of adding extra vCPUs to the main VM. Made check_steal_time_uapi() parameterless for all architectures. V7 -> V8: Used ST_GPA_BASE directly instead of st_gva[]/sync_global_to_guest() in x86_64 and ARM64 check_steal_time_uapi(). Created a temporary vcpu in ARM64 check_steal_time_uapi() to avoid EEXIST when steal_time_init() later sets IPA for vcpu[0]. Removed unnecessary comment in RISC-V check_steal_time_uapi(). --- tools/testing/selftests/kvm/steal_time.c | 67 ++++++++++++++++++------ 1 file changed, 51 insertions(+), 16 deletions(-) diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c index 8edc1fca345ba..75ad067f27260 100644 --- a/tools/testing/selftests/kvm/steal_time.c +++ b/tools/testing/selftests/kvm/steal_time.c @@ -69,16 +69,10 @@ static bool is_steal_time_supported(struct kvm_vcpu *vcpu) static void steal_time_init(struct kvm_vcpu *vcpu, uint32_t i) { - int ret; - /* ST_GPA_BASE is identity mapped */ st_gva[i] = (void *)(ST_GPA_BASE + i * STEAL_TIME_SIZE); sync_global_to_guest(vcpu->vm, st_gva[i]); - ret = _vcpu_set_msr(vcpu, MSR_KVM_STEAL_TIME, - (ulong)st_gva[i] | KVM_STEAL_RESERVED_MASK); - TEST_ASSERT(ret == 0, "Bad GPA didn't fail"); - vcpu_set_msr(vcpu, MSR_KVM_STEAL_TIME, (ulong)st_gva[i] | KVM_MSR_ENABLED); } @@ -99,6 +93,21 @@ static void steal_time_dump(struct kvm_vm *vm, uint32_t vcpu_idx) st->pad[8], st->pad[9], st->pad[10]); } +static void check_steal_time_uapi() +{ + struct kvm_vm *vm; + struct kvm_vcpu *vcpu; + int ret; + + vm = vm_create_with_one_vcpu(&vcpu, NULL); + + ret = _vcpu_set_msr(vcpu, MSR_KVM_STEAL_TIME, + (ulong)ST_GPA_BASE | KVM_STEAL_RESERVED_MASK); + TEST_ASSERT(ret == 0, "Bad GPA didn't fail"); + + kvm_vm_free(vm); +} + #elif defined(__aarch64__) /* PV_TIME_ST must have 64-byte alignment */ @@ -170,7 +179,6 @@ static void steal_time_init(struct kvm_vcpu *vcpu, uint32_t i) { struct kvm_vm *vm = vcpu->vm; uint64_t st_ipa; - int ret; struct kvm_device_attr dev = { .group = KVM_ARM_VCPU_PVTIME_CTRL, @@ -178,21 +186,12 @@ static void steal_time_init(struct kvm_vcpu *vcpu, uint32_t i) .addr = (uint64_t)&st_ipa, }; - vcpu_ioctl(vcpu, KVM_HAS_DEVICE_ATTR, &dev); - /* ST_GPA_BASE is identity mapped */ st_gva[i] = (void *)(ST_GPA_BASE + i * STEAL_TIME_SIZE); sync_global_to_guest(vm, st_gva[i]); - st_ipa = (ulong)st_gva[i] | 1; - ret = __vcpu_ioctl(vcpu, KVM_SET_DEVICE_ATTR, &dev); - TEST_ASSERT(ret == -1 && errno == EINVAL, "Bad IPA didn't report EINVAL"); - st_ipa = (ulong)st_gva[i]; vcpu_ioctl(vcpu, KVM_SET_DEVICE_ATTR, &dev); - - ret = __vcpu_ioctl(vcpu, KVM_SET_DEVICE_ATTR, &dev); - TEST_ASSERT(ret == -1 && errno == EEXIST, "Set IPA twice without EEXIST"); } static void steal_time_dump(struct kvm_vm *vm, uint32_t vcpu_idx) @@ -205,6 +204,36 @@ static void steal_time_dump(struct kvm_vm *vm, uint32_t vcpu_idx) ksft_print_msg(" st_time: %ld\n", st->st_time); } +static void check_steal_time_uapi() +{ + struct kvm_vm *vm; + struct kvm_vcpu *vcpu; + uint64_t st_ipa; + int ret; + + vm = vm_create_with_one_vcpu(&vcpu, NULL); + + struct kvm_device_attr dev = { + .group = KVM_ARM_VCPU_PVTIME_CTRL, + .attr = KVM_ARM_VCPU_PVTIME_IPA, + .addr = (uint64_t)&st_ipa, + }; + + vcpu_ioctl(vcpu, KVM_HAS_DEVICE_ATTR, &dev); + + st_ipa = (ulong)ST_GPA_BASE | 1; + ret = __vcpu_ioctl(vcpu, KVM_SET_DEVICE_ATTR, &dev); + TEST_ASSERT(ret == -1 && errno == EINVAL, "Bad IPA didn't report EINVAL"); + + st_ipa = (ulong)ST_GPA_BASE; + vcpu_ioctl(vcpu, KVM_SET_DEVICE_ATTR, &dev); + + ret = __vcpu_ioctl(vcpu, KVM_SET_DEVICE_ATTR, &dev); + TEST_ASSERT(ret == -1 && errno == EEXIST, "Set IPA twice without EEXIST"); + + kvm_vm_free(vm); +} + #elif defined(__riscv) /* SBI STA shmem must have 64-byte alignment */ @@ -301,6 +330,10 @@ static void steal_time_dump(struct kvm_vm *vm, uint32_t vcpu_idx) pr_info("\n"); } +static void check_steal_time_uapi() +{ +} + #endif static void *do_steal_time(void *arg) @@ -369,6 +402,8 @@ int main(int ac, char **av) TEST_REQUIRE(is_steal_time_supported(vcpus[0])); ksft_set_plan(NR_VCPUS); + check_steal_time_uapi(); + /* Run test on each VCPU */ for (i = 0; i < NR_VCPUS; ++i) { steal_time_init(vcpus[i], i); -- 2.34.1 Add RISC-V KVM selftests to verify the SBI Steal-Time Accounting (STA) shared memory alignment requirements. The SBI specification requires the STA shared memory GPA to be 64-byte aligned, or set to all-ones to explicitly disable steal-time accounting. This test verifies that KVM enforces the expected behavior when configuring the SBI STA shared memory via KVM_SET_ONE_REG. Specifically, the test checks that: - misaligned GPAs are rejected with -EINVAL - 64-byte aligned GPAs are accepted - all-ones GPA is accepted Signed-off-by: Jiakai Xu Signed-off-by: Jiakai Xu --- V8 -> V9: Dropped __riscv guard around INVALID_GPA, which is common to all architectures. V7 -> V8: Moved INVALID_GPA definition to kvm_util_types.h. Removed comments in RISC-V check_steal_time_uapi(). Corrected reg.id assignment for SBI STA. V6 -> V7: Removed RISCV_SBI_STA_REG() macro addition and used existing KVM_REG_RISCV_SBI_STA_REG(shmem_lo) instead. Refined assertion messages per review feedback. Split into two patches per Andrew Jones' suggestion: Refactored UAPI tests from steal_time_init() into dedicated check_steal_time_uapi() function and added empty stub for RISC-V. Filled in RISC-V stub with STA alignment tests. (this patch) --- .../selftests/kvm/include/kvm_util_types.h | 2 ++ tools/testing/selftests/kvm/steal_time.c | 31 +++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/tools/testing/selftests/kvm/include/kvm_util_types.h b/tools/testing/selftests/kvm/include/kvm_util_types.h index ec787b97cf184..0366e9bce7f93 100644 --- a/tools/testing/selftests/kvm/include/kvm_util_types.h +++ b/tools/testing/selftests/kvm/include/kvm_util_types.h @@ -17,4 +17,6 @@ typedef uint64_t vm_paddr_t; /* Virtual Machine (Guest) physical address */ typedef uint64_t vm_vaddr_t; /* Virtual Machine (Guest) virtual address */ +#define INVALID_GPA (~(uint64_t)0) + #endif /* SELFTEST_KVM_UTIL_TYPES_H */ diff --git a/tools/testing/selftests/kvm/steal_time.c b/tools/testing/selftests/kvm/steal_time.c index 75ad067f27260..0708b94ead895 100644 --- a/tools/testing/selftests/kvm/steal_time.c +++ b/tools/testing/selftests/kvm/steal_time.c @@ -332,6 +332,37 @@ static void steal_time_dump(struct kvm_vm *vm, uint32_t vcpu_idx) static void check_steal_time_uapi() { + struct kvm_vm *vm; + struct kvm_vcpu *vcpu; + struct kvm_one_reg reg; + uint64_t shmem; + int ret; + + vm = vm_create_with_one_vcpu(&vcpu, NULL); + + reg.id = KVM_REG_RISCV | + KVM_REG_SIZE_ULONG | + KVM_REG_RISCV_SBI_STATE | + KVM_REG_RISCV_SBI_STA | + KVM_REG_RISCV_SBI_STA_REG(shmem_lo); + reg.addr = (uint64_t)&shmem; + + shmem = ST_GPA_BASE + 1; + ret = __vcpu_ioctl(vcpu, KVM_SET_ONE_REG, ®); + TEST_ASSERT(ret == -1 && errno == EINVAL, + "misaligned STA shmem returns -EINVAL"); + + shmem = ST_GPA_BASE; + ret = __vcpu_ioctl(vcpu, KVM_SET_ONE_REG, ®); + TEST_ASSERT(ret == 0, + "aligned STA shmem succeeds"); + + shmem = INVALID_GPA; + ret = __vcpu_ioctl(vcpu, KVM_SET_ONE_REG, ®); + TEST_ASSERT(ret == 0, + "all-ones for STA shmem succeeds"); + + kvm_vm_free(vm); } #endif -- 2.34.1