[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v6 05/44] x86/boot: introduce struct boot_module
On 10/17/24 17:02, Andrew Cooper wrote: On 17/10/2024 6:02 pm, Daniel P. Smith wrote:diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index f7ea482920ef..d8ee5741740a 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -284,6 +284,8 @@ static struct boot_info *__init multiboot_fill_boot_info(unsigned long mbi_p) { 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;bi->nr_modules = (mbi->flags & MBI_MODULES) ? mbi->mods_count : 0; @@ -302,6 +304,13 @@ static struct boot_info *__init multiboot_fill_boot_info(unsigned long mbi_p)bi->memmap_length = mbi->mmap_length; }+ /*+ * Iterate over all modules, including the extra one which should have been + * reserved for Xen itself + */ + for ( i = 0; i <= bi->nr_modules; i++ ) + bi->mods[i].mod = &mods[i];I'm afraid you've got a bug here. bi->nr_modules is unsanitised from firmware at this point. It's checked/clamped later in __start_xen(), but not before you've potentially scribbled past the end of bi->mod[] in this loop. I think we want to retain the warning from clamping (which needs to be after printk() is set up, so after parsing the cmdline), so to compensate I think you want: i < ARRAY_SIZE(bi->mods) && i <= bi->nr_modules as the loop condition here, and a note to this effect. I'm not sure what I think about passing exactly 64 modules, and this interacting with the Xen slot. Completely agree. I think Alejandro was trying to call that out and I missed his point. Will fix. However, you also want to move part of "x86/boot: convert create_dom0 to use boot info" into this patch. Specifically, the conversion from sizeof(module_map)*8 to MAX_NR_BOOTMODS, or there's also a latent bug that depends Xen being compiled as 64bit (unsigned long vs MAX_NR_BOOTMODS). You are right, we should truncate to MAX_NR_BOOTMODS at this point and not later. v/r, dps
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |