abort when core.img is too big

PATCH In review Robert Millan 2 participants 3 messages Jan 21, 2008

Share & tools

Robert Millan

author

2008-01-21 14:46 UTC #0 lore raw

PATCH 1/1 abort when core.img is too big

With the addition of memdisk, it is too easy for user to create an image big enough that would corrupt upper memory (starting with vga region). This happens at roughly ~600 kiB memdisk size.

This patch adds a check in grub-mkimage to prevent this. I did also take the opportunity to reuse the upper memory macro to avoid hardcoding its address for VGA stuff.

Comments?

Signature · 5 lines

Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)

* conf/i386-pc.rmk (GRUB_MEMORY_MACHINE_LINK_ADDR): New variable. (kernel_img_LDFLAGS): Use `GRUB_MEMORY_MACHINE_LINK_ADDR' as link address.
(grub_mkimage_CFLAGS): Propagate `GRUB_MEMORY_MACHINE_LINK_ADDR' as a C macro.
* include/grub/i386/pc/memory.h (GRUB_MEMORY_MACHINE_UPPER): New macro. Indicates start of upper memory.
* util/i386/pc/grub-mkimage.c: Include `<grub/machine/memory.h>'. (generate_image): Abort when image size is big enough to corrupt upper memory.

* include/grub/i386/pc/vga.h: Include <grub/machine/memory.h>'. (GRUB_MEMORY_MACHINE_VGA_ADDR): Alias for GRUB_MEMORY_MACHINE_UPPER'.
* term/i386/pc/vga.c (VGA_MEM): Use `GRUB_MEMORY_MACHINE_VGA_ADDR' instead of hardcoding 0xA0000.
* video/i386/pc/vbe.c: Include <grub/machine/vga.h>'. (grub_vbe_set_video_mode): Use GRUB_MEMORY_MACHINE_VGA_ADDR' instead of hardcoding 0xA0000.

conf/i386-pc.rmk renamed from grub2/conf/i386-pc.rmk +4 -1

diff -x CVS -x '*~' -x '*.mk' -urp ../grub2/conf/i386-pc.rmk ./conf/i386-pc.rmk
@@ -1,5 +1,7 @@
 # -*- makefile -*-
 
+GRUB_MEMORY_MACHINE_LINK_ADDR = 0x8200
+
 COMMON_ASFLAGS = -nostdinc -fno-builtin -m32
 COMMON_CFLAGS = -fno-builtin -mrtd -mregparm=3 -m32
 COMMON_LDFLAGS = -m32 -nostdlib

@@ -42,7 +44,7 @@ kernel_img_HEADERS = arg.h boot.h cache.
 	machine/memory.h machine/loader.h machine/vga.h machine/vbe.h machine/kernel.h
 kernel_img_CFLAGS = $(COMMON_CFLAGS)
 kernel_img_ASFLAGS = $(COMMON_ASFLAGS)
-kernel_img_LDFLAGS = $(COMMON_LDFLAGS) -Wl,-N,-Ttext,8200 $(COMMON_CFLAGS)
+kernel_img_LDFLAGS = $(COMMON_LDFLAGS) -Wl,-N,-Ttext,$(GRUB_MEMORY_MACHINE_LINK_ADDR) $(COMMON_CFLAGS)
 
 MOSTLYCLEANFILES += symlist.c kernel_syms.lst
 DEFSYMFILES += kernel_syms.lst

@@ -63,6 +65,7 @@ endif
 # For grub-mkimage.
 grub_mkimage_SOURCES = util/i386/pc/grub-mkimage.c util/misc.c \
 	util/resolve.c
+grub_mkimage_CFLAGS = -DGRUB_MEMORY_MACHINE_LINK_ADDR=$(GRUB_MEMORY_MACHINE_LINK_ADDR)
 grub_mkimage_LDFLAGS = $(LIBLZO)
 
 # For grub-setup.

include/grub/i386/pc/memory.h renamed from grub2/include/grub/i386/pc/memory.h +4 -1

diff -x CVS -x '*~' -x '*.mk' -urp ../grub2/include/grub/i386/pc/memory.h ./include/grub/i386/pc/memory.h
@@ -1,7 +1,7 @@
 /* memory.h - describe the memory map */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002,2007  Free Software Foundation, Inc.
+ *  Copyright (C) 2002,2007,2008  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by

@@ -36,6 +36,9 @@
 /* The size of the protect mode stack.  */
 #define GRUB_MEMORY_MACHINE_PROT_STACK_SIZE	0x8000
 
+/* The upper memory area (starting at 640 kiB).  */
+#define GRUB_MEMORY_MACHINE_UPPER		0xa0000
+
 /* The protected mode stack.  */
 #define GRUB_MEMORY_MACHINE_PROT_STACK	\
 	(GRUB_MEMORY_MACHINE_SCRATCH_ADDR + GRUB_MEMORY_MACHINE_SCRATCH_SIZE \

include/grub/i386/pc/vga.h renamed from grub2/include/grub/i386/pc/vga.h +5 -1

diff -x CVS -x '*~' -x '*.mk' -urp ../grub2/include/grub/i386/pc/vga.h ./include/grub/i386/pc/vga.h
@@ -1,6 +1,6 @@
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2003,2007  Free Software Foundation, Inc.
+ *  Copyright (C) 2003,2007,2008  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by

@@ -20,6 +20,10 @@
 #define GRUB_VGA_MACHINE_HEADER	1
 
 #include <grub/symbol.h>
+#include <grub/machine/memory.h>
+
+/* The VGA (at the beginning of upper memory).  */
+#define GRUB_MEMORY_MACHINE_VGA_ADDR		GRUB_MEMORY_MACHINE_UPPER
 
 /* Set the video mode to MODE and return the previous mode.  */
 unsigned char EXPORT_FUNC(grub_vga_set_mode) (unsigned char mode);

term/i386/pc/vga.c renamed from grub2/term/i386/pc/vga.c +2 -2

diff -x CVS -x '*~' -x '*.mk' -urp ../grub2/term/i386/pc/vga.c ./term/i386/pc/vga.c
@@ -1,6 +1,6 @@
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2000,2001,2002,2003,2004,2005,2007  Free Software Foundation, Inc.
+ *  Copyright (C) 2000,2001,2002,2003,2004,2005,2007,2008  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by

@@ -35,7 +35,7 @@
 #define CHAR_HEIGHT	16
 #define TEXT_WIDTH	(VGA_WIDTH / CHAR_WIDTH)
 #define TEXT_HEIGHT	(VGA_HEIGHT / CHAR_HEIGHT)
-#define VGA_MEM		((unsigned char *) 0xA0000)
+#define VGA_MEM		((grub_uint8_t *) GRUB_MEMORY_MACHINE_VGA_ADDR)
 #define PAGE_OFFSET(x)	((x) * (VGA_WIDTH * VGA_HEIGHT / 8))
 
 #define DEFAULT_FG_COLOR	0xa

util/i386/pc/grub-mkimage.c renamed from grub2/util/i386/pc/grub-mkimage.c +6 -1

diff -x CVS -x '*~' -x '*.mk' -urp ../grub2/util/i386/pc/grub-mkimage.c ./util/i386/pc/grub-mkimage.c
@@ -1,7 +1,7 @@
 /* grub-mkimage.c - make a bootable image */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002,2003,2004,2005,2006,2007  Free Software Foundation, Inc.
+ *  Copyright (C) 2002,2003,2004,2005,2006,2007,2008  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by

@@ -21,6 +21,7 @@
 #include <grub/types.h>
 #include <grub/machine/boot.h>
 #include <grub/machine/kernel.h>
+#include <grub/machine/memory.h>
 #include <grub/kernel.h>
 #include <grub/disk.h>
 #include <grub/util/misc.h>

@@ -180,6 +181,10 @@ generate_image (const char *dir, char *p
     = grub_cpu_to_le32 (memdisk_size);
   *((grub_uint32_t *) (core_img + GRUB_KERNEL_MACHINE_COMPRESSED_SIZE))
     = grub_cpu_to_le32 (core_size - GRUB_KERNEL_MACHINE_RAW_SIZE);
+
+  if (core_size > GRUB_MEMORY_MACHINE_UPPER - GRUB_MEMORY_MACHINE_LINK_ADDR)
+    grub_util_error ("Core image is too big (%p > %p)\n", core_size,
+		     GRUB_MEMORY_MACHINE_UPPER - GRUB_MEMORY_MACHINE_LINK_ADDR);
   
   grub_util_write_image (core_img, core_size, out);
   free (kernel_img);

video/i386/pc/vbe.c renamed from grub2/video/i386/pc/vbe.c +2 -1

diff -x CVS -x '*~' -x '*.mk' -urp ../grub2/video/i386/pc/vbe.c ./video/i386/pc/vbe.c
@@ -18,6 +18,7 @@
 
 #include <grub/err.h>
 #include <grub/machine/memory.h>
+#include <grub/machine/vga.h>
 #include <grub/machine/vbe.h>
 #include <grub/machine/vbeblit.h>
 #include <grub/machine/vbefill.h>

@@ -193,7 +194,7 @@ grub_vbe_set_video_mode (grub_uint32_t m
   if (mode < 0x100)
     {
       /* If this is not a VESA mode, guess address.  */
-      framebuffer.ptr = (grub_uint8_t *) 0xA0000;
+      framebuffer.ptr = (grub_uint8_t *) GRUB_MEMORY_MACHINE_VGA_ADDR;
       framebuffer.index_color_mode = 1;
     }
   else

Marco Gerards

2008-01-21 15:03 UTC #1 lore raw

Quoted 9 lines · Robert Millan

With the addition of memdisk, it is too easy for user to create an image big enough that would corrupt upper memory (starting with vga region). This happens at roughly ~600 kiB memdisk size.
This patch adds a check in grub-mkimage to prevent this. I did also take the opportunity to reuse the upper memory macro to avoid hardcoding its address for VGA stuff.
Comments?

This makes sense to me.

--
Marco

Robert Millan

author

2008-01-21 15:51 UTC #2 lore raw

Quoted 6 lines · Marco Gerards

Quoted 4 lines · Robert Millan
With the addition of memdisk, it is too easy for user to create an image big enough that would corrupt upper memory (starting with vga region). This happens at roughly ~600 kiB memdisk size.

Quoted 7 lines

Quoted 5 lines
This patch adds a check in grub-mkimage to prevent this. I did also take the opportunity to reuse the upper memory macro to avoid hardcoding its address for VGA stuff.
Comments?
This makes sense to me.

Ok, committed.

Signature · 5 lines

Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call… if you are unable to speak?
(as seen on /.)