[flashrom] [PATCH] layout: Verify layout entries before building a new image using them.

[Stefan Tauner]

This fixes a SEGFAULT if a layout entry is included that addresses memory
outside the current chip's address range.

Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
---

Before continuing with the other layout patches, I'd like to fix this
issue (and copy the patch that fixes this also to the .7 stable branch).

The macros are open for discussion not only regarding naming...
e.g. it might be easier to declare a macro that provides the full printf
format specification for the address, '0x06" PRIx32 "' to make the
resulting code more readable.

 flash.h    | 11 ++++++++++-
 flashrom.c | 11 ++++++++---
 layout.c   | 29 +++++++++++++++++++++++++++--
 3 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/flash.h b/flash.h
index 7b88477..a5033ef 100644
--- a/flash.h
+++ b/flash.h
@@ -45,6 +45,14 @@
 typedef uintptr_t chipaddr;
 #define PRIxPTR_WIDTH ((int)(sizeof(uintptr_t)*2))
 
+/* To be used in technology-independent code. */
+#define FL_MAX_CHIPADDR (24)
+#define FL_MAX_CHIPSIZE ((1<<FL_MAX_CHIPADDR)-1)
+#define PRIxCHIPADDR_WIDTH ((int)(FL_MAX_CHIPADDR/4))
+typedef uint32_t chipoff_t; /* Able to store any addressable offset within a supported flash memory. */
+typedef uint32_t chipsize_t; /* Able to store the number of bytes of any supported flash memory. */
+#define PRIxCHIPADDR PRIx32
+
 int register_shutdown(int (*function) (void *data), void *data);
 void *programmer_map_flash_region(const char *descr, uintptr_t phys_addr, size_t len);
 void programmer_unmap_flash_region(void *virt_addr, size_t len);
@@ -319,7 +327,8 @@ __attribute__((format(printf, 2, 3)));
 int register_include_arg(char *name);
 int process_include_args(void);
 int read_romlayout(char *name);
-int handle_romentries(const struct flashctx *flash, uint8_t *oldcontents, uint8_t *newcontents);
+int normalize_romentries(const struct flashctx *flash);
+int build_new_image(const struct flashctx *flash, uint8_t *oldcontents, uint8_t *newcontents);
 void layout_cleanup(void);
 
 /* spi.c */
diff --git a/flashrom.c b/flashrom.c
index 9169620..afab57c 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -1916,6 +1916,12 @@ int doit(struct flashctx *flash, int force, const char *filename, int read_it,
 		goto out_nofree;
 	}
 
+	if (normalize_romentries(flash)) {
+		msg_cerr("Requested regions can not be handled. Aborting.\n");
+		ret = 1;
+		goto out_nofree;
+	}
+
 	/* Given the existence of read locks, we want to unlock for read,
 	 * erase and write.
 	 */
@@ -1995,9 +2001,8 @@ int doit(struct flashctx *flash, int force, const char *filename, int read_it,
 	}
 	msg_cinfo("done.\n");
 
-	// This should be moved into each flash part's code to do it 
-	// cleanly. This does the job.
-	handle_romentries(flash, oldcontents, newcontents);
+	/* Build a new image taking the given layout into account. */
+	build_new_image(flash, oldcontents, newcontents);
 
 	// ////////////////////////////////////////////////////////////
 
diff --git a/layout.c b/layout.c
index 86351b8..5adec67 100644
--- a/layout.c
+++ b/layout.c
@@ -37,7 +37,7 @@ typedef struct {
 
 /* rom_entries store the entries specified in a layout file and associated run-time data */
 static romentry_t rom_entries[MAX_ROMLAYOUT];
-static int num_rom_entries = 0; /* the number of valid rom_entries */
+static int num_rom_entries = 0; /* the number of successfully parsed rom_entries */
 
 /* include_args holds the arguments specified at the command line with -i. They must be processed at some point
  * so that desired regions are marked as "included" in the rom_entries list. */
@@ -232,7 +232,32 @@ romentry_t *get_next_included_romentry(unsigned int start)
 	return best_entry;
 }
 
-int handle_romentries(const struct flashctx *flash, uint8_t *oldcontents, uint8_t *newcontents)
+/* Validate and - if needed - normalize layout entries. */
+int normalize_romentries(const struct flashctx *flash)
+{
+	chipsize_t total_size = flash->chip->total_size * 1024;
+	int ret = 0;
+
+	int i;
+	for (i = 0; i < num_rom_entries; i++) {
+		if (rom_entries[i].start >= total_size || rom_entries[i].end >= total_size) {
+			msg_gwarn("Warning: Address range of region \"%s\" exeeds the current chip's "
+				  "address space.\n", rom_entries[i].name);
+			if (rom_entries[i].included)
+				ret = 1;
+		}
+		if (rom_entries[i].start >= rom_entries[i].end) {
+			msg_gwarn("Warning: Size of the address range of region \"%s\" is not positive.\n",
+				  rom_entries[i].name);
+			if (rom_entries[i].included)
+				ret = 1;
+		}
+	}
+
+	return ret;
+}
+
+int build_new_image(const struct flashctx *flash, uint8_t *oldcontents, uint8_t *newcontents)
 {
 	unsigned int start = 0;
 	romentry_t *entry;
-- 
Kind regards, Stefan Tauner