[Skiboot] [PATCH 1/5] nvram: force re-verification after writing

Stewart Smith stewart at linux.vnet.ibm.com
Fri Oct 14 17:01:29 AEDT 2016


From: Oliver O'Halloran <oohall at gmail.com>

The running OS is free to re-write the contents of NVRAM. The skiboot
NVRAM parser relies on the NVRAM contents being valid so we need to
force the NVRAM contents to be revalidated after the host OS has written
to it.

Signed-off-by: Oliver O'Halloran <oohall at gmail.com>
Signed-off-by: Stewart Smith <stewart at linux.vnet.ibm.com>
---
 core/nvram-format.c          | 23 ++++++++++++++++++----
 core/nvram.c                 | 45 +++++++++++++++++++++++++++++++-------------
 core/test/run-nvram-format.c |  5 +++++
 include/nvram.h              |  1 +
 4 files changed, 57 insertions(+), 17 deletions(-)

diff --git a/core/nvram-format.c b/core/nvram-format.c
index 227147efbaa9..b98aee1430ff 100644
--- a/core/nvram-format.c
+++ b/core/nvram-format.c
@@ -208,12 +208,27 @@ static const char *find_next_key(const char *start, const char *end)
  */
 const char *nvram_query(const char *key)
 {
-	const char *part_end = (const char *) skiboot_part_hdr +
-		skiboot_part_hdr->len * 16 - 1;
-	const char *start = (const char *) skiboot_part_hdr +
-		sizeof(*skiboot_part_hdr);
+	const char *part_end, *start;
 	int key_len = strlen(key);
 
+	/*
+	 * The running OS can modify the NVRAM as it pleases so we need to be
+	 * a little paranoid and check that it's ok before we try parse it.
+	 *
+	 * NB: nvram_validate() can update skiboot_part_hdr
+	 */
+	if (!nvram_validate()) {
+		prerror("NVRAM: Look up for '%s' failed due to bad format!\n",
+			key);
+		return NULL;
+	}
+
+	part_end = (const char *) skiboot_part_hdr
+		+ skiboot_part_hdr->len * 16 - 1;
+
+	start = (const char *) skiboot_part_hdr
+		+ sizeof(*skiboot_part_hdr);
+
 	if (!key_len) {
 		prlog(PR_WARNING, "NVRAM: search key is empty!\n");
 		return NULL;
diff --git a/core/nvram.c b/core/nvram.c
index 3ae0f12aca8b..2140706a15b9 100644
--- a/core/nvram.c
+++ b/core/nvram.c
@@ -24,7 +24,9 @@
 
 static void *nvram_image;
 static uint32_t nvram_size;
-static bool nvram_ready;
+
+static bool nvram_ready; /* has the nvram been loaded? */
+static bool nvram_valid; /* is the nvram format ok? */
 
 static int64_t opal_read_nvram(uint64_t buffer, uint64_t size, uint64_t offset)
 {
@@ -55,21 +57,37 @@ static int64_t opal_write_nvram(uint64_t buffer, uint64_t size, uint64_t offset)
 	memcpy(nvram_image + offset, (void *)buffer, size);
 	if (platform.nvram_write)
 		platform.nvram_write(offset, nvram_image + offset, size);
+
+	/* The host OS has written to the NVRAM so we can't be sure that it's
+	 * well formatted.
+	 */
+	nvram_valid = false;
+
 	return OPAL_SUCCESS;
 }
 opal_call(OPAL_WRITE_NVRAM, opal_write_nvram, 3);
 
-static void nvram_validate(void)
+bool nvram_validate(void)
+{
+	if (!nvram_valid)
+		nvram_valid = !nvram_check(nvram_image, nvram_size);
+
+	return nvram_valid;
+}
+
+static void nvram_reformat(void)
 {
-	/* Check and maybe format nvram */
-	if (nvram_check(nvram_image, nvram_size)) {
-		if (nvram_format(nvram_image, nvram_size))
-			prerror("NVRAM: Failed to format NVRAM!\n");
-
-		/* Write the whole thing back */
-		if (platform.nvram_write)
-			platform.nvram_write(0, nvram_image, nvram_size);
+	if (nvram_format(nvram_image, nvram_size)) {
+		prerror("NVRAM: Failed to format NVRAM!\n");
+		nvram_valid = false;
+		return;
 	}
+
+	/* Write the whole thing back */
+	if (platform.nvram_write)
+		platform.nvram_write(0, nvram_image, nvram_size);
+
+	nvram_valid = true;
 }
 
 void nvram_reinit(void)
@@ -77,8 +95,8 @@ void nvram_reinit(void)
 	/* It's possible we failed to load nvram at boot. */
 	if (!nvram_ready)
 		nvram_init();
-	else
-		nvram_validate();
+	else if (!nvram_validate())
+		nvram_reformat();
 }
 
 void nvram_read_complete(bool success)
@@ -92,7 +110,8 @@ void nvram_read_complete(bool success)
 		return;
 	}
 
-	nvram_validate();
+	if (!nvram_validate())
+		nvram_reformat();
 
 	/* Add nvram node */
 	np = dt_new(opal_node, "nvram");
diff --git a/core/test/run-nvram-format.c b/core/test/run-nvram-format.c
index 4bf3ae07f0eb..4ca2cded2a60 100644
--- a/core/test/run-nvram-format.c
+++ b/core/test/run-nvram-format.c
@@ -18,6 +18,11 @@
 
 #include "../nvram-format.c"
 
+bool nvram_validate(void)
+{
+	return true;
+}
+
 static char *nvram_reset(void *nvram_image, int size)
 {
 	struct chrp_nvram_hdr *h = nvram_image;
diff --git a/include/nvram.h b/include/nvram.h
index 19f8eff633e4..d87561d79397 100644
--- a/include/nvram.h
+++ b/include/nvram.h
@@ -20,6 +20,7 @@
 int nvram_format(void *nvram_image, uint32_t nvram_size);
 int nvram_check(void *nvram_image, uint32_t nvram_size);
 void nvram_reinit(void);
+bool nvram_validate(void);
 
 const char *nvram_query(const char *name);
 
-- 
2.1.4



More information about the Skiboot mailing list