[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v6 3/3] x86/boot: Improve MBI2 structure check
On Mon, Sep 30, 2024 at 4:55 PM Jan Beulich <jbeulich@xxxxxxxx> wrote: > > On 26.09.2024 11:21, Frediano Ziglio wrote: > > --- a/xen/arch/x86/efi/parse-mbi2.c > > +++ b/xen/arch/x86/efi/parse-mbi2.c > > @@ -13,6 +13,7 @@ efi_multiboot2_prelude(uint32_t magic, const > > multiboot2_fixed_t *mbi) > > EFI_HANDLE ImageHandle = NULL; > > EFI_SYSTEM_TABLE *SystemTable = NULL; > > const char *cmdline = NULL; > > + const void *const mbi_end = (const void *)mbi + mbi->total_size; > > bool have_bs = false; > > > > if ( magic != MULTIBOOT2_BOOTLOADER_MAGIC ) > > @@ -21,7 +22,9 @@ efi_multiboot2_prelude(uint32_t magic, const > > multiboot2_fixed_t *mbi) > > /* Skip Multiboot2 information fixed part. */ > > tag = _p(ROUNDUP((unsigned long)(mbi + 1), MULTIBOOT2_TAG_ALIGN)); > > > > - for ( ; (const void *)tag - (const void *)mbi < mbi->total_size && > > + for ( ; (const void *)(tag + 1) <= mbi_end && > > It may be possible to argue away overflow concerns here. I'm not so sure > though > for the case ... > Do you mean tag + 1 ? For the same reason, computing tag above could have an overflow. If the caller pass invalid data range, we will have overflows in either case. > > + tag->size >= sizeof(*tag) && > > + (const void *)tag + tag->size <= mbi_end && > > ... here. The earlier logic subtracting pointers wasn't susceptible to such. > > Jan Can you suggest a change ? Frediano
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |