@@ -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;
};
@@ -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;
}
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(-)