[coreboot] r783 - coreboot-v3/arch/x86

[Carl-Daniel Hailfinger]

On 18.08.2008 15:56, Stefan Reinauer wrote:
> Carl-Daniel Hailfinger asked:
>   
>>  
>> -	/* we can't statically init this hack. */
>> +	/* Why can't we statically init this hack? */
>>   
>>     
> No global variables in stage 1.
>   

Hm. The variable is only used locally. We could at least use automatic
init for it.

Now I see it. The reason is that we would get a buffer overflow. Nasty.
struct lb_memory has a zero-sized map array, but we use one element.
struct lb_memory {
        u32 tag;
        u32 size;
        struct lb_memory_range map[0];
};

The fix is to declare
struct lb_memory_one_map {
        u32 tag;
        u32 size;
        struct lb_memory_range map[1];
};
and cast it to struct lb_memory.

Compile-tested patch follows.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Index: corebootv3-stage1_elfload_cleanup/include/tables.h
===================================================================
--- corebootv3-stage1_elfload_cleanup/include/tables.h	(Revision 783)
+++ corebootv3-stage1_elfload_cleanup/include/tables.h	(Arbeitskopie)
@@ -135,6 +135,12 @@
 	struct lb_memory_range map[0];
 };
 
+struct lb_memory_one_map {
+	u32 tag;
+	u32 size;
+	struct lb_memory_range map[1];
+};
+
 #define LB_TAG_HWRPB	0x0002
 struct lb_hwrpb {
 	u32 tag;
Index: corebootv3-stage1_elfload_cleanup/arch/x86/stage1.c
===================================================================
--- corebootv3-stage1_elfload_cleanup/arch/x86/stage1.c	(Revision 783)
+++ corebootv3-stage1_elfload_cleanup/arch/x86/stage1.c	(Arbeitskopie)
@@ -127,18 +127,16 @@
 	void *entry;
 #ifdef CONFIG_PAYLOAD_ELF_LOADER
 	struct mem_file result;
-	int elfboot_mem(struct lb_memory *mem, void *where, int size);
 
 	/* Why can't we statically init this hack? */
-	unsigned char faker[64];
-	struct lb_memory *mem = (struct lb_memory*) faker;
+	struct lb_memory_one_map mem;
 
-	mem->tag = LB_TAG_MEMORY;
-	mem->size = 28;
-	mem->map[0].start.lo = mem->map[0].start.hi = 0;
-	mem->map[0].size.lo = (32*1024*1024);
-	mem->map[0].size.hi = 0;
-	mem->map[0].type = LB_MEM_RAM;
+	mem.tag = LB_TAG_MEMORY;
+	mem.size = sizeof(mem);
+	mem.map[0].start.lo = mem.map[0].start.hi = 0;
+	mem.map[0].size.lo = (32*1024*1024);
+	mem.map[0].size.hi = 0;
+	mem.map[0].type = LB_MEM_RAM;
 #endif /* CONFIG_PAYLOAD_ELF_LOADER */
 
 	post_code(POST_STAGE1_MAIN);
@@ -221,7 +219,7 @@
 #ifdef CONFIG_PAYLOAD_ELF_LOADER
 	ret = find_file(&archive, "normal/payload", &result);
 	if (! ret)
-		legacy(&archive, "normal/payload", (void *)UNCOMPRESS_AREA, mem);
+		legacy(&archive, "normal/payload", (void *)UNCOMPRESS_AREA, (struct lb_memory *)&mem);
 #endif /* CONFIG_PAYLOAD_ELF_LOADER */
 
 	entry = load_file_segments(&archive, "normal/payload");


-- 
http://www.hailfinger.org/

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: linuxbios3_stage1_elfload_cleanup.diff
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20080818/630bbc2b/attachment.ksh>