From fcba483e82462830dd368951c0df03a95676f34d Mon Sep 17 00:00:00 2001 From: Sean Christopherson Date: Wed, 1 Jun 2022 11:01:58 -0700 Subject: [PATCH] KVM: selftests: Sanity check input to ioctls() at build time Add a static assert to the KVM/VM/vCPU ioctl() helpers to verify that the size of the argument provided matches the expected size of the IOCTL. Because ioctl() ultimately takes a "void *", it's all too easy to pass in garbage and not detect the error until runtime. E.g. while working on a CPUID rework, selftests happily compiled when vcpu_set_cpuid() unintentionally passed the cpuid() function as the parameter to ioctl() (a local "cpuid" parameter was removed, but its use was not replaced with "vcpu->cpuid" as intended). Tweak a variety of benign issues that aren't compatible with the sanity check, e.g. passing a non-pointer for ioctls(). Note, static_assert() requires a string on older versions of GCC. Feed it an empty string to make the compiler happy. Signed-off-by: Sean Christopherson Signed-off-by: Paolo Bonzini --- .../testing/selftests/kvm/include/kvm_util_base.h | 59 ++++++++++++++++------ .../testing/selftests/kvm/lib/aarch64/processor.c | 2 +- tools/testing/selftests/kvm/lib/guest_modes.c | 2 +- tools/testing/selftests/kvm/lib/kvm_util.c | 29 +---------- .../selftests/kvm/lib/x86_64/perf_test_util.c | 6 +-- tools/testing/selftests/kvm/s390x/resets.c | 6 +-- .../selftests/kvm/x86_64/mmio_warning_test.c | 2 +- .../selftests/kvm/x86_64/pmu_event_filter_test.c | 2 +- .../testing/selftests/kvm/x86_64/xen_shinfo_test.c | 6 +-- 9 files changed, 58 insertions(+), 56 deletions(-) diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h index 04ddab322b6b..cdaea2383543 100644 --- a/tools/testing/selftests/kvm/include/kvm_util_base.h +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h @@ -180,29 +180,56 @@ static inline bool kvm_has_cap(long cap) #define __KVM_IOCTL_ERROR(_name, _ret) __KVM_SYSCALL_ERROR(_name, _ret) #define KVM_IOCTL_ERROR(_ioctl, _ret) __KVM_IOCTL_ERROR(#_ioctl, _ret) -#define __kvm_ioctl(kvm_fd, cmd, arg) \ - ioctl(kvm_fd, cmd, arg) +#define kvm_do_ioctl(fd, cmd, arg) \ +({ \ + static_assert(!_IOC_SIZE(cmd) || sizeof(*arg) == _IOC_SIZE(cmd), ""); \ + ioctl(fd, cmd, arg); \ +}) -static inline void _kvm_ioctl(int kvm_fd, unsigned long cmd, const char *name, - void *arg) -{ - int ret = __kvm_ioctl(kvm_fd, cmd, arg); +#define __kvm_ioctl(kvm_fd, cmd, arg) \ + kvm_do_ioctl(kvm_fd, cmd, arg) - TEST_ASSERT(!ret, __KVM_IOCTL_ERROR(name, ret)); -} + +#define _kvm_ioctl(kvm_fd, cmd, name, arg) \ +({ \ + int ret = __kvm_ioctl(kvm_fd, cmd, arg); \ + \ + TEST_ASSERT(!ret, __KVM_IOCTL_ERROR(name, ret)); \ +}) #define kvm_ioctl(kvm_fd, cmd, arg) \ _kvm_ioctl(kvm_fd, cmd, #cmd, arg) -int __vm_ioctl(struct kvm_vm *vm, unsigned long cmd, void *arg); -void _vm_ioctl(struct kvm_vm *vm, unsigned long cmd, const char *name, void *arg); -#define vm_ioctl(vm, cmd, arg) _vm_ioctl(vm, cmd, #cmd, arg) +#define __vm_ioctl(vm, cmd, arg) \ +({ \ + static_assert(sizeof(*(vm)) == sizeof(struct kvm_vm), ""); \ + kvm_do_ioctl((vm)->fd, cmd, arg); \ +}) + +#define _vm_ioctl(vm, cmd, name, arg) \ +({ \ + int ret = __vm_ioctl(vm, cmd, arg); \ + \ + TEST_ASSERT(!ret, __KVM_IOCTL_ERROR(name, ret)); \ +}) + +#define vm_ioctl(vm, cmd, arg) \ + _vm_ioctl(vm, cmd, #cmd, arg) + +#define __vcpu_ioctl(vcpu, cmd, arg) \ +({ \ + static_assert(sizeof(*(vcpu)) == sizeof(struct kvm_vcpu), ""); \ + kvm_do_ioctl((vcpu)->fd, cmd, arg); \ +}) + +#define _vcpu_ioctl(vcpu, cmd, name, arg) \ +({ \ + int ret = __vcpu_ioctl(vcpu, cmd, arg); \ + \ + TEST_ASSERT(!ret, __KVM_IOCTL_ERROR(name, ret)); \ +}) -int __vcpu_ioctl(struct kvm_vcpu *vcpu, unsigned long cmd, - void *arg); -void _vcpu_ioctl(struct kvm_vcpu *vcpu, unsigned long cmd, - const char *name, void *arg); -#define vcpu_ioctl(vcpu, cmd, arg) \ +#define vcpu_ioctl(vcpu, cmd, arg) \ _vcpu_ioctl(vcpu, cmd, #cmd, arg) /* diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c index 6bd27782f00c..6f5551368944 100644 --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c @@ -472,7 +472,7 @@ void aarch64_get_supported_page_sizes(uint32_t ipa, }; kvm_fd = open_kvm_dev_path_or_exit(); - vm_fd = __kvm_ioctl(kvm_fd, KVM_CREATE_VM, ipa); + vm_fd = __kvm_ioctl(kvm_fd, KVM_CREATE_VM, (void *)(unsigned long)ipa); TEST_ASSERT(vm_fd >= 0, KVM_IOCTL_ERROR(KVM_CREATE_VM, vm_fd)); vcpu_fd = ioctl(vm_fd, KVM_CREATE_VCPU, 0); diff --git a/tools/testing/selftests/kvm/lib/guest_modes.c b/tools/testing/selftests/kvm/lib/guest_modes.c index 0be56c63aed6..99a575bbbc52 100644 --- a/tools/testing/selftests/kvm/lib/guest_modes.c +++ b/tools/testing/selftests/kvm/lib/guest_modes.c @@ -65,7 +65,7 @@ void guest_modes_append_default(void) struct kvm_s390_vm_cpu_processor info; kvm_fd = open_kvm_dev_path_or_exit(); - vm_fd = __kvm_ioctl(kvm_fd, KVM_CREATE_VM, 0); + vm_fd = __kvm_ioctl(kvm_fd, KVM_CREATE_VM, NULL); kvm_device_attr_get(vm_fd, KVM_S390_VM_CPU_MODEL, KVM_S390_VM_CPU_PROCESSOR, &info); close(vm_fd); diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c index cca89d9a83ea..39f2f5f1338f 100644 --- a/tools/testing/selftests/kvm/lib/kvm_util.c +++ b/tools/testing/selftests/kvm/lib/kvm_util.c @@ -72,7 +72,7 @@ unsigned int kvm_check_cap(long cap) int kvm_fd; kvm_fd = open_kvm_dev_path_or_exit(); - ret = __kvm_ioctl(kvm_fd, KVM_CHECK_EXTENSION, cap); + ret = __kvm_ioctl(kvm_fd, KVM_CHECK_EXTENSION, (void *)cap); TEST_ASSERT(ret >= 0, KVM_IOCTL_ERROR(KVM_CHECK_EXTENSION, ret)); close(kvm_fd); @@ -92,7 +92,7 @@ static void vm_open(struct kvm_vm *vm) TEST_REQUIRE(kvm_has_cap(KVM_CAP_IMMEDIATE_EXIT)); - vm->fd = __kvm_ioctl(vm->kvm_fd, KVM_CREATE_VM, vm->type); + vm->fd = __kvm_ioctl(vm->kvm_fd, KVM_CREATE_VM, (void *)vm->type); TEST_ASSERT(vm->fd >= 0, KVM_IOCTL_ERROR(KVM_CREATE_VM, vm->fd)); } @@ -1450,19 +1450,6 @@ struct kvm_reg_list *vcpu_get_reg_list(struct kvm_vcpu *vcpu) return reg_list; } -int __vcpu_ioctl(struct kvm_vcpu *vcpu, unsigned long cmd, void *arg) -{ - return ioctl(vcpu->fd, cmd, arg); -} - -void _vcpu_ioctl(struct kvm_vcpu *vcpu, unsigned long cmd, const char *name, - void *arg) -{ - int ret = __vcpu_ioctl(vcpu, cmd, arg); - - TEST_ASSERT(!ret, __KVM_IOCTL_ERROR(name, ret)); -} - void *vcpu_map_dirty_ring(struct kvm_vcpu *vcpu) { uint32_t page_size = vcpu->vm->page_size; @@ -1492,18 +1479,6 @@ void *vcpu_map_dirty_ring(struct kvm_vcpu *vcpu) return vcpu->dirty_gfns; } -int __vm_ioctl(struct kvm_vm *vm, unsigned long cmd, void *arg) -{ - return ioctl(vm->fd, cmd, arg); -} - -void _vm_ioctl(struct kvm_vm *vm, unsigned long cmd, const char *name, void *arg) -{ - int ret = __vm_ioctl(vm, cmd, arg); - - TEST_ASSERT(!ret, __KVM_IOCTL_ERROR(name, ret)); -} - /* * Device Ioctl */ diff --git a/tools/testing/selftests/kvm/lib/x86_64/perf_test_util.c b/tools/testing/selftests/kvm/lib/x86_64/perf_test_util.c index 446820a549ba..bfe85c8c2f6e 100644 --- a/tools/testing/selftests/kvm/lib/x86_64/perf_test_util.c +++ b/tools/testing/selftests/kvm/lib/x86_64/perf_test_util.c @@ -103,9 +103,9 @@ void perf_test_setup_nested(struct kvm_vm *vm, int nr_vcpus, struct kvm_vcpu *vc * Override the vCPU to run perf_test_l1_guest_code() which will * bounce it into L2 before calling perf_test_guest_code(). */ - vcpu_regs_get(vm, vcpus[vcpu_id]->id, ®s); + vcpu_regs_get(vcpus[vcpu_id], ®s); regs.rip = (unsigned long) perf_test_l1_guest_code; - vcpu_regs_set(vm, vcpus[vcpu_id]->id, ®s); - vcpu_args_set(vm, vcpus[vcpu_id]->id, 2, vmx_gva, vcpu_id); + vcpu_regs_set(vcpus[vcpu_id], ®s); + vcpu_args_set(vcpus[vcpu_id], 2, vmx_gva, vcpu_id); } } diff --git a/tools/testing/selftests/kvm/s390x/resets.c b/tools/testing/selftests/kvm/s390x/resets.c index e1737411accc..19486084eb30 100644 --- a/tools/testing/selftests/kvm/s390x/resets.c +++ b/tools/testing/selftests/kvm/s390x/resets.c @@ -225,7 +225,7 @@ static void test_normal(void) inject_irq(vcpu); - vcpu_ioctl(vcpu, KVM_S390_NORMAL_RESET, 0); + vcpu_ioctl(vcpu, KVM_S390_NORMAL_RESET, NULL); /* must clears */ assert_normal(vcpu); @@ -248,7 +248,7 @@ static void test_initial(void) inject_irq(vcpu); - vcpu_ioctl(vcpu, KVM_S390_INITIAL_RESET, 0); + vcpu_ioctl(vcpu, KVM_S390_INITIAL_RESET, NULL); /* must clears */ assert_normal(vcpu); @@ -271,7 +271,7 @@ static void test_clear(void) inject_irq(vcpu); - vcpu_ioctl(vcpu, KVM_S390_CLEAR_RESET, 0); + vcpu_ioctl(vcpu, KVM_S390_CLEAR_RESET, NULL); /* must clears */ assert_normal(vcpu); diff --git a/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c index 0e4590afd0e1..fb02581953a3 100644 --- a/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c +++ b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c @@ -59,7 +59,7 @@ void test(void) kvm = open("/dev/kvm", O_RDWR); TEST_ASSERT(kvm != -1, "failed to open /dev/kvm"); - kvmvm = __kvm_ioctl(kvm, KVM_CREATE_VM, 0); + kvmvm = __kvm_ioctl(kvm, KVM_CREATE_VM, NULL); TEST_ASSERT(kvmvm > 0, KVM_IOCTL_ERROR(KVM_CREATE_VM, kvmvm)); kvmcpu = ioctl(kvmvm, KVM_CREATE_VCPU, 0); TEST_ASSERT(kvmcpu != -1, KVM_IOCTL_ERROR(KVM_CREATE_VCPU, kvmcpu)); diff --git a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c index 656fb2227a81..786b3a794f84 100644 --- a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c +++ b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c @@ -276,7 +276,7 @@ static void test_without_filter(struct kvm_vcpu *vcpu) static uint64_t test_with_filter(struct kvm_vcpu *vcpu, struct kvm_pmu_event_filter *f) { - vm_ioctl(vcpu->vm, KVM_SET_PMU_EVENT_FILTER, (void *)f); + vm_ioctl(vcpu->vm, KVM_SET_PMU_EVENT_FILTER, f); return run_vcpu_to_sync(vcpu); } diff --git a/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c b/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c index bdcb28186ccc..a4a78637c35a 100644 --- a/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c +++ b/tools/testing/selftests/kvm/x86_64/xen_shinfo_test.c @@ -472,7 +472,7 @@ int main(int argc, char *argv[]) irq_routes.entries[1].u.xen_evtchn.vcpu = vcpu->id; irq_routes.entries[1].u.xen_evtchn.priority = KVM_IRQ_ROUTING_XEN_EVTCHN_PRIO_2LEVEL; - vm_ioctl(vm, KVM_SET_GSI_ROUTING, &irq_routes); + vm_ioctl(vm, KVM_SET_GSI_ROUTING, &irq_routes.info); struct kvm_irqfd ifd = { }; @@ -716,7 +716,7 @@ int main(int argc, char *argv[]) if (verbose) printf("Testing restored oneshot timer\n"); - tmr.u.timer.expires_ns = rs->state_entry_time + 100000000, + tmr.u.timer.expires_ns = rs->state_entry_time + 100000000; vcpu_ioctl(vcpu, KVM_XEN_VCPU_SET_ATTR, &tmr); evtchn_irq_expected = true; alarm(1); @@ -743,7 +743,7 @@ int main(int argc, char *argv[]) if (verbose) printf("Testing SCHEDOP_poll wake on masked event\n"); - tmr.u.timer.expires_ns = rs->state_entry_time + 100000000, + tmr.u.timer.expires_ns = rs->state_entry_time + 100000000; vcpu_ioctl(vcpu, KVM_XEN_VCPU_SET_ATTR, &tmr); alarm(1); break; -- 2.11.0