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 <[EMAIL PROTECTED]>

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/

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");
--
coreboot mailing list
[email protected]
http://www.coreboot.org/mailman/listinfo/coreboot

Reply via email to