diff mbox series

[v9,2/2] x86/boot: Improve MBI2 structure check

Message ID 20241010094524.1836495-3-frediano.ziglio@cloud.com (mailing list archive)
State Superseded
Headers show
Series x86/boot: Improve MBI2 structure check (was: Reduce assembly code) | expand

Commit Message

Frediano Ziglio Oct. 10, 2024, 9:45 a.m. UTC 
Tag structure should contain at least the tag header.
Entire tag structure must be contained inside MBI2 data.

Reviewed-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>
---
Changes since v6:
- compare against total_size every time to avoid overflows.

Changes since v7:
- check for tag type being MULTIBOOT2_TAG_TYPE_END before checking
  tag size. This allows caller to not fill full tag (type field is
  before size field).
---
 xen/arch/x86/efi/mbi2.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Jan Beulich Oct. 15, 2024, 6:42 a.m. UTC | #1 
On 10.10.2024 11:45, Frediano Ziglio wrote:
> Tag structure should contain at least the tag header.
> Entire tag structure must be contained inside MBI2 data.
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Frediano Ziglio <frediano.ziglio@cloud.com>

Nit (for the future): Tags in chronological order please (maybe
with the exception of release acks, which personally I think best
go last).

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/efi/mbi2.c b/xen/arch/x86/efi/mbi2.c
index 55a1777483..d2f0f0ee61 100644
--- a/xen/arch/x86/efi/mbi2.c
+++ b/xen/arch/x86/efi/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_raw = (const void *)mbi;
     bool have_bs = false;
 
     if ( magic != MULTIBOOT2_BOOTLOADER_MAGIC )
@@ -21,8 +22,10 @@  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 &&
-            tag->type != MULTIBOOT2_TAG_TYPE_END;
+    for ( ; (const void *)(tag + 1) - mbi_raw <= mbi->total_size &&
+            tag->type != MULTIBOOT2_TAG_TYPE_END &&
+            tag->size >= sizeof(*tag) &&
+            (const void *)tag + tag->size - mbi_raw <= mbi->total_size;
           tag = _p(ROUNDUP((unsigned long)tag + tag->size,
                    MULTIBOOT2_TAG_ALIGN)) )
     {