cbfstool: make fmap search more strict

Since fmap doesn't come with a checksum, we resort to a number of
heuristics to determine if a given location hosts an fmap (instead of
another data structure that happens to store the fmap magic string at
the right location).

The version test is particularly effective against strings containing
the magic (which either terminate with 0, or have some other ASCII data,
but rarely a '\001' byte inside the string).

Change-Id: Ic66eb0015c7ffdfe25e0054b7838445b8ba098e9
Signed-off-by: Patrick Georgi <pgeorgi@chromium.org>
Reviewed-on: http://review.coreboot.org/11690
Tested-by: build bot (Jenkins)
Reviewed-by: Aaron Durbin <adurbin@chromium.org>

1 files changed, 39 insertions, 6 deletions

diff --git a/util/cbfstool/flashmap/fmap.c b/util/cbfstool/flashmap/fmap.c
index f143beb90f..f803d6c0da 100644
--- a/util/cbfstool/flashmap/fmap.c
+++ b/util/cbfstool/flashmap/fmap.c
@@ -34,6 +34,7 @@
 
 #define _XOPEN_SOURCE 700
 
+#include <ctype.h>
 #include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
@@ -69,6 +70,41 @@ int fmap_size(const struct fmap *fmap)
 	return sizeof(*fmap) + (fmap->nareas * sizeof(struct fmap_area));
 }
 
+/* Make a best-effort assessment if the given fmap is real */
+static int is_valid_fmap(const struct fmap *fmap)
+{
+	if (memcmp(fmap, FMAP_SIGNATURE, strlen(FMAP_SIGNATURE)) != 0)
+		return 0;
+	/* strings containing the magic tend to fail here */
+	if (fmap->ver_major != FMAP_VER_MAJOR)
+		return 0;
+	/* a basic consistency check: flash should be larger than fmap */
+	if (fmap->size <
+		sizeof(*fmap) + fmap->nareas * sizeof(struct fmap_area))
+		return 0;
+
+	/* fmap-alikes along binary data tend to fail on having a valid,
+	 * null-terminated string in the name field.*/
+	int i = 0;
+	while (i < FMAP_STRLEN) {
+		if (fmap->name[i] == 0)
+			break;
+		if (!isalnum(fmap->name[i]))
+			return 0;
+		if (i == FMAP_STRLEN - 1) {
+			/* name is specified to be null terminated. We didn't
+			 * break in the 0 test, we didn't fail on the alnum
+			 * test, so we're seeing FMAP_STRLEN alphanumerical
+			 * symbols, which is one too many.
+			 */
+			 return 0;
+		}
+		i++;
+	}
+	return 1;
+
+}
+
 /* brute force linear search */
 static long int fmap_lsearch(const uint8_t *image, size_t len)
 {
@@ -76,9 +112,7 @@ static long int fmap_lsearch(const uint8_t *image, size_t len)
 	int fmap_found = 0;
 
 	for (offset = 0; offset < len - strlen(FMAP_SIGNATURE); offset++) {
-		if (!memcmp(&image[offset],
-			    FMAP_SIGNATURE,
-			    strlen(FMAP_SIGNATURE))) {
+		if (is_valid_fmap((const struct fmap *)&image[offset])) {
 			fmap_found = 1;
 			break;
 		}
@@ -114,9 +148,8 @@ static long int fmap_bsearch(const uint8_t *image, size_t len)
 		     offset += stride) {
 			if ((offset % (stride * 2) == 0) && (offset != 0))
 					continue;
-			if (!memcmp(&image[offset],
-				    FMAP_SIGNATURE,
-				    strlen(FMAP_SIGNATURE))) {
+			if (is_valid_fmap(
+				(const struct fmap *)&image[offset])) {
 				fmap_found = 1;
 				break;
 			}