[coreboot] [PATCH] v3: fix bug in ad-hoc archive finding code in mc146818rtc.c

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Sun Feb 10 13:02:04 CET 2008


I found another bug in LAR handling of the option table by staring at
the logs. (Yes, I read logs.)
Looking at the log of a qemu x86 boot, I'd say something is really
broken. Look at the excerpts of my log quoted below:
> [...]
> LAR: Attempting to open 'fallback/initram/segment0'.
> LAR: Start 0xfff00000 len 0x100000
> LAR: seen member normal/option_table
> LAR: seen member normal/stage2/segment0
> LAR: seen member normal/stage2/segment1
> LAR: seen member normal/stage2/segment2
> LAR: seen member normal/initram/segment0
> LAR: seen member bootblock
> LAR: File not found!
> [...]
> LAR: Attempting to open 'normal/initram/segment0'.
> LAR: Start 0xfff00000 len 0x100000
> LAR: seen member normal/option_table
> LAR: seen member normal/stage2/segment0
> LAR: seen member normal/stage2/segment1
> LAR: seen member normal/stage2/segment2
> LAR: seen member normal/initram/segment0
> LAR: CHECK normal/initram/segment0 @ 0xfff040d0
> [...]
> LAR: Attempting to open 'normal/stage2/segment0'.
> LAR: Start 0xfff00000 len 0x100000
> LAR: seen member normal/option_table
> LAR: seen member normal/stage2/segment0
> LAR: CHECK normal/stage2/segment0 @ 0xfff00400
> [...]
> LAR: Attempting to open 'normal/stage2/segment1'.
> LAR: Start 0xfff00000 len 0x100000
> LAR: seen member normal/option_table
> LAR: seen member normal/stage2/segment0
> LAR: seen member normal/stage2/segment1
> LAR: CHECK normal/stage2/segment1 @ 0xfff004c0
> [...]
> LAR: Attempting to open 'normal/stage2/segment2'.
> LAR: Start 0xfff00000 len 0x100000
> LAR: seen member normal/option_table
> LAR: seen member normal/stage2/segment0
> LAR: seen member normal/stage2/segment1
> LAR: seen member normal/stage2/segment2
> LAR: CHECK normal/stage2/segment2 @ 0xfff03f40
> [...]
> LAR: Attempting to open 'normal/stage2/segment3'.
> LAR: Start 0xfff00000 len 0x100000
> LAR: seen member normal/option_table
> LAR: seen member normal/stage2/segment0
> LAR: seen member normal/stage2/segment1
> LAR: seen member normal/stage2/segment2
> LAR: seen member normal/initram/segment0
> LAR: seen member bootblock
> LAR: File not found!
> [...]
> LAR: Attempting to open 'normal/option_table'.
> LAR: Start 0xfffc0000 len 0x3c000

WTF?!? This start address is obviously very wrong.

> LAR: seen member bootblock
> LAR: File not found!

Which results in not finding the option table.

> [...]
> LAR: Attempting to open 'normal/payload'.
> LAR: Start 0xfff00000 len 0x100000
> LAR: seen member normal/option_table
> LAR: seen member normal/stage2/segment0
> LAR: seen member normal/stage2/segment1
> LAR: seen member normal/stage2/segment2
> LAR: seen member normal/initram/segment0
> LAR: seen member bootblock
> LAR: File not found!
> [...]
> LAR: Attempting to open 'normal/payload/segment0'.
> LAR: Start 0xfff00000 len 0x100000
> LAR: seen member normal/option_table
> LAR: seen member normal/stage2/segment0
> LAR: seen member normal/stage2/segment1
> LAR: seen member normal/stage2/segment2
> LAR: seen member normal/initram/segment0
> LAR: seen member bootblock
> LAR: File not found!
> [...]

Short version of the above:
> LAR: Start 0xfff00000 len 0x100000
> LAR: Start 0xfff00000 len 0x100000
> LAR: Start 0xfff00000 len 0x100000
> LAR: Start 0xfff00000 len 0x100000
> LAR: Start 0xfff00000 len 0x100000
> LAR: Start 0xfff00000 len 0x100000
> LAR: Start 0xfffc0000 len 0x3c000 <------BUG!
> LAR: Start 0xfff00000 len 0x100000
> LAR: Start 0xfff00000 len 0x100000

The bug is in arch/x86/mc146818rtc.c:

> struct cmos_option_table *get_option_table(void)
> {
> 	struct mem_file result, archive;
> 	int ret;
>
> 	// FIXME - i want to be dynamic.
> 	archive.len=(CONFIG_COREBOOT_ROMSIZE_KB-16)*1024;

len is wrong.

> 	archive.start=(void *)(0UL-(CONFIG_COREBOOT_ROMSIZE_KB*1024)); 

start is even more wrong.

> 	ret = find_file(&archive, "normal/option_table", &result);
> 	if (ret) {
> 		printk(BIOS_ERR, "No such file '%s'.\n",
> 				"normal/option_table");
> 		return (struct cmos_option_table *)NULL;
> 	}
> 	return (struct cmos_option_table *) result.start;
> }

Use the existing init_archive function to find the LAR in memory.
This fixes the case where the option table was not found depending
on a few unrelated parameters.

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


Index: LinuxBIOSv3-optiontable/arch/x86/mc146818rtc.c
===================================================================
--- LinuxBIOSv3-optiontable/arch/x86/mc146818rtc.c	(Revision 583)
+++ LinuxBIOSv3-optiontable/arch/x86/mc146818rtc.c	(Arbeitskopie)
@@ -183,9 +183,7 @@
 	struct mem_file result, archive;
 	int ret;
 
-	// FIXME - i want to be dynamic.
-	archive.len=(CONFIG_COREBOOT_ROMSIZE_KB-16)*1024;
-	archive.start=(void *)(0UL-(CONFIG_COREBOOT_ROMSIZE_KB*1024)); 
+	init_archive(&archive);
 
 	ret = find_file(&archive, "normal/option_table", &result);
 	if (ret) {


Regards,
Carl-Daniel

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





More information about the coreboot mailing list