[PATCH] abort when core.img is too big

[PATCH] abort when core.img is too big

[Robert Millan]

[-- Attachment #1: Type: text/plain, Size: 520 bytes --]


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?

-- 
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 /.)

[-- Attachment #2: size_limit.diff --]
[-- Type: text/x-diff, Size: 7093 bytes --]


	* 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.


diff -x CVS -x '*~' -x '*.mk' -urp ../grub2/conf/i386-pc.rmk ./conf/i386-pc.rmk
--- ../grub2/conf/i386-pc.rmk	2008-01-21 00:41:58.000000000 +0100
+++ ./conf/i386-pc.rmk	2008-01-21 14:21:43.000000000 +0100
@@ -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.
diff -x CVS -x '*~' -x '*.mk' -urp ../grub2/include/grub/i386/pc/memory.h ./include/grub/i386/pc/memory.h
--- ../grub2/include/grub/i386/pc/memory.h	2008-01-21 00:41:58.000000000 +0100
+++ ./include/grub/i386/pc/memory.h	2008-01-21 15:22:02.000000000 +0100
@@ -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 \
diff -x CVS -x '*~' -x '*.mk' -urp ../grub2/include/grub/i386/pc/vga.h ./include/grub/i386/pc/vga.h
--- ../grub2/include/grub/i386/pc/vga.h	2007-07-22 01:32:24.000000000 +0200
+++ ./include/grub/i386/pc/vga.h	2008-01-21 15:22:58.000000000 +0100
@@ -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);
diff -x CVS -x '*~' -x '*.mk' -urp ../grub2/term/i386/pc/vga.c ./term/i386/pc/vga.c
--- ../grub2/term/i386/pc/vga.c	2007-12-25 12:10:47.000000000 +0100
+++ ./term/i386/pc/vga.c	2008-01-21 15:23:38.000000000 +0100
@@ -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
diff -x CVS -x '*~' -x '*.mk' -urp ../grub2/util/i386/pc/grub-mkimage.c ./util/i386/pc/grub-mkimage.c
--- ../grub2/util/i386/pc/grub-mkimage.c	2008-01-21 00:41:59.000000000 +0100
+++ ./util/i386/pc/grub-mkimage.c	2008-01-21 15:23:56.000000000 +0100
@@ -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);
diff -x CVS -x '*~' -x '*.mk' -urp ../grub2/video/i386/pc/vbe.c ./video/i386/pc/vbe.c
--- ../grub2/video/i386/pc/vbe.c	2008-01-01 13:02:06.000000000 +0100
+++ ./video/i386/pc/vbe.c	2008-01-21 14:22:52.000000000 +0100
@@ -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

Re: [PATCH] abort when core.img is too big

[Marco Gerards]

Robert Millan <rmh@aybabtu.com> writes:

> 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

Re: [PATCH] abort when core.img is too big

[Robert Millan]

On Mon, Jan 21, 2008 at 04:05:04PM +0100, Marco Gerards wrote:
> Robert Millan <rmh@aybabtu.com> writes:
> 
> > 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.

Ok, committed.

-- 
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 /.)