diff mbox series

[v3,4/5] KVM: selftests: Test max vCPU IDs corner cases

Message ID 20240614202859.3597745-5-minipli@grsecurity.net (mailing list archive)
State New, archived
Headers show
Series KVM: Reject vCPU IDs above 2^32 | expand

Commit Message

Mathias Krause June 14, 2024, 8:28 p.m. UTC 
The KVM_CREATE_VCPU ioctl ABI had an implicit integer truncation bug,
allowing 2^32 aliases for a vCPU ID by setting the upper 32 bits of a 64
bit ioctl() argument.

It also allowed excluding a once set boot CPU ID.

Verify this no longer works and gets rejected with an error.

Signed-off-by: Mathias Krause <minipli@grsecurity.net>
---
v3:
- test BOOT_CPU_ID interaction too

 .../kvm/x86_64/max_vcpuid_cap_test.c          | 22 +++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

Comments

Sean Christopherson June 18, 2024, 9:46 p.m. UTC | #1 
On Fri, Jun 14, 2024, Mathias Krause wrote:
> The KVM_CREATE_VCPU ioctl ABI had an implicit integer truncation bug,
> allowing 2^32 aliases for a vCPU ID by setting the upper 32 bits of a 64
> bit ioctl() argument.
> 
> It also allowed excluding a once set boot CPU ID.
> 
> Verify this no longer works and gets rejected with an error.
> 
> Signed-off-by: Mathias Krause <minipli@grsecurity.net>
> ---
> v3:
> - test BOOT_CPU_ID interaction too
> 
>  .../kvm/x86_64/max_vcpuid_cap_test.c          | 22 +++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/x86_64/max_vcpuid_cap_test.c b/tools/testing/selftests/kvm/x86_64/max_vcpuid_cap_test.c
> index 3cc4b86832fe..c2da915201be 100644
> --- a/tools/testing/selftests/kvm/x86_64/max_vcpuid_cap_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/max_vcpuid_cap_test.c
> @@ -26,19 +26,37 @@ int main(int argc, char *argv[])
>  	TEST_ASSERT(ret < 0,
>  		    "Setting KVM_CAP_MAX_VCPU_ID beyond KVM cap should fail");
>  
> +	/* Test BOOT_CPU_ID interaction (MAX_VCPU_ID cannot be lower) */
> +	if (kvm_has_cap(KVM_CAP_SET_BOOT_CPU_ID)) {
> +		vm_ioctl(vm, KVM_SET_BOOT_CPU_ID, (void *)MAX_VCPU_ID);
> +
> +		/* Try setting KVM_CAP_MAX_VCPU_ID below BOOT_CPU_ID */
> +		ret = __vm_enable_cap(vm, KVM_CAP_MAX_VCPU_ID, MAX_VCPU_ID - 1);
> +		TEST_ASSERT(ret < 0,
> +			    "Setting KVM_CAP_MAX_VCPU_ID below BOOT_CPU_ID should fail");
> +	}
> +
>  	/* Set KVM_CAP_MAX_VCPU_ID */
>  	vm_enable_cap(vm, KVM_CAP_MAX_VCPU_ID, MAX_VCPU_ID);
>  
> -
>  	/* Try to set KVM_CAP_MAX_VCPU_ID again */
>  	ret = __vm_enable_cap(vm, KVM_CAP_MAX_VCPU_ID, MAX_VCPU_ID + 1);
>  	TEST_ASSERT(ret < 0,
>  		    "Setting KVM_CAP_MAX_VCPU_ID multiple times should fail");
>  
> -	/* Create vCPU with id beyond KVM_CAP_MAX_VCPU_ID cap*/
> +	/* Create vCPU with id beyond KVM_CAP_MAX_VCPU_ID cap */
>  	ret = __vm_ioctl(vm, KVM_CREATE_VCPU, (void *)MAX_VCPU_ID);
>  	TEST_ASSERT(ret < 0, "Creating vCPU with ID > MAX_VCPU_ID should fail");
>  
> +	/* Create vCPU with id beyond UINT_MAX */

I changed this comment to

	/* Create vCPU with bits 63:32 != 0, but an otherwise valid id */

mostly because it's specifically testing the bad truncation of the upper bits,
but also because I initially misinterpreted the intent and confused it with the
INT_MAX BUILD_BUG_ON().

> +	ret = __vm_ioctl(vm, KVM_CREATE_VCPU, (void *)(1L << 32));
> +	TEST_ASSERT(ret < 0, "Creating vCPU with ID > UINT_MAX should fail");
> +
> +	/* Create vCPU with id within bounds */
> +	ret = __vm_ioctl(vm, KVM_CREATE_VCPU, (void *)0);
> +	TEST_ASSERT(ret >= 0, "Creating vCPU with ID 0 should succeed");
> +
> +	close(ret);
>  	kvm_vm_free(vm);
>  	return 0;
>  }
> -- 
> 2.30.2
>
diff mbox series

Patch

diff --git a/tools/testing/selftests/kvm/x86_64/max_vcpuid_cap_test.c b/tools/testing/selftests/kvm/x86_64/max_vcpuid_cap_test.c
index 3cc4b86832fe..c2da915201be 100644
--- a/tools/testing/selftests/kvm/x86_64/max_vcpuid_cap_test.c
+++ b/tools/testing/selftests/kvm/x86_64/max_vcpuid_cap_test.c
@@ -26,19 +26,37 @@  int main(int argc, char *argv[])
 	TEST_ASSERT(ret < 0,
 		    "Setting KVM_CAP_MAX_VCPU_ID beyond KVM cap should fail");
 
+	/* Test BOOT_CPU_ID interaction (MAX_VCPU_ID cannot be lower) */
+	if (kvm_has_cap(KVM_CAP_SET_BOOT_CPU_ID)) {
+		vm_ioctl(vm, KVM_SET_BOOT_CPU_ID, (void *)MAX_VCPU_ID);
+
+		/* Try setting KVM_CAP_MAX_VCPU_ID below BOOT_CPU_ID */
+		ret = __vm_enable_cap(vm, KVM_CAP_MAX_VCPU_ID, MAX_VCPU_ID - 1);
+		TEST_ASSERT(ret < 0,
+			    "Setting KVM_CAP_MAX_VCPU_ID below BOOT_CPU_ID should fail");
+	}
+
 	/* Set KVM_CAP_MAX_VCPU_ID */
 	vm_enable_cap(vm, KVM_CAP_MAX_VCPU_ID, MAX_VCPU_ID);
 
-
 	/* Try to set KVM_CAP_MAX_VCPU_ID again */
 	ret = __vm_enable_cap(vm, KVM_CAP_MAX_VCPU_ID, MAX_VCPU_ID + 1);
 	TEST_ASSERT(ret < 0,
 		    "Setting KVM_CAP_MAX_VCPU_ID multiple times should fail");
 
-	/* Create vCPU with id beyond KVM_CAP_MAX_VCPU_ID cap*/
+	/* Create vCPU with id beyond KVM_CAP_MAX_VCPU_ID cap */
 	ret = __vm_ioctl(vm, KVM_CREATE_VCPU, (void *)MAX_VCPU_ID);
 	TEST_ASSERT(ret < 0, "Creating vCPU with ID > MAX_VCPU_ID should fail");
 
+	/* Create vCPU with id beyond UINT_MAX */
+	ret = __vm_ioctl(vm, KVM_CREATE_VCPU, (void *)(1L << 32));
+	TEST_ASSERT(ret < 0, "Creating vCPU with ID > UINT_MAX should fail");
+
+	/* Create vCPU with id within bounds */
+	ret = __vm_ioctl(vm, KVM_CREATE_VCPU, (void *)0);
+	TEST_ASSERT(ret >= 0, "Creating vCPU with ID 0 should succeed");
+
+	close(ret);
 	kvm_vm_free(vm);
 	return 0;
 }