* [PATCH v3 0/5] KVM: Reject vCPU IDs above 2^32
@ 2024-06-14 20:28 Mathias Krause
2024-06-14 20:28 ` [PATCH v3 1/5] KVM: Reject overly excessive IDs in KVM_CREATE_VCPU Mathias Krause
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Mathias Krause @ 2024-06-14 20:28 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson; +Cc: kvm, Mathias Krause
This small series evolved from a single vCPU ID limit check to
multiple ones, including sanity checks among them and enhanced
selftests.
Please apply!
Thanks,
Mathias
v2: https://lore.kernel.org/kvm/20240612215415.3450952-1-minipli@grsecurity.net/
changes v2->v3:
- test max_vcpu_ids on bsp_vcpu_id limit check if already set (Sean)
- add patch to check max_vcpu_ids to not exclude bsp_vcpu_id (Sean)
- reorder series to have selftests at the end
- minor commit log cosmetics
changes v1->v2:
- add comment and build bug to make truncation check more obvious (Sean)
- handle KVM_SET_BOOT_CPU_ID similar
Mathias Krause (4):
KVM: Reject overly excessive IDs in KVM_CREATE_VCPU
KVM: x86: Limit check IDs for KVM_SET_BOOT_CPU_ID
KVM: selftests: Test max vCPU IDs corner cases
KVM: selftests: Test vCPU boot IDs above 2^32
Sean Christopherson (1):
KVM: x86: Prevent excluding the BSP on setting max_vcpu_ids
arch/x86/kvm/x86.c | 7 +++++-
.../kvm/x86_64/max_vcpuid_cap_test.c | 22 +++++++++++++++++--
.../selftests/kvm/x86_64/set_boot_cpu_id.c | 11 ++++++++++
virt/kvm/kvm_main.c | 10 ++++++++-
4 files changed, 46 insertions(+), 4 deletions(-)
--
2.30.2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v3 1/5] KVM: Reject overly excessive IDs in KVM_CREATE_VCPU
2024-06-14 20:28 [PATCH v3 0/5] KVM: Reject vCPU IDs above 2^32 Mathias Krause
@ 2024-06-14 20:28 ` Mathias Krause
2024-06-18 21:43 ` Sean Christopherson
2024-06-14 20:28 ` [PATCH v3 2/5] KVM: x86: Limit check IDs for KVM_SET_BOOT_CPU_ID Mathias Krause
` (4 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Mathias Krause @ 2024-06-14 20:28 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson
Cc: kvm, Mathias Krause, Emese Revfy, PaX Team
If, on a 64 bit system, a vCPU ID is provided that has the upper 32 bits
set to a non-zero value, it may get accepted if the truncated to 32 bits
integer value is below KVM_MAX_VCPU_IDS and 'max_vcpus'. This feels very
wrong and triggered the reporting logic of PaX's SIZE_OVERFLOW plugin.
Instead of silently truncating and accepting such values, pass the full
value to kvm_vm_ioctl_create_vcpu() and make the existing limit checks
return an error.
Even if this is a userland ABI breaking change, no sane userland could
have ever relied on that behaviour.
Reported-by: PaX's SIZE_OVERFLOW plugin running on grsecurity's syzkaller
Fixes: 6aa8b732ca01 ("[PATCH] kvm: userspace interface")
Cc: Emese Revfy <re.emese@gmail.com>
Cc: PaX Team <pageexec@freemail.hu>
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
---
virt/kvm/kvm_main.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 14841acb8b95..b04e87f6568f 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -4200,12 +4200,20 @@ static void kvm_create_vcpu_debugfs(struct kvm_vcpu *vcpu)
/*
* Creates some virtual cpus. Good luck creating more than one.
*/
-static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
+static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id)
{
int r;
struct kvm_vcpu *vcpu;
struct page *page;
+ /*
+ * KVM tracks vCPU IDs as 'int', be kind to userspace and reject
+ * too-large values instead of silently truncating.
+ *
+ * Also ensure we're not breaking this assumption by accidentally
+ * pushing KVM_MAX_VCPU_IDS above INT_MAX.
+ */
+ BUILD_BUG_ON(KVM_MAX_VCPU_IDS > INT_MAX);
if (id >= KVM_MAX_VCPU_IDS)
return -EINVAL;
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 2/5] KVM: x86: Limit check IDs for KVM_SET_BOOT_CPU_ID
2024-06-14 20:28 [PATCH v3 0/5] KVM: Reject vCPU IDs above 2^32 Mathias Krause
2024-06-14 20:28 ` [PATCH v3 1/5] KVM: Reject overly excessive IDs in KVM_CREATE_VCPU Mathias Krause
@ 2024-06-14 20:28 ` Mathias Krause
2024-06-14 20:28 ` [PATCH v3 3/5] KVM: x86: Prevent excluding the BSP on setting max_vcpu_ids Mathias Krause
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Mathias Krause @ 2024-06-14 20:28 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson; +Cc: kvm, Mathias Krause
Do not accept IDs which are definitely invalid by limit checking the
passed value against KVM_MAX_VCPU_IDS and 'max_vcpu_ids' if it was
already set.
This ensures invalid values, especially on 64-bit systems, don't go
unnoticed and lead to a valid id by chance when truncated by the final
assignment.
Fixes: 73880c80aa9c ("KVM: Break dependency between vcpu index in vcpus array and vcpu_id.")
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
---
v3:
- check against arch.max_vcpu_ids too, if already set (Sean)
- switch to else-if instead of goto (Sean)
arch/x86/kvm/x86.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 082ac6d95a3a..686606f61dee 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7222,6 +7222,9 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
mutex_lock(&kvm->lock);
if (kvm->created_vcpus)
r = -EBUSY;
+ else if (arg > KVM_MAX_VCPU_IDS ||
+ (kvm->arch.max_vcpu_ids && arg > kvm->arch.max_vcpu_ids))
+ r = -EINVAL;
else
kvm->arch.bsp_vcpu_id = arg;
mutex_unlock(&kvm->lock);
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 3/5] KVM: x86: Prevent excluding the BSP on setting max_vcpu_ids
2024-06-14 20:28 [PATCH v3 0/5] KVM: Reject vCPU IDs above 2^32 Mathias Krause
2024-06-14 20:28 ` [PATCH v3 1/5] KVM: Reject overly excessive IDs in KVM_CREATE_VCPU Mathias Krause
2024-06-14 20:28 ` [PATCH v3 2/5] KVM: x86: Limit check IDs for KVM_SET_BOOT_CPU_ID Mathias Krause
@ 2024-06-14 20:28 ` Mathias Krause
2024-06-14 20:28 ` [PATCH v3 4/5] KVM: selftests: Test max vCPU IDs corner cases Mathias Krause
` (2 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Mathias Krause @ 2024-06-14 20:28 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson; +Cc: kvm, Mathias Krause
From: Sean Christopherson <seanjc@google.com>
If the BSP vCPU ID was already set, ensure it doesn't get excluded when
limiting vCPU IDs via KVM_CAP_MAX_VCPU_ID.
[mks: provide commit message, code by Sean]
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
---
arch/x86/kvm/x86.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 686606f61dee..eeb35fb1573a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6701,7 +6701,9 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
break;
mutex_lock(&kvm->lock);
- if (kvm->arch.max_vcpu_ids == cap->args[0]) {
+ if (kvm->arch.bsp_vcpu_id > cap->args[0]) {
+ ;
+ } else if (kvm->arch.max_vcpu_ids == cap->args[0]) {
r = 0;
} else if (!kvm->arch.max_vcpu_ids) {
kvm->arch.max_vcpu_ids = cap->args[0];
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 4/5] KVM: selftests: Test max vCPU IDs corner cases
2024-06-14 20:28 [PATCH v3 0/5] KVM: Reject vCPU IDs above 2^32 Mathias Krause
` (2 preceding siblings ...)
2024-06-14 20:28 ` [PATCH v3 3/5] KVM: x86: Prevent excluding the BSP on setting max_vcpu_ids Mathias Krause
@ 2024-06-14 20:28 ` Mathias Krause
2024-06-18 21:46 ` Sean Christopherson
2024-06-14 20:28 ` [PATCH v3 5/5] KVM: selftests: Test vCPU boot IDs above 2^32 Mathias Krause
2024-06-18 21:41 ` [PATCH v3 0/5] KVM: Reject vCPU " Sean Christopherson
5 siblings, 1 reply; 11+ messages in thread
From: Mathias Krause @ 2024-06-14 20:28 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson; +Cc: kvm, Mathias Krause
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 */
+ 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v3 5/5] KVM: selftests: Test vCPU boot IDs above 2^32
2024-06-14 20:28 [PATCH v3 0/5] KVM: Reject vCPU IDs above 2^32 Mathias Krause
` (3 preceding siblings ...)
2024-06-14 20:28 ` [PATCH v3 4/5] KVM: selftests: Test max vCPU IDs corner cases Mathias Krause
@ 2024-06-14 20:28 ` Mathias Krause
2024-06-18 21:50 ` Sean Christopherson
2024-06-18 21:41 ` [PATCH v3 0/5] KVM: Reject vCPU " Sean Christopherson
5 siblings, 1 reply; 11+ messages in thread
From: Mathias Krause @ 2024-06-14 20:28 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson; +Cc: kvm, Mathias Krause
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));
+
+ 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++)
--
2.30.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/5] KVM: Reject vCPU IDs above 2^32
2024-06-14 20:28 [PATCH v3 0/5] KVM: Reject vCPU IDs above 2^32 Mathias Krause
` (4 preceding siblings ...)
2024-06-14 20:28 ` [PATCH v3 5/5] KVM: selftests: Test vCPU boot IDs above 2^32 Mathias Krause
@ 2024-06-18 21:41 ` Sean Christopherson
2024-06-19 6:28 ` Mathias Krause
5 siblings, 1 reply; 11+ messages in thread
From: Sean Christopherson @ 2024-06-18 21:41 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini, Mathias Krause; +Cc: kvm
On Fri, 14 Jun 2024 22:28:54 +0200, Mathias Krause wrote:
> This small series evolved from a single vCPU ID limit check to
> multiple ones, including sanity checks among them and enhanced
> selftests.
Applied to kvm-x86 generic, with a few tweaks (emails incoming). Thanks!
[1/5] KVM: Reject overly excessive IDs in KVM_CREATE_VCPU
https://github.com/kvm-x86/linux/commit/8b8e57e5096e
[2/5] KVM: x86: Limit check IDs for KVM_SET_BOOT_CPU_ID
https://github.com/kvm-x86/linux/commit/7c305d5118e6
[3/5] KVM: x86: Prevent excluding the BSP on setting max_vcpu_ids
https://github.com/kvm-x86/linux/commit/d29bf2ca1404
[4/5] KVM: selftests: Test max vCPU IDs corner cases
https://github.com/kvm-x86/linux/commit/4b451a57809c
[5/5] KVM: selftests: Test vCPU boot IDs above 2^32
https://github.com/kvm-x86/linux/commit/438a496b9041
--
https://github.com/kvm-x86/linux/tree/next
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 1/5] KVM: Reject overly excessive IDs in KVM_CREATE_VCPU
2024-06-14 20:28 ` [PATCH v3 1/5] KVM: Reject overly excessive IDs in KVM_CREATE_VCPU Mathias Krause
@ 2024-06-18 21:43 ` Sean Christopherson
0 siblings, 0 replies; 11+ messages in thread
From: Sean Christopherson @ 2024-06-18 21:43 UTC (permalink / raw)
To: Mathias Krause; +Cc: Paolo Bonzini, kvm, Emese Revfy, PaX Team
On Fri, Jun 14, 2024, Mathias Krause wrote:
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 14841acb8b95..b04e87f6568f 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -4200,12 +4200,20 @@ static void kvm_create_vcpu_debugfs(struct kvm_vcpu *vcpu)
> /*
> * Creates some virtual cpus. Good luck creating more than one.
> */
> -static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id)
> +static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, unsigned long id)
> {
> int r;
> struct kvm_vcpu *vcpu;
> struct page *page;
>
> + /*
> + * KVM tracks vCPU IDs as 'int', be kind to userspace and reject
> + * too-large values instead of silently truncating.
> + *
> + * Also ensure we're not breaking this assumption by accidentally
> + * pushing KVM_MAX_VCPU_IDS above INT_MAX.
I tweaked this slightly because it's not just accidental changes we need to
guard against, and to "hint" that vcpu_id really should be an "unsigned int".
/*
* KVM tracks vCPU IDs as 'int', be kind to userspace and reject
* too-large values instead of silently truncating.
*
* Ensure KVM_MAX_VCPU_IDS isn't pushed above INT_MAX without first
* changing the storage type (at the very least, IDs should be tracked
* as unsigned ints).
*/
> + */
> + BUILD_BUG_ON(KVM_MAX_VCPU_IDS > INT_MAX);
> if (id >= KVM_MAX_VCPU_IDS)
> return -EINVAL;
>
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 4/5] KVM: selftests: Test max vCPU IDs corner cases
2024-06-14 20:28 ` [PATCH v3 4/5] KVM: selftests: Test max vCPU IDs corner cases Mathias Krause
@ 2024-06-18 21:46 ` Sean Christopherson
0 siblings, 0 replies; 11+ messages in thread
From: Sean Christopherson @ 2024-06-18 21:46 UTC (permalink / raw)
To: Mathias Krause; +Cc: Paolo Bonzini, kvm
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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 5/5] KVM: selftests: Test vCPU boot IDs above 2^32
2024-06-14 20:28 ` [PATCH v3 5/5] KVM: selftests: Test vCPU boot IDs above 2^32 Mathias Krause
@ 2024-06-18 21:50 ` Sean Christopherson
0 siblings, 0 replies; 11+ messages in thread
From: Sean Christopherson @ 2024-06-18 21:50 UTC (permalink / raw)
To: Mathias Krause; +Cc: Paolo Bonzini, kvm
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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v3 0/5] KVM: Reject vCPU IDs above 2^32
2024-06-18 21:41 ` [PATCH v3 0/5] KVM: Reject vCPU " Sean Christopherson
@ 2024-06-19 6:28 ` Mathias Krause
0 siblings, 0 replies; 11+ messages in thread
From: Mathias Krause @ 2024-06-19 6:28 UTC (permalink / raw)
To: Sean Christopherson; +Cc: kvm, Paolo Bonzini
On 18.06.24 23:41, Sean Christopherson wrote:
> On Fri, 14 Jun 2024 22:28:54 +0200, Mathias Krause wrote:
>> This small series evolved from a single vCPU ID limit check to
>> multiple ones, including sanity checks among them and enhanced
>> selftests.
>
> Applied to kvm-x86 generic, with a few tweaks (emails incoming). Thanks!
>
> [1/5] KVM: Reject overly excessive IDs in KVM_CREATE_VCPU
> https://github.com/kvm-x86/linux/commit/8b8e57e5096e
> [2/5] KVM: x86: Limit check IDs for KVM_SET_BOOT_CPU_ID
> https://github.com/kvm-x86/linux/commit/7c305d5118e6
> [3/5] KVM: x86: Prevent excluding the BSP on setting max_vcpu_ids
> https://github.com/kvm-x86/linux/commit/d29bf2ca1404
> [4/5] KVM: selftests: Test max vCPU IDs corner cases
> https://github.com/kvm-x86/linux/commit/4b451a57809c
> [5/5] KVM: selftests: Test vCPU boot IDs above 2^32
> https://github.com/kvm-x86/linux/commit/438a496b9041
Looking good, thanks Sean!
Mathias
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-06-19 6:28 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-14 20:28 [PATCH v3 0/5] KVM: Reject vCPU IDs above 2^32 Mathias Krause
2024-06-14 20:28 ` [PATCH v3 1/5] KVM: Reject overly excessive IDs in KVM_CREATE_VCPU Mathias Krause
2024-06-18 21:43 ` Sean Christopherson
2024-06-14 20:28 ` [PATCH v3 2/5] KVM: x86: Limit check IDs for KVM_SET_BOOT_CPU_ID Mathias Krause
2024-06-14 20:28 ` [PATCH v3 3/5] KVM: x86: Prevent excluding the BSP on setting max_vcpu_ids Mathias Krause
2024-06-14 20:28 ` [PATCH v3 4/5] KVM: selftests: Test max vCPU IDs corner cases Mathias Krause
2024-06-18 21:46 ` Sean Christopherson
2024-06-14 20:28 ` [PATCH v3 5/5] KVM: selftests: Test vCPU boot IDs above 2^32 Mathias Krause
2024-06-18 21:50 ` Sean Christopherson
2024-06-18 21:41 ` [PATCH v3 0/5] KVM: Reject vCPU " Sean Christopherson
2024-06-19 6:28 ` Mathias Krause
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).