mirror of
https://github.com/xen-project/xen.git
synced 2026-10-28 02:01:24 +01:00
x86/boot: Fix PVH boot during boot_info transition period
multiboot_fill_boot_info() taking the physical address of the multiboot_info
structure leads to a subtle use-after-free on the PVH path, with rather less
subtle fallout.
The pointers used by __start_xen(), mbi and mod, are either:
- MB: Directmap pointers into the trampoline, or
- PVH: Xen pointers into .initdata, or
- EFI: Directmap pointers into Xen.
Critically, these either remain valid across move_xen() (MB, PVH), or rely on
move_xen() being inhibited (EFI).
The conversion to multiboot_fill_boot_info(), taking only mbi_p, makes the PVH
path use directmap pointers into Xen, as well as move_xen() which invalidates
said pointers.
Switch multiboot_fill_boot_info() to consume the same virtual addresses that
__start_xen() currently uses. This keeps all the pointers valid for the
duration of __start_xen(), for all boot protocols.
It can be safely untangled once multiboot_fill_boot_info() takes a full copy
the multiboot info data, and __start_xen() has been moved over to using the
new boot_info consistently.
Right now, bi->{loader,cmdline,mods} are problematic. Nothing uses
bi->mods[], and nothing uses bi->cmdline after move_xen().
bi->loader is used after move_xen(), although only for cmdline_cook() of
dom0's cmdline, where it happens to be benign because PVH boot skips the
inspection of the bootloader name.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
This commit is contained in:
1 parent
b837d02163
commit
0fe607b2a1
1 file changed
+20
-5
+20
-5
@@ -283,11 +283,10 @@ struct boot_info __initdata xen_boot_info = {
|
||||
.cmdline = "",
|
||||
};
|
||||
|
||||
static struct boot_info *__init multiboot_fill_boot_info(unsigned long mbi_p)
|
||||
static struct boot_info *__init multiboot_fill_boot_info(
|
||||
const multiboot_info_t *mbi, module_t *mods)
|
||||
{
|
||||
struct boot_info *bi = &xen_boot_info;
|
||||
const multiboot_info_t *mbi = __va(mbi_p);
|
||||
module_t *mods = __va(mbi->mods_addr);
|
||||
unsigned int i;
|
||||
|
||||
if ( mbi->flags & MBI_MODULES )
|
||||
@@ -1065,15 +1064,31 @@ void asmlinkage __init noreturn __start_xen(unsigned long mbi_p)
|
||||
{
|
||||
ASSERT(mbi_p == 0);
|
||||
pvh_init(&mbi, &mod);
|
||||
mbi_p = __pa(mbi);
|
||||
/*
|
||||
* mbi and mod are regular pointers to .initdata. These remain valid
|
||||
* across move_xen().
|
||||
*/
|
||||
}
|
||||
else
|
||||
{
|
||||
mbi = __va(mbi_p);
|
||||
mod = __va(mbi->mods_addr);
|
||||
|
||||
/*
|
||||
* For MB1/2, mbi and mod are directmap pointers into the trampoline.
|
||||
* These remain valid across move_xen().
|
||||
*
|
||||
* For EFI, these are directmap pointers into the Xen image. They do
|
||||
* not remain valid across move_xen(). EFI boot only functions
|
||||
* because a non-zero xen_phys_start inhibits move_xen().
|
||||
*
|
||||
* Don't be fooled by efi_arch_post_exit_boot() passing "D" (&mbi).
|
||||
* This is a EFI physical-mode (i.e. identity map) pointer.
|
||||
*/
|
||||
ASSERT(mbi_p < MB(1) || xen_phys_start);
|
||||
}
|
||||
|
||||
bi = multiboot_fill_boot_info(mbi_p);
|
||||
bi = multiboot_fill_boot_info(mbi, mod);
|
||||
|
||||
/* Parse the command-line options. */
|
||||
if ( (kextra = strstr(bi->cmdline, " -- ")) != NULL )
|
||||
|
||||