[coreboot] [PATCH] flashrom: Improve coreboot image detection heuristic

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Fri Jul 4 21:00:06 CEST 2008


On 04.07.2008 19:17, Stefan Reinauer wrote:
> Carl-Daniel Hailfinger wrote:
>
>   
>> Sorry. I had a correct version, but it seems I pressed undo before saving.
>>
>> Fix coreboot image detection heuristic.
>>
>> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>>   
>>     
>
> I wonder whether it makes sense to explicitly say !=0 for the
> non-isprint parts to express that those are not booleans but checks
> whether a char is 0
>   

I came to the conclusion that matching a possibly empty string in the
ROM does not make sense.

Cleaned up patch follows. It compiles for me, is a LOT more readable and
even removes hard-to-follow code from layout.c.

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

Index: layout.c
===================================================================
--- layout.c	(Revision 3412)
+++ layout.c	(Arbeitskopie)
@@ -45,7 +45,12 @@
 int show_id(uint8_t *bios, int size, int force)
 {
 	unsigned int *walk;
+	unsigned int mb_part_offset, mb_vendor_offset;
+	char *mb_part, *mb_vendor;
 
+	mainboard_vendor = def_name;
+	mainboard_part = def_name;
+
 	walk = (unsigned int *)(bios + size - 0x10);
 	walk--;
 
@@ -63,25 +68,27 @@
 	 * are outside the image of if the start of ID strings are nonsensical
 	 * (nonprintable and not \0).
 	 */
-	if ((*walk) == 0 || ((*walk) & 0x3ff) != 0 || *walk > size ||
-		*(walk - 1) > size || *(walk - 2) > size ||
-		(!isprint((const char *)(bios + size - *(walk - 1))) &&
-		((const char *)(bios + size - *(walk - 1)))) ||
-		(!isprint((const char *)(bios + size - *(walk - 2))) &&
-		((const char *)(bios + size - *(walk - 2))))) {
+	mb_part_offset = *(walk - 1);
+	mb_vendor_offset = *(walk - 2);
+	if ((*walk) == 0 || ((*walk) & 0x3ff) != 0 || (*walk) > size ||
+	    mb_part_offset > size || mb_vendor_offset > size) {
 		printf("Flash image seems to be a legacy BIOS. Disabling checks.\n");
-		mainboard_vendor = def_name;
-		mainboard_part = def_name;
 		return 0;
 	}
+	
+	mb_part = (char *)(bios + size - mb_part_offset);
+	mb_vendor = (char *)(bios + size - mb_vendor_offset);
+	if (!isprint(*mb_part) || !isprint(*mb_vendor)) {
+		printf("Flash image seems to have garbage in the ID location."
+			" Disabling checks.\n");
+		return 0;
+	}
 
 	printf_debug("coreboot last image size "
 		     "(not ROM size) is %d bytes.\n", *walk);
 
-	walk--;
-	mainboard_part = strdup((const char *)(bios + size - *walk));
-	walk--;
-	mainboard_vendor = strdup((const char *)(bios + size - *walk));
+	mainboard_part = strdup(mb_part);
+	mainboard_vendor = strdup(mb_vendor);
 	printf_debug("Manufacturer: %s\n", mainboard_vendor);
 	printf_debug("Mainboard ID: %s\n", mainboard_part);
 



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





More information about the coreboot mailing list