[Skiboot] [PATCH v4 04/18] secvar_main: rework secvar_main error flow, make storage locking explicit

[Eric Richter]

This patch adjusts the behavior of secvar_main to actually halt the boot
in some form if there is an issue initializing secure variables. We
halt in skiboot if the secvar storage driver fails to initialize, as this
should only happen due to unresolvable hardware issues, and contains our
secure boot state. For all other cases we enforce secure boot in the
bootloader (secure mode flag is set, but there will be no keys).

Previously, the storage driver was expected to handle any locking
procedures implicitly as part of the write operation. This patch makes
the locking behavior now explicit, and part of the secvar_main flow.

The storage driver is now locked unconditionally when exiting
secvar_main, and the lock() call should halt the boot if it encounters
any sign of struggle.

Signed-off-by: Eric Richter <erichte at linux.ibm.com>
---
V4:
 - adjusted lines to 80 columns
 - calls secureboot_enforce() instead of abort()

 libstb/secvar/secvar_main.c | 38 +++++++++++++++++++++++++++++++++----
 1 file changed, 34 insertions(+), 4 deletions(-)

diff --git a/libstb/secvar/secvar_main.c b/libstb/secvar/secvar_main.c
index 3b842d16..f4b98925 100644
--- a/libstb/secvar/secvar_main.c
+++ b/libstb/secvar/secvar_main.c
@@ -8,6 +8,7 @@
 #include <stdlib.h>
 #include <skiboot.h>
 #include <opal.h>
+#include <libstb/secureboot.h>
 #include "secvar.h"
 #include "secvar_devtree.h"
 
@@ -39,11 +40,15 @@ int secvar_main(struct secvar_storage_driver storage_driver,
 	list_head_init(&variable_bank);
 	list_head_init(&update_bank);
 
+	/* Failures here should indicate some kind of hardware problem,
+	 * therefore we don't even attempt to continue */
 	rc = secvar_storage.store_init();
 	if (rc)
-		goto fail;
+		secureboot_enforce();
 
-	/* Failures here should indicate some kind of hardware problem */
+	/* Failures here may be recoverable,
+	 *  (PNOR was corrupted, restore from backup)
+	 * so we want to boot up to skiroot to give the user a chance to fix */
 	rc = secvar_storage.load_bank(&variable_bank, SECVAR_VARIABLE_BANK);
 	if (rc)
 		goto fail;
@@ -61,6 +66,8 @@ int secvar_main(struct secvar_storage_driver storage_driver,
 		rc = secvar_backend.pre_process();
 		if (rc) {
 			prlog(PR_ERR, "Error in backend pre_process = %d\n", rc);
+			/* Early failure state, lock the storage */
+			secvar_storage.lock();
 			goto out;
 		}
 	}
@@ -83,11 +90,20 @@ int secvar_main(struct secvar_storage_driver storage_driver,
 					       SECVAR_VARIABLE_BANK);
 		if (rc)
 			goto out;
-
-		rc = secvar_storage.write_bank(&update_bank, SECVAR_UPDATE_BANK);
+	}
+	/* Write (and probably clear) the update bank if .process() actually
+	 * detected and handled updates in the update bank. Unlike above, this
+	 * includes error cases, where the backend should probably be clearing
+	 * the bank.
+	 */
+	if (rc != OPAL_EMPTY) {
+		rc = secvar_storage.write_bank(&update_bank,
+					       SECVAR_UPDATE_BANK);
 		if (rc)
 			goto out;
 	}
+	/* Unconditionally lock the storage at this point */
+	secvar_storage.lock();
 
 	if (secvar_backend.post_process) {
 		rc = secvar_backend.post_process();
@@ -100,9 +116,23 @@ int secvar_main(struct secvar_storage_driver storage_driver,
 	prlog(PR_INFO, "secvar initialized successfully\n");
 
 	return OPAL_SUCCESS;
+
 fail:
+	/* Early failure, base secvar support failed to initialize */
 	secvar_set_status("fail");
+	secvar_storage.lock();
+	secvar_set_secure_mode();
+
+	prerror("secvar failed to initialize, rc = %04x\n", rc);
+	return rc;
+
 out:
+	/* Soft-failure, enforce secure boot in bootloader for debug/recovery */
+	clear_bank_list(&variable_bank);
+	clear_bank_list(&update_bank);
+	secvar_storage.lock();
+	secvar_set_secure_mode();
+
 	prerror("secvar failed to initialize, rc = %04x\n", rc);
 	return rc;
 }
-- 
2.21.1