| Message ID | 20240614202859.3597745-6-minipli@grsecurity.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: Reject vCPU IDs above 2^32 | expand |
On Fri, Jun 14, 2024, Mathias Krause wrote: > The KVM_SET_BOOT_CPU_ID ioctl missed to reject invalid vCPU IDs. Verify > this no longer works and gets rejected with an appropriate error code. > > Signed-off-by: Mathias Krause <minipli@grsecurity.net> > --- > tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c b/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c > index d691d86e5bc3..50a0c3f61baf 100644 > --- a/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c > +++ b/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c > @@ -33,6 +33,13 @@ static void guest_not_bsp_vcpu(void *arg) > GUEST_DONE(); > } > > +static void test_set_invalid_bsp(struct kvm_vm *vm) > +{ > + int r = __vm_ioctl(vm, KVM_SET_BOOT_CPU_ID, (void *)(1L << 32)); > + I also added a test to verify KVM_CAP_MAX_VCPU_ID+1 also fails, because why not. unsigned long max_vcpu_id = vm_check_cap(vm, KVM_CAP_MAX_VCPU_ID); int r; if (max_vcpu_id) { r = __vm_ioctl(vm, KVM_SET_BOOT_CPU_ID, (void *)(max_vcpu_id + 1)); TEST_ASSERT(r == -1 && errno == EINVAL, "BSP with ID > MAX should fail"); } r = __vm_ioctl(vm, KVM_SET_BOOT_CPU_ID, (void *)(1L << 32)); TEST_ASSERT(r == -1 && errno == EINVAL, "BSP with ID[63:32]!=0 should fail"); > + TEST_ASSERT(r == -1 && errno == EINVAL, "invalid KVM_SET_BOOT_CPU_ID set"); > +} > + > static void test_set_bsp_busy(struct kvm_vcpu *vcpu, const char *msg) > { > int r = __vm_ioctl(vcpu->vm, KVM_SET_BOOT_CPU_ID, > @@ -75,11 +82,15 @@ static void run_vcpu(struct kvm_vcpu *vcpu) > static struct kvm_vm *create_vm(uint32_t nr_vcpus, uint32_t bsp_vcpu_id, > struct kvm_vcpu *vcpus[]) > { > + static int invalid_bsp_tested; > struct kvm_vm *vm; > uint32_t i; > > vm = vm_create(nr_vcpus); > > + if (!invalid_bsp_tested++) I dropped this and just had every VM run the negative test. There's zero chance anyone will ever notice an extra failed ioctl() or three, whereas it took me a second to realize this is just a somewhat lazy way of writing a one-off negative test. > + test_set_invalid_bsp(vm); > + > vm_ioctl(vm, KVM_SET_BOOT_CPU_ID, (void *)(unsigned long)bsp_vcpu_id); > > for (i = 0; i < nr_vcpus; i++) > -- > 2.30.2 >
diff --git a/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c b/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c index d691d86e5bc3..50a0c3f61baf 100644 --- a/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c +++ b/tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c @@ -33,6 +33,13 @@ static void guest_not_bsp_vcpu(void *arg) GUEST_DONE(); } +static void test_set_invalid_bsp(struct kvm_vm *vm) +{ + int r = __vm_ioctl(vm, KVM_SET_BOOT_CPU_ID, (void *)(1L << 32)); + + TEST_ASSERT(r == -1 && errno == EINVAL, "invalid KVM_SET_BOOT_CPU_ID set"); +} + static void test_set_bsp_busy(struct kvm_vcpu *vcpu, const char *msg) { int r = __vm_ioctl(vcpu->vm, KVM_SET_BOOT_CPU_ID, @@ -75,11 +82,15 @@ static void run_vcpu(struct kvm_vcpu *vcpu) static struct kvm_vm *create_vm(uint32_t nr_vcpus, uint32_t bsp_vcpu_id, struct kvm_vcpu *vcpus[]) { + static int invalid_bsp_tested; struct kvm_vm *vm; uint32_t i; vm = vm_create(nr_vcpus); + if (!invalid_bsp_tested++) + test_set_invalid_bsp(vm); + vm_ioctl(vm, KVM_SET_BOOT_CPU_ID, (void *)(unsigned long)bsp_vcpu_id); for (i = 0; i < nr_vcpus; i++)
The KVM_SET_BOOT_CPU_ID ioctl missed to reject invalid vCPU IDs. Verify this no longer works and gets rejected with an appropriate error code. Signed-off-by: Mathias Krause <minipli@grsecurity.net> --- tools/testing/selftests/kvm/x86_64/set_boot_cpu_id.c | 11 +++++++++++ 1 file changed, 11 insertions(+)