[xen master] x86/boot: Fix PVH boot during boot_info transition period

[patchbot@xxxxxxx]

commit 0fe607b2a1440191b59cc6da83a3e717bf3ff7c0
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Tue Oct 22 11:15:26 2024 +0100
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Tue Oct 22 16:16:43 2024 +0100

    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@xxxxxxxxxx>
    Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
 xen/arch/x86/setup.c | 25 ++++++++++++++++++++-----
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index db670258d6..20392c2abf 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -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 )
--
generated by git-patchbot for /home/xen/git/xen.git#master