From cb0009f4d2058f95dc6a68c804bb08ebf88e7c6b Mon Sep 17 00:00:00 2001 From: Shiqing Gao Date: Thu, 12 Jul 2018 11:25:26 +0800 Subject: [PATCH] hv: cpu: fix 'Pointer arithmetic is not on array' Use the array for lapic_id directly to avoid the unnecessary pointer arithmetic. With current implementation, lapic_id_base is always a byte array with CPU_PAGE_SIZE elements What this patch does: - replace 'uint8_t *lapic_id_base' with 'uint8_t lapic_id_array[CPU_PAGE_SIZE]' to make the boundary explicit - add a range check to ensure that there is no overflow v2 -> v3: * update the array size of lapic_id_array per discussion with Fengwei v1 -> v2: * remove the unnecessary range check in parse_madt in cpu.c Signed-off-by: Shiqing Gao Reviewed-by: Junjie Mao Acked-by: Eddie Dong --- hypervisor/arch/x86/cpu.c | 34 +++++++++++++------------------ hypervisor/boot/acpi.c | 19 +++++++++++------ hypervisor/include/arch/x86/cpu.h | 3 +++ 3 files changed, 30 insertions(+), 26 deletions(-) diff --git a/hypervisor/arch/x86/cpu.c b/hypervisor/arch/x86/cpu.c index 14572c8b3..14f50ee8b 100644 --- a/hypervisor/arch/x86/cpu.c +++ b/hypervisor/arch/x86/cpu.c @@ -28,7 +28,7 @@ volatile uint16_t up_count = 0U; uint64_t pcpu_active_bitmap = 0UL; /* TODO: add more capability per requirement */ -/*APICv features*/ +/* APICv features */ #define VAPIC_FEATURE_VIRT_ACCESS (1U << 0) #define VAPIC_FEATURE_VIRT_REG (1U << 1) #define VAPIC_FEATURE_INTR_DELIVERY (1U << 2) @@ -251,45 +251,39 @@ static void alloc_phy_cpu_data(uint16_t pcpu_num) ASSERT(per_cpu_data_base_ptr != NULL, ""); } -uint16_t __attribute__((weak)) parse_madt(uint8_t *lapic_id_base) +uint16_t __attribute__((weak)) parse_madt(uint8_t lapic_id_array[MAX_PCPU_NUM]) { static const uint8_t lapic_id[] = {0U, 2U, 4U, 6U}; uint32_t i; for (i = 0U; i < ARRAY_SIZE(lapic_id); i++) { - *lapic_id_base = lapic_id[i]; - lapic_id_base++; + lapic_id_array[i] = lapic_id[i]; } return ((uint16_t)ARRAY_SIZE(lapic_id)); } -static void init_phy_cpu_storage(void) +static void init_percpu_data_area(void) { uint16_t i; - uint16_t pcpu_num=0U; + uint16_t pcpu_num = 0U; uint16_t bsp_cpu_id; uint8_t bsp_lapic_id = 0U; - uint8_t *lapic_id_base; + uint8_t lapic_id_array[MAX_PCPU_NUM]; - /* - * allocate memory to save all lapic_id detected in parse_mdt. - * We allocate 4K size which could save 4K CPUs lapic_id info. - */ - lapic_id_base = alloc_page(); - ASSERT(lapic_id_base != NULL, "fail to alloc page"); + /* Save all lapic_id detected via parse_mdt in lapic_id_array */ + pcpu_num = parse_madt(lapic_id_array); + if (pcpu_num == 0U) { + /* failed to get the physcial cpu number */ + ASSERT(false); + } - pcpu_num = parse_madt(lapic_id_base); alloc_phy_cpu_data(pcpu_num); for (i = 0U; i < pcpu_num; i++) { - per_cpu(lapic_id, i) = *lapic_id_base; - lapic_id_base++; + per_cpu(lapic_id, i) = lapic_id_array[i]; } - /* free memory after lapic_id are saved in per_cpu data */ - free((void *)lapic_id_base); - bsp_lapic_id = get_cur_lapic_id(); bsp_cpu_id = get_cpu_id_from_lapic_id(bsp_lapic_id); @@ -458,7 +452,7 @@ void bsp_boot_init(void) early_init_lapic(); - init_phy_cpu_storage(); + init_percpu_data_area(); load_gdtr_and_tr(); diff --git a/hypervisor/boot/acpi.c b/hypervisor/boot/acpi.c index c91052d80..5a2dc62a5 100644 --- a/hypervisor/boot/acpi.c +++ b/hypervisor/boot/acpi.c @@ -221,7 +221,7 @@ void *get_acpi_tbl(const char *sig) return HPA2HVA(addr); } -static uint16_t _parse_madt(void *madt, uint8_t *lapic_id_base) +static uint16_t _parse_madt(void *madt, uint8_t lapic_id_array[MAX_PCPU_NUM]) { uint16_t pcpu_id = 0; struct acpi_madt_local_apic *processor; @@ -243,9 +243,16 @@ static uint16_t _parse_madt(void *madt, uint8_t *lapic_id_base) if (entry->type == ACPI_MADT_TYPE_LOCAL_APIC) { processor = (struct acpi_madt_local_apic *)entry; if ((processor->lapic_flags & ACPI_MADT_ENABLED) != 0U) { - *lapic_id_base = processor->id; - lapic_id_base++; + lapic_id_array[pcpu_id] = processor->id; pcpu_id++; + /* + * set the pcpu_num as 0U to indicate the + * potential overflow + */ + if (pcpu_id >= MAX_PCPU_NUM) { + pcpu_id = 0U; + break; + } } } @@ -256,8 +263,8 @@ static uint16_t _parse_madt(void *madt, uint8_t *lapic_id_base) return pcpu_id; } -/* The lapic_id info gotten from madt will be returned in lapic_id_base */ -uint16_t parse_madt(uint8_t *lapic_id_base) +/* The lapic_id info gotten from madt will be returned in lapic_id_array */ +uint16_t parse_madt(uint8_t lapic_id_array[MAX_PCPU_NUM]) { void *madt; @@ -267,7 +274,7 @@ uint16_t parse_madt(uint8_t *lapic_id_base) madt = get_acpi_tbl(ACPI_SIG_MADT); ASSERT(madt != NULL, "fail to get madt"); - return _parse_madt(madt, lapic_id_base); + return _parse_madt(madt, lapic_id_array); } void *get_dmar_table(void) diff --git a/hypervisor/include/arch/x86/cpu.h b/hypervisor/include/arch/x86/cpu.h index 032e6ca80..142452d13 100644 --- a/hypervisor/include/arch/x86/cpu.h +++ b/hypervisor/include/arch/x86/cpu.h @@ -43,6 +43,9 @@ #define CPU_PAGE_SIZE 0x1000U #define CPU_PAGE_MASK 0xFFFFFFFFFFFFF000UL +/* Assume the max physcial cpu number is 128 */ +#define MAX_PCPU_NUM 128U + #define MMU_PTE_PAGE_SHIFT CPU_PAGE_SHIFT #define MMU_PDE_PAGE_SHIFT 21