diff mbox series

[v3,05/15] secvar_main: rework secvar_main error flow, make storage locking explicit

Message ID 20200401003426.7198-6-erichte@linux.ibm.com
State Superseded
Headers show
Series Add initial secure variable storage and backend drivers | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch master (6826095796c91583f344950ff76ef8ccf6e756b5)
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot success Test snowpatch/job/snowpatch-skiboot on branch master
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco success Signed-off-by present

Commit Message

Eric Richter April 1, 2020, 12:34 a.m. UTC
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@linux.ibm.com>
---
 include/secvar.h            |  7 ++++---
 libstb/secvar/secvar_main.c | 32 +++++++++++++++++++++++++++++---
 2 files changed, 33 insertions(+), 6 deletions(-)
diff mbox series

Patch

diff --git a/include/secvar.h b/include/secvar.h
index c41fb739..cabde036 100644
--- a/include/secvar.h
+++ b/include/secvar.h
@@ -9,9 +9,10 @@ 
 struct secvar;
 
 struct secvar_storage_driver {
-        int (*load_bank)(struct list_head *bank, int section);
-        int (*write_bank)(struct list_head *bank, int section);
-        int (*store_init)(void);
+	int (*load_bank)(struct list_head *bank, int section);
+	int (*write_bank)(struct list_head *bank, int section);
+	int (*store_init)(void);
+	void (*lock)(void);
 	uint64_t max_var_size;
 };
 
diff --git a/libstb/secvar/secvar_main.c b/libstb/secvar/secvar_main.c
index 787e4473..536b1643 100644
--- a/libstb/secvar/secvar_main.c
+++ b/libstb/secvar/secvar_main.c
@@ -39,11 +39,14 @@  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;
+		abort();
 
-	/* 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;
@@ -60,6 +63,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;
 		}
 	}
@@ -81,11 +86,18 @@  int secvar_main(struct secvar_storage_driver storage_driver,
 		rc = secvar_storage.write_bank(&variable_bank, SECVAR_VARIABLE_BANK);
 		if (rc)
 			goto out;
-
+	}
+	/* 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();
@@ -98,9 +110,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;
 }