hv: fix possible buffer overflow issues

 - cpu_secondary_init() @cpu.c
 - ptirq_intx_pin_remap() @ assign.c
   etc.

Tracked-On: #1252
Signed-off-by: Yonghua Huang <yonghua.huang@intel.com>
Acked-by: Eddie Dong <eddie.dong@intel.com>

Author: yonghuah

hypervisor/arch/x86/assign.c

@@ -628,7 +628,8 @@ static void activate_physical_ioapic(struct acrn_vm *vm,
 int32_t ptirq_intx_pin_remap(struct acrn_vm *vm, uint8_t virt_pin,
 		enum ptirq_vpin_source vpin_src)
 {
-	struct ptirq_remapping_info *entry;
+	int32_t status = 0;
+	struct ptirq_remapping_info *entry = NULL;
 	bool need_switch_vpin_src = false;
 	DEFINE_IOAPIC_SID(virt_sid, virt_pin, vpin_src);
 	bool pic_pin = (vpin_src == PTDEV_VPIN_PIC);
@@ -647,80 +648,80 @@ int32_t ptirq_intx_pin_remap(struct acrn_vm *vm, uint8_t virt_pin,
 	/* no remap for hypervisor owned intx */
 #ifdef CONFIG_COM_IRQ
 	if (ptdev_hv_owned_intx(vm, &virt_sid)) {
-		goto END;
+		status = -ENODEV;
 	}
 #endif /* CONFIG_COM_IRQ */
 
-	/* query if we have virt to phys mapping */
-	spinlock_obtain(&ptdev_lock);
-	entry = ptirq_lookup_entry_by_vpin(vm, virt_pin, pic_pin);
-	if (entry == NULL) {
-		if (is_vm0(vm)) {
-
-			/* for vm0, there is chance of vpin source switch
-			 * between vPIC & vIOAPIC for one legacy phys_pin.
-			 *
-			 * here checks if there is already mapping entry from
-			 * the other vpin source for legacy pin. If yes, then
-			 * switch vpin source is needed
-			 */
-			if (virt_pin < NR_LEGACY_PIN) {
-				uint8_t vpin = pic_ioapic_pin_map[virt_pin];
-
-				entry = ptirq_lookup_entry_by_vpin(vm, vpin, !pic_pin);
-				if (entry != NULL) {
-					need_switch_vpin_src = true;
+	if (status || (pic_pin && (virt_pin >= NR_VPIC_PINS_TOTAL))) {
+		status = -EINVAL;
+	} else {
+		/* query if we have virt to phys mapping */
+		spinlock_obtain(&ptdev_lock);
+		entry = ptirq_lookup_entry_by_vpin(vm, virt_pin, pic_pin);
+		if (entry == NULL) {
+			if (is_vm0(vm)) {
+
+				/* for vm0, there is chance of vpin source switch
+				 * between vPIC & vIOAPIC for one legacy phys_pin.
+				 *
+				 * here checks if there is already mapping entry from
+				 * the other vpin source for legacy pin. If yes, then
+				 * switch vpin source is needed
+				 */
+				if (virt_pin < NR_LEGACY_PIN) {
+					uint8_t vpin = pic_ioapic_pin_map[virt_pin];
+
+					entry = ptirq_lookup_entry_by_vpin(vm, vpin, !pic_pin);
+					if (entry != NULL) {
+						need_switch_vpin_src = true;
+					}
 				}
-			}
-
-			/* entry could be updated by above switch check */
-			if (entry == NULL) {
-				uint8_t phys_pin = virt_pin;
 
-				/* fix vPIC pin to correct native IOAPIC pin */
-				if (pic_pin) {
-					phys_pin = pic_ioapic_pin_map[virt_pin];
-				}
-				entry = add_intx_remapping(vm,
-						virt_pin, phys_pin, pic_pin);
+				/* entry could be updated by above switch check */
 				if (entry == NULL) {
-					pr_err("%s, add intx remapping failed",
-						__func__);
-					spinlock_release(&ptdev_lock);
-					return -ENODEV;
+					uint8_t phys_pin = virt_pin;
+
+					/* fix vPIC pin to correct native IOAPIC pin */
+					if (pic_pin) {
+						phys_pin = pic_ioapic_pin_map[virt_pin];
+					}
+					entry = add_intx_remapping(vm,
+							virt_pin, phys_pin, pic_pin);
+					if (entry == NULL) {
+						pr_err("%s, add intx remapping failed",
+								__func__);
+						status = -ENODEV;
+					}
 				}
+			} else {
+				/* ptirq_intx_pin_remap is triggered by vPIC/vIOAPIC
+				 * everytime a pin get unmask, here filter out pins
+				 * not get mapped.
+				 */
+				status = -ENODEV;
 			}
-		} else {
-			/* ptirq_intx_pin_remap is triggered by vPIC/vIOAPIC
-			 * everytime a pin get unmask, here filter out pins
-			 * not get mapped.
-			 */
-			 spinlock_release(&ptdev_lock);
-			goto END;
 		}
+		spinlock_release(&ptdev_lock);
 	}
 
-	/* if vpin source need switch */
-	if (need_switch_vpin_src) {
-		dev_dbg(ACRN_DBG_IRQ,
-			"IOAPIC pin=%hhu pirq=%u vpin=%d switch from %s to %s "
-			"vpin=%d for vm%d", entry->phys_sid.intx_id.pin,
-			entry->allocated_pirq, entry->virt_sid.intx_id.pin,
-			(vpin_src == 0)? "vPIC" : "vIOAPIC",
-			(vpin_src == 0)? "vIOPIC" : "vPIC",
-			virt_pin, entry->vm->vm_id);
-	        entry->virt_sid.value = virt_sid.value;
+	if (!status) {
+		spinlock_obtain(&ptdev_lock);
+		/* if vpin source need switch */
+		if ((need_switch_vpin_src) && (entry != NULL)) {
+			dev_dbg(ACRN_DBG_IRQ,
+				"IOAPIC pin=%hhu pirq=%u vpin=%d switch from %s to %s vpin=%d for vm%d",
+				entry->phys_sid.intx_id.pin,
+				entry->allocated_pirq, entry->virt_sid.intx_id.pin,
+				(vpin_src == 0) ? "vPIC" : "vIOAPIC",
+				(vpin_src == 0) ? "vIOPIC" : "vPIC",
+				virt_pin, entry->vm->vm_id);
+			entry->virt_sid.value = virt_sid.value;
+		}
+		spinlock_release(&ptdev_lock);
+		activate_physical_ioapic(vm, entry);
 	}
-	spinlock_release(&ptdev_lock);
 
-	activate_physical_ioapic(vm, entry);
-	dev_dbg(ACRN_DBG_IRQ,
-			"IOAPIC pin=%hhu pirq=%u assigned to vm%d %s vpin=%d",
-			entry->phys_sid.intx_id.pin, entry->allocated_pirq,
-			entry->vm->vm_id, vpin_src == PTDEV_VPIN_PIC ?
-			"vPIC" : "vIOAPIC", virt_pin);
-END:
-	return 0;
+	return status;
 }
 
 /* @pre vm != NULL

hypervisor/arch/x86/cpu.c

@@ -407,6 +407,9 @@ void init_cpu_pre(uint16_t pcpu_id)
 		early_init_lapic();
 
 		pcpu_id = get_cpu_id_from_lapic_id(get_cur_lapic_id());
+		if (pcpu_id >= CONFIG_MAX_PCPU_NUM) {
+			panic("Invalid pCPU ID!");
+		}
 	}
 
 	bitmap_set_nolock(pcpu_id, &pcpu_active_bitmap);

hypervisor/common/hypercall.c

@@ -1049,46 +1049,44 @@ int32_t hcall_get_cpu_pm_state(struct acrn_vm *vm, uint64_t cmd, uint64_t param)
  */
 int32_t hcall_vm_intr_monitor(struct acrn_vm *vm, uint16_t vmid, uint64_t param)
 {
+	int32_t status = -EINVAL;
 	struct acrn_intr_monitor *intr_hdr;
 	uint64_t hpa;
 	struct acrn_vm *target_vm = get_vm_from_vmid(vmid);
 
-	if (target_vm == NULL) {
-		return -1;
-	}
-
-	/* the param for this hypercall is page aligned */
-	hpa = gpa2hpa(vm, param);
-	if (hpa == INVALID_HPA) {
-		pr_err("%s,vm[%hu] gpa 0x%llx,GPA is unmapping.",
-			__func__, vm->vm_id, param);
-		return -EINVAL;
-	}
-
-	intr_hdr = (struct acrn_intr_monitor *)hpa2hva(hpa);
-
-	stac();
-	switch (intr_hdr->cmd) {
-	case INTR_CMD_GET_DATA:
-		intr_hdr->buf_cnt = ptirq_get_intr_data(target_vm,
-			intr_hdr->buffer, intr_hdr->buf_cnt);
-		break;
-
-	case INTR_CMD_DELAY_INT:
-		/* buffer[0] is the delay time (in MS), if 0 to cancel delay */
-		target_vm->intr_inject_delay_delta =
-			intr_hdr->buffer[0] * CYCLES_PER_MS;
-		break;
-
-	default:
-		/* if cmd wrong it goes here should not happen */
-		break;
+	if (target_vm != NULL) {
+
+		/* the param for this hypercall is page aligned */
+		hpa = gpa2hpa(vm, param);
+		if (hpa != INVALID_HPA) {
+			intr_hdr = (struct acrn_intr_monitor *)hpa2hva(hpa);
+			stac();
+			if (intr_hdr->buf_cnt <= (MAX_PTDEV_NUM * 2U)) {
+				switch (intr_hdr->cmd) {
+				case INTR_CMD_GET_DATA:
+					intr_hdr->buf_cnt = ptirq_get_intr_data(target_vm,
+						intr_hdr->buffer, intr_hdr->buf_cnt);
+					break;
+
+				case INTR_CMD_DELAY_INT:
+					/* buffer[0] is the delay time (in MS), if 0 to cancel delay */
+					target_vm->intr_inject_delay_delta =
+						intr_hdr->buffer[0] * CYCLES_PER_MS;
+					break;
+
+				default:
+					/* if cmd wrong it goes here should not happen */
+					break;
+				}
+
+				status = 0;
+				pr_dbg("intr monitor:%d, cnt=%d", intr_hdr->cmd, intr_hdr->buf_cnt);
+			}
+			clac();
+		}
 	}
 
-	pr_dbg("intr monitor:%d, cnt=%d", intr_hdr->cmd, intr_hdr->buf_cnt);
-	clac();
-
-	return 0;
+	return status;
 }
 
 /**

hypervisor/common/ptdev.c

@@ -123,7 +123,8 @@ void ptirq_release_entry(struct ptirq_remapping_info *entry)
 	list_del_init(&entry->softirq_node);
 	spinlock_irqrestore_release(&entry->vm->softirq_dev_lock, rflags);
 	atomic_clear32(&entry->active, ACTIVE_FLAG);
-	bitmap_clear_nolock((entry->ptdev_entry_id) & 0x3FU, &ptirq_entry_bitmaps[(entry->ptdev_entry_id) >> 6U]);
+	bitmap_clear_nolock((entry->ptdev_entry_id) & 0x3FU,
+		&ptirq_entry_bitmaps[((entry->ptdev_entry_id) & 0x3FU) >> 6U]);
 }
 
 /* interrupt context */

hypervisor/include/public/acrn_common.h

@@ -626,7 +626,7 @@ enum pm_cmd_type {
  *
  * the parameter for HC_VM_INTR_MONITOR hypercall
  */
-#define MAX_PTDEV_NUM 24
+#define MAX_PTDEV_NUM 24U
 struct acrn_intr_monitor {
 	/** sub command for intr monitor */
 	uint32_t cmd;