[flashrom-gerrit] [M] Change in flashrom[master]: flashrom: Check for flash access restricitons in write_flash()

[Edward O'Callaghan (Code Review)]

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/70516 )

(

7 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
 )Change subject: flashrom: Check for flash access restricitons in write_flash()
......................................................................

flashrom: Check for flash access restricitons in write_flash()

Make write_flash() skip unwritable regions if
FLASHROM_FLAG_SKIP_UNWRITABLE_REGIONS is true. If the flag is false
write operations that include an unwritable region will not write
anything and return an error.

BUG=b:260440773
BRANCH=none
TEST=flashrom -w on dedede (JSL)

Change-Id: Idacf0d5218da9d9929f4877fc7665fe608b87fe0
CoAuthored-by: Edward O'Callaghan 
Signed-off-by: Edward O'Callaghan 
Signed-off-by: Nikolai Artemiev 
Reviewed-on: https://review.coreboot.org/c/flashrom/+/70516
Reviewed-by: Edward O'Callaghan 
Reviewed-by: Angel Pons 
Tested-by: build bot (Jenkins) 
---
M flashrom.c
1 file changed, 92 insertions(+), 3 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Angel Pons: Looks good to me, approved
  Edward O'Callaghan: Looks good to me, but someone else must approve

diff --git a/flashrom.c b/flashrom.c
index 785da6f..b611beb 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -374,6 +374,23 @@
    }
 }

+static int check_for_unwritable_regions(const struct flashctx *flash, unsigned int start, unsigned int len)
+{
+	struct flash_region region;
+	for (unsigned int addr = start; addr < start + len; addr = region.end) {
+		get_flash_region(flash, addr, &region);
+
+		if (region.write_prot) {
+			msg_gerr("%s: cannot write/erase inside %s region (%#08x..%#08x).\n",
+				 __func__, region.name, region.start, region.end - 1);
+			free(region.name);
+			return -1;
+		}
+		free(region.name);
+	}
+	return 0;
+}
+
 /* special unit-test hook */
 erasefunc_t *g_test_erase_injector;

@@ -966,10 +983,57 @@
    return NULL;
 }

-static int write_flash(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len)
+/*
+ * write_flash - wrapper for flash->write() with additional high-level policy
+ *
+ * @param flash flash chip
+ * @param buf   buffer to write to flash
+ * @param start start address in flash
+ * @param len   number of bytes to write
+ * @return      0 on success,
+ *              -1 if any write fails.
+ *
+ * This wrapper simplifies most cases when the flash chip needs to be written
+ * since policy decisions such as non-fatal error handling is centralized.
+ */
+static int write_flash(struct flashctx *flash, const uint8_t *buf,
+		       unsigned int start, unsigned int len)
 {
-	write_func_t *write_func = lookup_write_func_ptr(flash->chip);
-	return write_func(flash, buf, start, len);
+	if (!flash->flags.skip_unwritable_regions) {
+		if (check_for_unwritable_regions(flash, start, len))
+			return -1;
+	}
+
+	unsigned int write_len;
+	for (unsigned int addr = start; addr < start + len; addr += write_len) {
+		struct flash_region region;
+		get_flash_region(flash, addr, &region);
+
+		write_len = min(start + len, region.end) - addr;
+		const uint8_t *rbuf = buf + addr - start;
+
+		if (region.write_prot) {
+			msg_gdbg("%s: cannot write inside %s region (%#08x..%#08x), skipping (%#08x..%#08x).\n",
+				 __func__, region.name, region.start, region.end - 1, addr, addr + write_len - 1);
+			free(region.name);
+			continue;
+		}
+
+		msg_gdbg("%s: %s region (%#08x..%#08x) is writable, writing range (%#08x..%#08x).\n",
+			 __func__, region.name, region.start, region.end - 1, addr, addr + write_len - 1);
+
+		write_func_t *write_func = lookup_write_func_ptr(flash->chip);
+		int ret = write_func(flash, rbuf, addr, write_len);
+		if (ret) {
+			msg_gerr("%s: failed to write (%#08x..%#08x).\n", __func__, addr, addr + write_len - 1);
+			free(region.name);
+			return -1;
+		}
+
+		free(region.name);
+	}
+
+	return 0;
 }

typedef int (probe_func_t)(struct flashctx *flash);