Skip to content

Commit

Permalink
elfloader: fix variable prototype and remove var
Browse files Browse the repository at this point in the history
shadowing.

The variable "dtb_size" is of type "size_t" and is defined in
src/arch-arm/sys_boot.c, line 36.

"size_t" is most certainly NOT the same size as "uint32_t", even on
32-bit architectures. Thus, the declaration in smp_boot.c is incorrect,
since it does not match the definition in sys_boot.c. Why even create
a local declaration - put this in a common header file and you will
see those problems right away. Single point of maintenance!

This may lead to an incorrectly sized memory access, that only happens
to be correct by chance in Little-Endian mode. For ARM in Big-Endian
mode this is a bug and will most likely result in an incorrect DTB
size of zero.

This fixes c573511 ("elfloader: pass DTB from bootloader to seL4 on
ARM").

Moreover, remove the shadowing of global variables by defining local
ones with the same name => Rename the local one in src/common.c.

This could have been detected with "-Wshadow".

Practically speaking our DTBs are always (a lot) smaller than 32-bit.
Thus, continue to pass a 32-bit size to the kernel in order to not
change the API here.

Signed-off-by: Matthias Rosenfelder <[email protected]>
  • Loading branch information
mro-github-12345 authored and Andy Bui committed Feb 25, 2024
1 parent c329bef commit e8a15ec
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 10 deletions.
6 changes: 3 additions & 3 deletions elfloader-tool/src/arch-arm/smp_boot.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ static volatile int non_boot_lock = 0;
void arm_disable_dcaches(void);

extern void const *dtb;
extern uint32_t dtb_size;
extern size_t dtb_size;

/* Entry point for all CPUs other than the initial. */
void non_boot_main(void)
Expand Down Expand Up @@ -69,10 +69,10 @@ void non_boot_main(void)
arm_enable_mmu();
}

/* Jump to the kernel. */
/* Jump to the kernel. Note: Our DTB is smaller than 4 GiB. */
((init_arm_kernel_t)kernel_info.virt_entry)(user_info.phys_region_start,
user_info.phys_region_end, user_info.phys_virt_offset,
user_info.virt_entry, (paddr_t)dtb, dtb_size);
user_info.virt_entry, (paddr_t)dtb, (uint32_t)dtb_size);

printf("AP Kernel returned back to the elf-loader.\n");
abort();
Expand Down
3 changes: 2 additions & 1 deletion elfloader-tool/src/arch-arm/sys_boot.c
Original file line number Diff line number Diff line change
Expand Up @@ -242,12 +242,13 @@ void continue_boot(int was_relocated)
printf("Jumping to kernel-image entry point...\n\n");
}

/* Jump to the kernel. Note: Our DTB is smaller than 4 GiB. */
((init_arm_kernel_t)kernel_info.virt_entry)(user_info.phys_region_start,
user_info.phys_region_end,
user_info.phys_virt_offset,
user_info.virt_entry,
(word_t)dtb,
dtb_size);
(uint32_t)dtb_size);

/* We should never get here. */
printf("ERROR: Kernel returned back to the ELF Loader\n");
Expand Down
12 changes: 6 additions & 6 deletions elfloader-tool/src/common.c
Original file line number Diff line number Diff line change
Expand Up @@ -468,29 +468,29 @@ int load_images(
/* keep it page aligned */
next_phys_addr = dtb_phys_start = ROUND_UP(kernel_phys_end, PAGE_BITS);

size_t dtb_size = fdt_size(dtb);
if (0 == dtb_size) {
size_t dtb_sz = fdt_size(dtb);
if (0 == dtb_sz) {
printf("ERROR: Invalid device tree blob supplied\n");
return -1;
}

/* Make sure this is a sane thing to do */
ret = ensure_phys_range_valid(next_phys_addr,
next_phys_addr + dtb_size);
next_phys_addr + dtb_sz);
if (0 != ret) {
printf("ERROR: Physical address of DTB invalid\n");
return -1;
}

memmove((void *)next_phys_addr, dtb, dtb_size);
next_phys_addr += dtb_size;
memmove((void *)next_phys_addr, dtb, dtb_sz);
next_phys_addr += dtb_sz;
next_phys_addr = ROUND_UP(next_phys_addr, PAGE_BITS);
dtb_phys_end = next_phys_addr;

printf("Loaded DTB from %p.\n", dtb);
printf(" paddr=[%p..%p]\n", dtb_phys_start, dtb_phys_end - 1);
*chosen_dtb = (void *)dtb_phys_start;
*chosen_dtb_size = dtb_size;
*chosen_dtb_size = dtb_sz;
} else {
next_phys_addr = ROUND_UP(kernel_phys_end, PAGE_BITS);
}
Expand Down

0 comments on commit e8a15ec

Please sign in to comment.