#102: flashrom: coreboot ROM image file identification heuristic is broken -------------------------+-------------------------------------------------- Reporter: stuge | Owner: somebody Type: defect | Status: reopened Priority: major | Milestone: flashrom v1.0 Component: flashrom | Version: Resolution: | Keywords: rom image heuristic Dependencies: | Patchstatus: there is no patch -------------------------+--------------------------------------------------
Comment(by hailfinger):
Replying to [comment:6 anonymous]:
Is this what the author possibly meant?
Not completely. The patch below is a pure warning fix, but it doesn't fix the implementation of the mechanism completely.
{{{ Index: layout.c =================================================================== --- layout.c (revision 3412) +++ layout.c (working copy) @@ -65,9 +65,9 @@ */ if ((*walk) == 0 || ((*walk) & 0x3ff) != 0 || *walk > size || *(walk - 1) > size || *(walk - 2) > size ||
(!isprint((const char *)(bios + size - *(walk - 1))) &&
(!isprint(*(const char *)(bios + size - *(walk - 1))) && ((const char *)(bios + size - *(walk - 1)))) ||
(!isprint((const char *)(bios + size - *(walk - 2))) &&
(!isprint(*(const char *)(bios + size - *(walk - 2))) && ((const char *)(bios + size - *(walk - 2))))) { printf("Flash image seems to be a legacy BIOS. Disabling
checks.\n");
mainboard_vendor = def_name;}}}
Was this code ever tested or reviewed? It obviously never worked like
that.
It compiled cleanly without warnings on my machine (gcc 4.2.1), otherwise I wouldn't have committed it.
And I would be very careful with claiming "it obviously never worked". If it was so obvious, why did you miss 50% of the pointer dereference bugs? Probably because you didn't read the code and only looked at compiler warnings.