Hi,
On amd64 EFI machines, the kernel is potentially booted with an
obsolete memory map.
The file is "src/sys/arch/amd64/stand/libsa/exec_i386.c"
The memory map is passed to the kernel:
/* Pass memory map to the kernel */
mem_pass();
and then later, efi_cleanup() is called:
/*
* Move the loaded kernel image to the usual place after calling
* ExitBootServices().
*/
delta = DEFAULT_KERNEL_ADDRESS - efi_loadaddr;
efi_cleanup();
For reference, here is efi_cleanup() from
"src/sys/arch/amd64/stand/efiboot/efiboot.c":
void
efi_cleanup(void)
{
int retry;
EFI_STATUS status;
/* retry once in case of failure */
for (retry = 1; retry >= 0; retry--) {
efi_memprobe_internal(); /* sync the current map */
status = EFI_CALL(BS->ExitBootServices, IH, mmap_key);
if (status == EFI_SUCCESS)
break;
if (retry == 0)
panic("ExitBootServices failed (%d)", status);
}
}
When ExitBootServices is invoked, it has the potential to change
the memory map. If this happens, it must be invoked a 2nd time.
This is normal behaviour according to the EFI specification, so
there is nothing wrong with efi_cleanup() itself.
The bug is that the potentially updated memory map is never
passed to the kernel in "exec_i386.c".
My understanding is that if the kernel boots with an obsolete
memory map, it would then have the potential to corrupt EFI
data in memory, with unpredictable and possibly catastrophic
results.
The solution would be to ensure that the memory map is passed
to the kernel *after* efi_cleanup() in "exec_i386.c".
This diff is my attempt to resolve the problem (and clean up
some redundancy in the ifdef branching). I'd like to submit it here
for review. I'm running 6.2 -stable, but this diff is against the most
current version of the file at the time of writing.
diff -ur a/sys/arch/amd64/stand/libsa/exec_i386.c
b/sys/arch/amd64/stand/libsa/exec_i386.c
--- a/sys/arch/amd64/stand/libsa/exec_i386.c Sun Feb 11 22:37:16 2018
+++ b/sys/arch/amd64/stand/libsa/exec_i386.c Sun Feb 11 22:40:23 2018
@@ -78,7 +78,7 @@
bios_bootsr_t bootsr;
struct sr_boot_volume *bv;
#endif
-#if defined(EFIBOOT)
+#ifdef EFIBOOT
int i;
u_long delta;
extern u_long efi_loadaddr;
@@ -86,6 +86,7 @@
if ((av = alloc(ac)) == NULL)
panic("alloc for bootarg");
efi_makebootargs();
+ delta = DEFAULT_KERNEL_ADDRESS - efi_loadaddr;
#endif
if (sa_cleanup != NULL)
(*sa_cleanup)();
@@ -124,6 +125,17 @@
sr_clear_keys();
#endif
+ entry = marks[MARK_ENTRY] & 0x0fffffff;
+#ifdef EFIBOOT
+ entry += delta;
+#endif
+
+ printf("entry point at 0x%lx\n", entry);
+
+#ifdef EFIBOOT
+ /* Sync the memory map and call ExitBootServices() */
+ efi_cleanup();
+#endif
/* Pass memory map to the kernel */
mem_pass();
@@ -137,33 +149,24 @@
makebootargs(av, &ac);
#endif
- entry = marks[MARK_ENTRY] & 0x0fffffff;
-
- printf("entry point at 0x%lx\n", entry);
-
-#ifndef EFIBOOT
- /* stack and the gung is ok at this point, so, no need for asm setup */
- (*(startfuncp)entry)(howto, bootdev, BOOTARG_APIVER, marks[MARK_END],
- extmem, cnvmem, ac, (int)av);
-#else
+#ifdef EFIBOOT
/*
* Move the loaded kernel image to the usual place after calling
* ExitBootServices().
*/
- delta = DEFAULT_KERNEL_ADDRESS - efi_loadaddr;
- efi_cleanup();
memcpy((void *)marks[MARK_START] + delta, (void *)marks[MARK_START],
marks[MARK_END] - marks[MARK_START]);
for (i = 0; i < MARK_MAX; i++)
marks[i] += delta;
- entry += delta;
+#endif
+
#ifdef __amd64__
(*run_i386)((u_long)run_i386, entry, howto, bootdev, BOOTARG_APIVER,
marks[MARK_END], extmem, cnvmem, ac, (intptr_t)av);
#else
+ /* stack and the gung is ok at this point, so, no need for asm setup */
(*(startfuncp)entry)(howto, bootdev, BOOTARG_APIVER, marks[MARK_END],
extmem, cnvmem, ac, (int)av);
-#endif
#endif
/* not reached */
}
