hv: fix the vm pointer check before use

After using get_vm_from_vmid(), vm pointer is always not NULL. But there are still many NULL pointer checks.
This commit replaced the NULL vm pointer check with a validation check which checks the vm status.
In addition, NULL check for pointer returned by get_sos_vm() and get_vm_config() is removed.

Tracked-On: #2520
Signed-off-by: Yan, Like <like.yan@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>

hypervisor/arch/x86/configs/vm_config.c

@@ -53,6 +53,7 @@ static struct acrn_vm_config vm_configs[CONFIG_MAX_VM_NUM] __aligned(PAGE_SIZE)
 
 /*
  * @pre vm_id < CONFIG_MAX_VM_NUM
+ * @post return != NULL 
  */
 struct acrn_vm_config *get_vm_config(uint16_t vm_id)
 {

hypervisor/arch/x86/guest/vm.c

@@ -48,18 +48,24 @@ uint16_t find_free_vm_id(void)
 	return (vm_config->type == UNDEFINED_VM) ? id : INVALID_VM_ID;
 }
 
+/**
+ * @pre vm != NULL && vm->vmid < CONFIG_MAX_VM_NUM
+ */
 static inline void free_vm_id(const struct acrn_vm *vm)
 {
 	struct acrn_vm_config *vm_config = get_vm_config(vm->vm_id);
 
-	if (vm_config != NULL) {
 	vm_config->type = UNDEFINED_VM;
-	}
+}
+
+bool is_valid_vm(const struct acrn_vm *vm)
+{
+	return (vm != NULL) && (vm->state != VM_STATE_INVALID);
 }
 
 bool is_sos_vm(const struct acrn_vm *vm)
 {
-	return (vm != NULL) && (vm == sos_vm_ptr);
+	return (vm != NULL)  && (get_vm_config(vm->vm_id)->type == SOS_VM);
 }
 
 /**
@@ -447,7 +453,7 @@ int32_t create_vm(uint16_t vm_id, struct acrn_vm_config *vm_config, struct acrn_
 		}
 	}
 
-	if (need_cleanup && (vm != NULL)) {
+	if (need_cleanup && is_valid_vm(vm)) {
 		if (vm->arch_vm.nworld_eptp != NULL) {
 			(void)memset(vm->arch_vm.nworld_eptp, 0U, PAGE_SIZE);
 		}

hypervisor/common/hypercall.c

@@ -181,7 +181,7 @@ int32_t hcall_destroy_vm(uint16_t vmid)
 	int32_t ret;
 	struct acrn_vm *target_vm = get_vm_from_vmid(vmid);
 
-	if (target_vm == NULL) {
+	if (!is_valid_vm(target_vm)) {
 		ret = -1;
 	} else {
 		ret = shutdown_vm(target_vm);
@@ -206,7 +206,7 @@ int32_t hcall_start_vm(uint16_t vmid)
 	int32_t ret = 0;
 	struct acrn_vm *target_vm = get_vm_from_vmid(vmid);
 
-	if (target_vm == NULL) {
+	if (!is_valid_vm(target_vm)) {
 		ret = -1;
 	} else if (target_vm->sw.io_shared_page == NULL) {
 		ret = -1;
@@ -233,7 +233,7 @@ int32_t hcall_pause_vm(uint16_t vmid)
 	struct acrn_vm *target_vm = get_vm_from_vmid(vmid);
 	int32_t ret;
 
-	if (target_vm == NULL) {
+	if (!is_valid_vm(target_vm)) {
 	        ret = -1;
 	} else {
 		pause_vm(target_vm);
@@ -265,7 +265,7 @@ int32_t hcall_create_vcpu(struct acrn_vm *vm, uint16_t vmid, uint64_t param)
 	struct acrn_create_vcpu cv;
 	struct acrn_vm *target_vm = get_vm_from_vmid(vmid);
 
-	if ((target_vm == NULL) || (param == 0U)) {
+	if (!is_valid_vm(target_vm) || (param == 0U)) {
 	        ret = -1;
 	} else if (copy_from_gpa(vm, &cv, param, sizeof(cv)) != 0) {
 		pr_err("%s: Unable copy param to vm\n", __func__);
@@ -300,7 +300,7 @@ int32_t hcall_reset_vm(uint16_t vmid)
 	struct acrn_vm *target_vm = get_vm_from_vmid(vmid);
 	int32_t ret;
 
-	if ((target_vm == NULL) || is_sos_vm(target_vm)) {
+	if (!is_valid_vm(target_vm) || is_sos_vm(target_vm)) {
 	        ret = -1;
 	} else {
 	        ret = reset_vm(target_vm);
@@ -330,7 +330,7 @@ int32_t hcall_set_vcpu_regs(struct acrn_vm *vm, uint16_t vmid, uint64_t param)
 	int32_t ret = -1;
 
 	/* Only allow setup init ctx while target_vm is inactive */
-	if ((target_vm != NULL) && (param != 0U) && (!is_sos_vm(target_vm)) && (target_vm->state != VM_STARTED)) {
+	if (is_valid_vm(target_vm) && (param != 0U) && (!is_sos_vm(target_vm)) && (target_vm->state != VM_STARTED)) {
 		if (copy_from_gpa(vm, &vcpu_regs, param, sizeof(vcpu_regs)) != 0) {
 			pr_err("%s: Unable copy param to vm\n", __func__);
 		} else if (vcpu_regs.vcpu_id >= CONFIG_MAX_VCPUS_PER_VM) {
@@ -368,7 +368,7 @@ int32_t hcall_set_irqline(const struct acrn_vm *vm, uint16_t vmid,
 	struct acrn_vm *target_vm = get_vm_from_vmid(vmid);
 	int32_t ret;
 
-	if (target_vm == NULL) {
+	if (!is_valid_vm(target_vm)) {
 	        ret = -EINVAL;
 	} else if (ops->gsi >= vioapic_pincount(vm)) {
 	        ret = -EINVAL;
@@ -457,7 +457,7 @@ int32_t hcall_inject_msi(struct acrn_vm *vm, uint16_t vmid, uint64_t param)
 	struct acrn_msi_entry msi;
 	struct acrn_vm *target_vm = get_vm_from_vmid(vmid);
 
-	if (target_vm != NULL) {
+	if (is_valid_vm(target_vm)) {
 		(void)memset((void *)&msi, 0U, sizeof(msi));
 		if (copy_from_gpa(vm, &msi, param, sizeof(msi)) != 0) {
 			pr_err("%s: Unable copy param to vm\n", __func__);
@@ -499,7 +499,7 @@ int32_t hcall_set_ioreq_buffer(struct acrn_vm *vm, uint16_t vmid, uint64_t param
 	int32_t ret;
 
 	(void)memset((void *)&iobuf, 0U, sizeof(iobuf));
-	if ((target_vm == NULL) || (copy_from_gpa(vm, &iobuf, param, sizeof(iobuf)) != 0)) {
+	if (!is_valid_vm(target_vm) || (copy_from_gpa(vm, &iobuf, param, sizeof(iobuf)) != 0)) {
 		pr_err("%p %s: target_vm is not valid or Unable copy param to vm\n", target_vm, __func__);
 	        ret = -1;
         } else {
@@ -543,7 +543,7 @@ int32_t hcall_notify_ioreq_finish(uint16_t vmid, uint16_t vcpu_id)
 	int32_t ret = -EINVAL;
 
 	/* make sure we have set req_buf */
-	if ((target_vm != NULL) && (target_vm->sw.io_shared_page != NULL)) {
+	if (is_valid_vm(target_vm) && (target_vm->sw.io_shared_page != NULL)) {
 		dev_dbg(ACRN_DBG_HYCALL, "[%d] NOTIFY_FINISH for vcpu %d",
 			vmid, vcpu_id);
 
@@ -761,7 +761,7 @@ int32_t hcall_write_protect_page(struct acrn_vm *vm, uint16_t vmid, uint64_t wp_
 	struct acrn_vm *target_vm = get_vm_from_vmid(vmid);
 	int32_t ret;
 
-	if ((target_vm == NULL) || is_sos_vm(target_vm)) {
+	if (!is_valid_vm(target_vm) || is_sos_vm(target_vm)) {
 		pr_err("%p %s: target_vm is invalid or Targeting to service vm", target_vm, __func__);
 	        ret = -EINVAL;
 	} else {
@@ -798,7 +798,7 @@ int32_t hcall_gpa_to_hpa(struct acrn_vm *vm, uint16_t vmid, uint64_t param)
 	struct acrn_vm *target_vm = get_vm_from_vmid(vmid);
 
 	(void)memset((void *)&v_gpa2hpa, 0U, sizeof(v_gpa2hpa));
-	if ((target_vm == NULL) || (copy_from_gpa(vm, &v_gpa2hpa, param, sizeof(v_gpa2hpa)) != 0)) {
+	if (!is_valid_vm(target_vm) || (copy_from_gpa(vm, &v_gpa2hpa, param, sizeof(v_gpa2hpa)) != 0)) {
 		pr_err("target_vm is invalid or HCALL gpa2hpa: Unable copy param from vm\n");
 	        ret = -1;
 	} else {
@@ -838,7 +838,7 @@ int32_t hcall_assign_ptdev(struct acrn_vm *vm, uint16_t vmid, uint64_t param)
 	bool bdf_valid = true;
 	bool iommu_valid = true;
 
-	if (target_vm != NULL) {
+	if (is_valid_vm(target_vm)) {
 		if (param < 0x10000UL) {
 			bdf = (uint16_t) param;
 		} else {
@@ -898,7 +898,7 @@ int32_t hcall_deassign_ptdev(struct acrn_vm *vm, uint16_t vmid, uint64_t param)
 	bool bdf_valid = true;
 	struct acrn_vm *target_vm = get_vm_from_vmid(vmid);
 
-	if (target_vm == NULL) {
+	if (!is_valid_vm(target_vm)) {
 	        ret = -1;
 	} else {
 		if (param < 0x10000UL) {
@@ -938,7 +938,7 @@ int32_t hcall_set_ptdev_intr_info(struct acrn_vm *vm, uint16_t vmid, uint64_t pa
 	struct acrn_vm *target_vm = get_vm_from_vmid(vmid);
 
 	(void)memset((void *)&irq, 0U, sizeof(irq));
-	if (target_vm == NULL) {
+	if (!is_valid_vm(target_vm)) {
 	        ret = -1;
 	} else if (copy_from_gpa(vm, &irq, param, sizeof(irq)) != 0) {
 		pr_err("%s: Unable copy param to vm\n", __func__);
@@ -983,7 +983,7 @@ hcall_reset_ptdev_intr_info(struct acrn_vm *vm, uint16_t vmid, uint64_t param)
 	struct hc_ptdev_irq irq;
 	struct acrn_vm *target_vm = get_vm_from_vmid(vmid);
 
-	if (target_vm != NULL) {
+	if (is_valid_vm(target_vm)) {
 		(void)memset((void *)&irq, 0U, sizeof(irq));
 
 		if (copy_from_gpa(vm, &irq, param, sizeof(irq)) != 0) {
@@ -1036,7 +1036,7 @@ int32_t hcall_get_cpu_pm_state(struct acrn_vm *vm, uint64_t cmd, uint64_t param)
 	target_vm_id = (uint16_t)((cmd & PMCMD_VMID_MASK) >> PMCMD_VMID_SHIFT);
 	target_vm = get_vm_from_vmid(target_vm_id);
 
-	if (target_vm == NULL) {
+	if (!is_valid_vm(target_vm)) {
 	        ret = -1;
 	} else {
 
@@ -1153,8 +1153,7 @@ int32_t hcall_vm_intr_monitor(struct acrn_vm *vm, uint16_t vmid, uint64_t param)
 	uint64_t hpa;
 	struct acrn_vm *target_vm = get_vm_from_vmid(vmid);
 
-	if (target_vm != NULL) {
+	if (is_valid_vm(target_vm)) {
-
 		/* the param for this hypercall is page aligned */
 		hpa = gpa2hpa(vm, param);
 		if (hpa != INVALID_HPA) {

hypervisor/common/io_req.c

@@ -23,11 +23,9 @@ static void fire_vhm_interrupt(void)
 	struct acrn_vcpu *vcpu;
 
 	sos_vm = get_sos_vm();
-	if (sos_vm != NULL) {
 	vcpu = vcpu_from_vid(sos_vm, BOOT_CPU_ID);
 
 	vlapic_set_intr(vcpu, acrn_vhm_vector, LAPIC_TRIG_EDGE);
-	}
 }
 
 #if defined(HV_DEBUG)

hypervisor/debug/profiling.c

@@ -881,7 +881,7 @@ int32_t profiling_vm_list_info(struct acrn_vm *vm, uint64_t addr)
 
 	for (j = 0U; j < CONFIG_MAX_VM_NUM; j++) {
 		tmp_vm = get_vm_from_vmid(j);
-		if (tmp_vm == NULL) {
+		if (!is_valid_vm(tmp_vm)) {
 			break;
 		}
 		vm_info_list.num_vms++;

hypervisor/debug/shell.c

@@ -604,7 +604,7 @@ static int32_t shell_list_vcpu(__unused int32_t argc, __unused char **argv)
 
 	for (idx = 0U; idx < CONFIG_MAX_VM_NUM; idx++) {
 		vm = get_vm_from_vmid(idx);
-		if (vm == NULL) {
+		if (!is_valid_vm(vm)) {
 			continue;
 		}
 		foreach_vcpu(i, vm, vcpu) {
@@ -754,7 +754,7 @@ static int32_t shell_vcpu_dumpreg(int32_t argc, char **argv)
 	vcpu_id = (uint16_t)strtol_deci(argv[2]);
 
 	vm = get_vm_from_vmid(vm_id);
-	if (vm == NULL) {
+	if (!is_valid_vm(vm)) {
 		shell_puts("No vm found in the input <vm_id, vcpu_id>\r\n");
 		status = -EINVAL;
 		goto out;
@@ -852,13 +852,13 @@ static int32_t shell_to_sos_console(__unused int32_t argc, __unused char **argv)
 #endif
 	/* Get the virtual device node */
 	vm = get_vm_from_vmid(vm_id);
-	if (vm == NULL) {
+	if (!is_valid_vm(vm)) {
 		return -EINVAL;
 	}
 
 #ifdef CONFIG_PARTITION_MODE
 	vm_config = get_vm_config(vm_id);
-	if (vm_config != NULL && vm_config->vm_vuart == false) {
+	if (!vm_config->vm_vuart) {
 		snprintf(temp_str, TEMP_STR_SIZE, "No vUART configured for vm%d\n", vm_id);
 		shell_puts(temp_str);
 		return 0;
@@ -1072,7 +1072,7 @@ static void get_vioapic_info(char *str_arg, size_t str_max, uint16_t vmid)
 	struct acrn_vm *vm = get_vm_from_vmid(vmid);
 	uint32_t pin, pincount;
 
-	if (vm == NULL) {
+	if (!is_valid_vm(vm)) {
 		len = snprintf(str, size, "\r\nvm is not exist for vmid %hu", vmid);
 		if (len >= size) {
 			goto overflow;

hypervisor/debug/vuart.c

@@ -390,7 +390,7 @@ struct acrn_vuart *vuart_console_active(void)
 	vm = get_sos_vm();
 #endif
 
-	if (vm != NULL) {
+	if (is_valid_vm(vm)) {
 		struct acrn_vuart *vu = vm_vuart(vm);
 
 		if (vu->active) {

hypervisor/dm/vpci/vpci.c

@@ -554,11 +554,9 @@ void vpci_reset_ptdev_intr_info(const struct acrn_vm *target_vm, uint16_t vbdf,
 		if (vdev->vpci->vm == target_vm) {
 			vm = get_sos_vm();
 
-			if (vm != NULL) {
 			vdev->vpci = &vm->vpci;
 			/* vbdf equals to pbdf in sos */
 			vdev->vbdf.value = vdev->pdev->bdf.value;
 		}
 	}
-	}
 }

hypervisor/include/arch/x86/guest/vm.h

@@ -209,6 +209,7 @@ int32_t reset_vm(struct acrn_vm *vm);
 int32_t create_vm(uint16_t vm_id, struct acrn_vm_config *vm_config, struct acrn_vm **rtn_vm);
 void prepare_vm(uint16_t vm_id, struct acrn_vm_config *vm_config);
 void launch_vms(uint16_t pcpu_id);
+bool is_valid_vm(const struct acrn_vm *vm);
 bool is_sos_vm(const struct acrn_vm *vm);
 uint16_t find_free_vm_id(void);
 struct acrn_vm *get_vm_from_vmid(uint16_t vm_id);