[ubsan] fix some bugs and warnings discovered by ubsan

- X86 cpuid feature list dump was using the wrong array and walking off
  the end of one.
- GICv2 code had a left shift by up to 31 of an integer. Needs to be
  unsigned.
- PLIC same as GIC code.
- fdtwalker code should be using a bytewise accessor based helper
  function for reading large integers out of an unaliged FDT.
- PCI BIOS32 search code could do a 32bit unaligned read of a string,
  switch to using memcmp.
This commit is contained in:
Travis Geiselbrecht
2025-10-05 15:29:09 -07:00
parent 23cbdcc971
commit 664bb17afa
6 changed files with 24 additions and 23 deletions

View File

@@ -229,7 +229,7 @@ static void x86_feature_dump_cpuid(void) {
for (uint32_t i = X86_CPUID_EXT_BASE; i <= max_cpuid_leaf_ext; i++) { for (uint32_t i = X86_CPUID_EXT_BASE; i <= max_cpuid_leaf_ext; i++) {
uint32_t index = i - X86_CPUID_EXT_BASE; uint32_t index = i - X86_CPUID_EXT_BASE;
printf("X86: cpuid leaf %#x: %08x %08x %08x %08x\n", i, printf("X86: cpuid leaf %#x: %08x %08x %08x %08x\n", i,
saved_cpuids[index].a, saved_cpuids[index].b, saved_cpuids[index].c, saved_cpuids[index].d); saved_cpuids_ext[index].a, saved_cpuids_ext[index].b, saved_cpuids_ext[index].c, saved_cpuids_ext[index].d);
} }
} }

View File

@@ -105,7 +105,8 @@ enum x86_cpuid_leaf_num {
X86_CPUID_BRAND = 0x80000002, X86_CPUID_BRAND = 0x80000002,
X86_CPUID_ADDR_WIDTH = 0x80000008, X86_CPUID_ADDR_WIDTH = 0x80000008,
X86_CPUID_AMD_TOPOLOGY = 0x8000001e, X86_CPUID_AMD_TOPOLOGY = 0x8000001e,
__X86_MAX_SUPPORTED_CPUID_EXT = X86_CPUID_AMD_TOPOLOGY, X86_CPUID_AMD_EXTENDED_TOPOLOGY = 0x80000025,
__X86_MAX_SUPPORTED_CPUID_EXT = X86_CPUID_AMD_EXTENDED_TOPOLOGY,
}; };
struct x86_cpuid_bit { struct x86_cpuid_bit {

View File

@@ -82,7 +82,7 @@ static pci_bios_info *find_pci_bios_info(void) {
uint i; uint i;
while (head < (uint32_t *) (0x000ffff0 + KERNEL_BASE)) { while (head < (uint32_t *) (0x000ffff0 + KERNEL_BASE)) {
if (*head == *(uint32_t *) pci_bios_magic) { if (memcmp(head, pci_bios_magic, sizeof(pci_bios_magic)) == 0) {
// perform the checksum // perform the checksum
sum = 0; sum = 0;
b = (int8_t *) head; b = (int8_t *) head;

View File

@@ -244,9 +244,9 @@ static status_t gic_configure_interrupt(unsigned int vector,
uint32_t bit_shift = ((vector & 0xf) << 1) + 1; uint32_t bit_shift = ((vector & 0xf) << 1) + 1;
uint32_t reg_val = gicreg_read32(0, GICD_ICFGR(reg_ndx)); uint32_t reg_val = gicreg_read32(0, GICD_ICFGR(reg_ndx));
if (tm == IRQ_TRIGGER_MODE_EDGE) { if (tm == IRQ_TRIGGER_MODE_EDGE) {
reg_val |= (1 << bit_shift); reg_val |= (1U << bit_shift);
} else { } else {
reg_val &= ~(1 << bit_shift); reg_val &= ~(1U << bit_shift);
} }
gicreg_write32(0, GICD_ICFGR(reg_ndx), reg_val); gicreg_write32(0, GICD_ICFGR(reg_ndx), reg_val);

View File

@@ -95,7 +95,7 @@ void plic_early_init(uintptr_t base, size_t num_irqs_, bool hart0_m_only_) {
// mask all irqs and set their priority to 1 // mask all irqs and set their priority to 1
// TODO: mask on all the other cpus too // TODO: mask on all the other cpus too
for (size_t i = 1; i < num_irqs; i++) { for (size_t i = 1; i < num_irqs; i++) {
*REG32(PLIC_ENABLE(i, riscv_current_hart())) &= ~(1 << (i % 32)); *REG32(PLIC_ENABLE(i, riscv_current_hart())) &= ~(1U << (i % 32));
*REG32(PLIC_PRIORITY(i)) = 1; *REG32(PLIC_PRIORITY(i)) = 1;
} }
@@ -107,13 +107,13 @@ void plic_init(void) {}
status_t mask_interrupt(unsigned int vector) { status_t mask_interrupt(unsigned int vector) {
LTRACEF("vector %u, current hart %u\n", vector, riscv_current_hart()); LTRACEF("vector %u, current hart %u\n", vector, riscv_current_hart());
*REG32(PLIC_ENABLE(vector, riscv_current_hart())) &= ~(1 << (vector % 32)); *REG32(PLIC_ENABLE(vector, riscv_current_hart())) &= ~(1U << (vector % 32));
return NO_ERROR; return NO_ERROR;
} }
status_t unmask_interrupt(unsigned int vector) { status_t unmask_interrupt(unsigned int vector) {
LTRACEF("vector %u, current hart %u\n", vector, riscv_current_hart()); LTRACEF("vector %u, current hart %u\n", vector, riscv_current_hart());
*REG32(PLIC_ENABLE(vector, riscv_current_hart())) |= (1 << (vector % 32)); *REG32(PLIC_ENABLE(vector, riscv_current_hart())) |= (1U << (vector % 32));
return NO_ERROR; return NO_ERROR;
} }

View File

@@ -35,13 +35,13 @@ void read_address_size_cells(const void *fdt, int offset, int depth,
const void *prop_ptr = fdt_getprop(fdt, offset, "#address-cells", &len); const void *prop_ptr = fdt_getprop(fdt, offset, "#address-cells", &len);
LTRACEF_LEVEL(3, "%p, len %d\n", prop_ptr, len); LTRACEF_LEVEL(3, "%p, len %d\n", prop_ptr, len);
if (prop_ptr && len == 4) { if (prop_ptr && len == 4) {
address_cells[depth] = fdt32_to_cpu(*(const uint32_t *)prop_ptr); address_cells[depth] = fdt32_ld((const fdt32_t *)prop_ptr);
} }
prop_ptr = fdt_getprop(fdt, offset, "#size-cells", &len); prop_ptr = fdt_getprop(fdt, offset, "#size-cells", &len);
LTRACEF_LEVEL(3, "%p, len %d\n", prop_ptr, len); LTRACEF_LEVEL(3, "%p, len %d\n", prop_ptr, len);
if (prop_ptr && len == 4) { if (prop_ptr && len == 4) {
size_cells[depth] = fdt32_to_cpu(*(const uint32_t *)prop_ptr); size_cells[depth] = fdt32_ld((const fdt32_t *)prop_ptr);
} }
LTRACEF_LEVEL(3, "address-cells %u size-cells %u\n", address_cells[depth], size_cells[depth]); LTRACEF_LEVEL(3, "address-cells %u size-cells %u\n", address_cells[depth], size_cells[depth]);
@@ -55,11 +55,11 @@ status_t read_base_len_pair(const uint8_t *prop_ptr, size_t prop_len,
/* we're looking at a memory descriptor */ /* we're looking at a memory descriptor */
if (address_cell_size == 2 && prop_len >= 8) { if (address_cell_size == 2 && prop_len >= 8) {
*base = fdt64_to_cpu(*(const uint64_t *)prop_ptr); *base = fdt64_ld((const fdt64_t *)prop_ptr);
prop_ptr += 8; prop_ptr += 8;
prop_len -= 8; prop_len -= 8;
} else if (address_cell_size == 1 && prop_len >= 4) { } else if (address_cell_size == 1 && prop_len >= 4) {
*base = fdt32_to_cpu(*(const uint32_t *)prop_ptr); *base = fdt32_ld((const fdt32_t *)prop_ptr);
prop_ptr += 4; prop_ptr += 4;
prop_len -= 4; prop_len -= 4;
} else { } else {
@@ -67,11 +67,11 @@ status_t read_base_len_pair(const uint8_t *prop_ptr, size_t prop_len,
} }
if (size_cell_size == 2 && prop_len >= 8) { if (size_cell_size == 2 && prop_len >= 8) {
*len = fdt64_to_cpu(*((const uint64_t *)prop_ptr)); *len = fdt64_ld(((const fdt64_t *)prop_ptr));
prop_ptr += 8; prop_ptr += 8;
prop_len -= 8; prop_len -= 8;
} else if (size_cell_size == 1 && prop_len >= 4) { } else if (size_cell_size == 1 && prop_len >= 4) {
*len = fdt32_to_cpu(*(const uint32_t *)prop_ptr); *len = fdt32_ld((const fdt32_t *)prop_ptr);
prop_ptr += 4; prop_ptr += 4;
prop_len -= 4; prop_len -= 4;
} else { } else {
@@ -212,7 +212,7 @@ status_t fdt_walk_find_cpus(const void *fdt, struct fdt_walk_cpu_info *cpu, size
state.curr_address_cell(), state.curr_size_cell()); state.curr_address_cell(), state.curr_size_cell());
uint32_t id = 0; uint32_t id = 0;
if (state.curr_address_cell() == 1 && lenp >= 4) { if (state.curr_address_cell() == 1 && lenp >= 4) {
id = fdt32_to_cpu(*(const uint32_t *)prop_ptr); id = fdt32_ld((const fdt32_t *)prop_ptr);
prop_ptr += 4; prop_ptr += 4;
lenp -= 4; lenp -= 4;
} else { } else {
@@ -358,9 +358,9 @@ status_t fdt_walk_find_pcie_info(const void *fdt, struct fdt_walk_pcie_info *inf
state.curr_address_cell(), state.curr_size_cell()); state.curr_address_cell(), state.curr_size_cell());
/* seems to always be full address cells 2, size cells 2, despite it being 3/2 */ /* seems to always be full address cells 2, size cells 2, despite it being 3/2 */
info[*count].ecam_base = fdt64_to_cpu(*(const uint64_t *)prop_ptr); info[*count].ecam_base = fdt64_ld((const fdt64_t *)prop_ptr);
prop_ptr += 8; prop_ptr += 8;
info[*count].ecam_len = fdt64_to_cpu(*(const uint64_t *)prop_ptr); info[*count].ecam_len = fdt64_ld((const fdt64_t *)prop_ptr);
} }
/* find which bus range the ecam covers */ /* find which bus range the ecam covers */
@@ -371,9 +371,9 @@ status_t fdt_walk_find_pcie_info(const void *fdt, struct fdt_walk_pcie_info *inf
state.curr_address_cell(), state.curr_size_cell()); state.curr_address_cell(), state.curr_size_cell());
if (lenp == 8) { if (lenp == 8) {
info[*count].bus_start = fdt32_to_cpu(*(const uint32_t *)prop_ptr); info[*count].bus_start = fdt32_ld((const fdt32_t *)prop_ptr);
prop_ptr += 4; prop_ptr += 4;
info[*count].bus_end = fdt32_to_cpu(*(const uint32_t *)prop_ptr); info[*count].bus_end = fdt32_ld((const fdt32_t *)prop_ptr);
} }
} }
@@ -386,16 +386,16 @@ status_t fdt_walk_find_pcie_info(const void *fdt, struct fdt_walk_pcie_info *inf
/* iterate this packed property */ /* iterate this packed property */
const uint8_t *prop_end = prop_ptr + lenp; const uint8_t *prop_end = prop_ptr + lenp;
while (prop_ptr < prop_end) { while (prop_ptr < prop_end) {
uint32_t type = fdt32_to_cpu(*(const uint32_t *)(prop_ptr)); uint32_t type = fdt32_ld((const fdt32_t *)prop_ptr);
prop_ptr += 4; prop_ptr += 4;
/* read 3 64bit values */ /* read 3 64bit values */
uint64_t base1, base2, size; uint64_t base1, base2, size;
base1 = fdt64_to_cpu(*(const uint64_t *)prop_ptr); base1 = fdt64_ld((const fdt64_t *)prop_ptr);
prop_ptr += 8; prop_ptr += 8;
base2 = fdt64_to_cpu(*(const uint64_t *)prop_ptr); base2 = fdt64_ld((const fdt64_t *)prop_ptr);
prop_ptr += 8; prop_ptr += 8;
size = fdt64_to_cpu(*(const uint64_t *)prop_ptr); size = fdt64_ld((const fdt64_t *)prop_ptr);
prop_ptr += 8; prop_ptr += 8;
switch (type) { switch (type) {