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 <seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
index 04ddab3..cdaea23 100644 (file)
--- 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 6bd2778..6f55513 100644 (file)
--- 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 0be56c6..99a575b 100644 (file)
--- 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 cca89d9..39f2f5f 100644 (file)
--- 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 446820a..bfe85c8 100644 (file)
--- 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, &regs);
+               vcpu_regs_get(vcpus[vcpu_id], &regs);
                regs.rip = (unsigned long) perf_test_l1_guest_code;
-               vcpu_regs_set(vm, vcpus[vcpu_id]->id, &regs);
-               vcpu_args_set(vm, vcpus[vcpu_id]->id, 2, vmx_gva, vcpu_id);
+               vcpu_regs_set(vcpus[vcpu_id], &regs);
+               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 e173741..1948608 100644 (file)
--- 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 0e4590a..fb02581 100644 (file)
--- 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 656fb22..786b3a7 100644 (file)
--- 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 bdcb281..a4a7863 100644 (file)
--- 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;