Fix panic in vm_find() for reserved IDs
vm_find() is called from VM API and should return NULL if the given ID
does not correspond to a valid VM. This was not the case for ID zero
(reserved for the hypervisor), upon which Hafnium would fail a CHECK
and panic. Fix the issue and add a test case.
Test: hf_vcpu_get_count.reserved_vm_id
Change-Id: I0205e64c5cb5a98cdb24ba2cd75d417845ffaddd
diff --git a/src/vm.c b/src/vm.c
index 08e9531..530ff6b 100644
--- a/src/vm.c
+++ b/src/vm.c
@@ -27,15 +27,6 @@
static struct vm vms[MAX_VMS];
static spci_vm_count_t vm_count;
-/**
- * Returns the index of the VM within the VM array.
- */
-static uint16_t vm_get_vm_index(spci_vm_id_t vm_id)
-{
- CHECK(vm_id >= HF_VM_ID_OFFSET);
- return vm_id - HF_VM_ID_OFFSET;
-}
-
bool vm_init(spci_vcpu_count_t vcpu_count, struct mpool *ppool,
struct vm **new_vm)
{
@@ -89,14 +80,21 @@
struct vm *vm_find(spci_vm_id_t id)
{
- uint16_t vm_index = vm_get_vm_index(id);
+ uint16_t index;
- /* Ensure the VM is initialized. */
- if (vm_index >= vm_count) {
+ /* Check that this is not a reserved ID. */
+ if (id < HF_VM_ID_OFFSET) {
return NULL;
}
- return &vms[vm_index];
+ index = id - HF_VM_ID_OFFSET;
+
+ /* Ensure the VM is initialized. */
+ if (index >= vm_count) {
+ return NULL;
+ }
+
+ return &vms[index];
}
/**
diff --git a/test/vmapi/primary_only/primary_only.c b/test/vmapi/primary_only/primary_only.c
index 4bede95..45cbd52 100644
--- a/test/vmapi/primary_only/primary_only.c
+++ b/test/vmapi/primary_only/primary_only.c
@@ -24,6 +24,12 @@
#include "hftest.h"
+/*
+ * TODO: Some of these tests are duplicated between 'primary_only' and
+ * 'primary_with_secondaries'. Move them to a common place consider running
+ * them inside secondary VMs too.
+ */
+
/**
* Confirms the primary VM has the primary ID.
*/
@@ -58,10 +64,22 @@
}
/**
+ * Confirm an error is returned when getting the vcpu count for a reserved ID.
+ */
+TEST(hf_vcpu_get_count, reserved_vm_id)
+{
+ spci_vm_id_t id;
+
+ for (id = 0; id < HF_VM_ID_OFFSET; ++id) {
+ EXPECT_EQ(hf_vcpu_get_count(id), 0);
+ }
+}
+
+/**
* Confirm an error is returned when getting the vcpu count of a VM with an ID
* that is likely to be far outside the resource limit.
*/
-TEST(hf_vcpu_get_count, large_invalid_vm_index)
+TEST(hf_vcpu_get_count, large_invalid_vm_id)
{
EXPECT_EQ(hf_vcpu_get_count(0xffff), 0);
}
diff --git a/test/vmapi/primary_with_secondaries/no_services.c b/test/vmapi/primary_with_secondaries/no_services.c
index 2f2c0b5..edaa90a 100644
--- a/test/vmapi/primary_with_secondaries/no_services.c
+++ b/test/vmapi/primary_with_secondaries/no_services.c
@@ -59,10 +59,22 @@
}
/**
+ * Confirm an error is returned when getting the vcpu count for a reserved ID.
+ */
+TEST(hf_vcpu_get_count, reserved_vm_id)
+{
+ spci_vm_id_t id;
+
+ for (id = 0; id < HF_VM_ID_OFFSET; ++id) {
+ EXPECT_EQ(hf_vcpu_get_count(id), 0);
+ }
+}
+
+/**
* Confirm it is an error to query how many VCPUs are assigned to a nonexistent
* secondary VM.
*/
-TEST(hf_vcpu_get_count, large_invalid_vm_index)
+TEST(hf_vcpu_get_count, large_invalid_vm_id)
{
EXPECT_EQ(hf_vcpu_get_count(0xffff), 0);
}
