[coreboot-gerrit] Patch set updated for coreboot: cbfstool: make fmap search more strict

[Patrick Georgi]

Patrick Georgi ([e-mail]) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/11690

-gerrit

commit b1dbdb2b15b92d16278d7be82b957d484fe9e568
Author: Patrick Georgi 
Date:   Sat Sep 19 13:59:36 2015 +0200

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 
---
 util/cbfstool/flashmap/fmap.c | 45 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/util/cbfstool/flashmap/fmap.c b/util/cbfstool/flashmap/fmap.c
index f143beb..f803d6c 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;
    		}