Re: [PATCH v3 5/5] KVM: selftests: Test vCPU boot IDs above 2^32

[Sean Christopherson <seanjc@google.com>]

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
>