| Message ID | 20221128112326.27564-3-christian.storm@siemens.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/3] bootloader: Add is_bootloader() to distinguish bootloaders | expand |
Hi Christian, CI fails, see: https://source.denx.de/swupdate/swupdate/-/pipelines/14326 On 28.11.22 12:23, Christian Storm wrote: > Completely rework the EFI Boot Guard Binding accommodating to > libebgenv's implicit assumptions and actions. To this end, > environment modification transactions are introduced. > The assumptions and rationale is documented and in addition > log messages are emitted for particular combinations of > bootloader_transaction_marker and bootloader_state_marker. > > Signed-off-by: Christian Storm <christian.storm@siemens.com> > --- > bootloader/ebg.c | 567 ++++++++++++++++++++++++++++++---------- > core/stream_interface.c | 34 +++ > 2 files changed, 469 insertions(+), 132 deletions(-) > > diff --git a/bootloader/ebg.c b/bootloader/ebg.c > index 4194a38..99ee40b 100644 > --- a/bootloader/ebg.c > +++ b/bootloader/ebg.c > @@ -1,22 +1,22 @@ > /* > * Author: Christian Storm > - * Author: Andreas Reichel > - * Copyright (C) 2018, Siemens AG > + * Copyright (C) 2022, Siemens AG > * > * SPDX-License-Identifier: GPL-2.0-only > */ > > +#include <stdint.h> > #include <unistd.h> > #include <stdlib.h> > #include <limits.h> > #include <stdbool.h> > #include <stdio.h> > #include <string.h> > -#include <util.h> > +#include <dlfcn.h> > #include <efibootguard/ebgenv.h> > -#include <generated/autoconf.h> > -#include <state.h> > -#include "dlfcn.h" > +#include "generated/autoconf.h" > +#include "util.h" > +#include "state.h" > #include "bootloader.h" > > static struct { > @@ -25,184 +25,481 @@ static struct { > int (*env_open_current)(ebgenv_t *e); > int (*env_get)(ebgenv_t *e, char *key, char* buffer); > int (*env_set)(ebgenv_t *e, char *key, char *value); > - int (*env_set_ex)(ebgenv_t *e, char *key, uint64_t datatype, uint8_t *value, uint32_t datalen); > + int (*env_set_ex)(ebgenv_t *e, char *key, uint64_t datatype, > + uint8_t *value, uint32_t datalen); > uint16_t (*env_getglobalstate)(ebgenv_t *e); > int (*env_setglobalstate)(ebgenv_t *e, uint16_t ustate); > int (*env_close)(ebgenv_t *e); > int (*env_finalize_update)(ebgenv_t *e); > } libebg; > > -static ebgenv_t ebgenv = {0}; > > -static int do_env_set(const char *name, const char *value) > +/* > + * ---------------------------------------------------------------------------- > + * | Logics, Assumptions & Rationale | > + * ---------------------------------------------------------------------------- > + * > + * EFI Boot Guard boots the environment having `EBGENV_IN_PROGRESS == 0` > + * and the highest revision number. If multiple environments have the highest > + * revision number, environment probing order is decisive. > + * This environment is called the *current boot path*. > + * Sorted descending on revision numbers and arbitrated by probing order, the > + * other environments are termed *alternative boot paths*. > + * > + * Environment modifications ― except blessing a successful update ― must not > + * touch the current boot path. Instead, a new boot path is created by > + * "upcycling" the least recent alternative boot path. > + * More specifically, environment modifications are captured in a *transaction*: > + * An in-memory working copy of the current boot path environment is created > + * which has a by one incremented higher revision than the current boot path. > + * Modifications are performed on this working copy environment. > + * When committing the transaction, i.e., writing it to disk, the new current > + * boot path is persisted and booted next. > + * > + * A transaction is started by setting > + * `EBGENV_USTATE = STATE_IN_PROGRESS` or > + * `BOOTVAR_TRANSACTION = STATE_IN_PROGRESS` > + * which is idempotent. Then, `libebgenv` sets > + * - `EBGENV_IN_PROGRESS = 1` and > + * - `EBGENV_REVISION` to the current boot path's revision plus one, and > + * - the transaction `inflight` marker is set to `true`. > + * > + * A transaction is committed when setting `EBGENV_USTATE = STATE_INSTALLED`. > + * Then, `libebgenv` sets > + * - `EBGENV_IN_PROGRESS = 0` and > + * - `EBGENV_USTATE = USTATE_INSTALLED`, > + * - the new current boot path is persisted to disk, and > + * - the transaction `inflight` marker is reset to `false`. > + * With this, the current boot path becomes the most recent alternative boot > + * path serving as rollback boot path if the new current boot path fails to > + * boot. In this case, the failed boot path is marked with > + * - `EBGENV_USTATE = USTATE_FAILED` and > + * - `EBGENV_REVISION = 0` > + * by which the rollback boot path becomes the current boot path (again). > + * If the new current boot path boots successfully, the /current/ boot path > + * needs to be written to acknowledge the successful update via setting > + * `EBGENV_USTATE = USTATE_OK`. > + * > + * Note: The modification of EFI Boot Guard's environment variable > + * `EBGENV_IN_PROGRESS` cannot be disabled as it's hard-wired in EFI > + * Boot Guard with particular semantics. > + * Note: Successive calls to libebgenv's `env_open_current()` after the first > + * invocation are no-ops and select the in-memory working copy's current > + * environment. > + * Note: libebgenv's `env_close()` first *writes* to the current environment and > + * then closes it. There's currently no way to just close it, e.g., for re- > + * loading the environment from disk. > + * > + */ > + > + > +/* EFI Boot Guard hard-coded environment variable names. */ > +#define EBGENV_IN_PROGRESS (char *)"in_progress" > +#define EBGENV_REVISION (char *)"revision" > +#define EBGENV_USTATE (char *)"ustate" > + > +static ebgenv_t ebgenv = { 0 }; > +static bool inflight = false; > + > +static inline bool is(const char *s1, const char *s2) > { > - int ret; > + return strcmp(s1, s2) == 0; > +} > > - errno = 0; > - libebg.beverbose(&ebgenv, loglevel > INFOLEVEL ? true : false); > +static char *_env_get(const char *name) > +{ > + /* > + * libebgenv's env_get() is two-staged: The first call yields the > + * value's size in bytes, the second call, with an accordingly > + * sized buffer, yields the actual value. > + */ > + size_t size = libebg.env_get(&ebgenv, (char *)name, NULL); > + if (size == 0) { > + WARN("Cannot find key %s", name); > + return NULL; > + } > > - if ((ret = libebg.env_open_current(&ebgenv)) != 0) { > - ERROR("Cannot open current bootloader environment: %s.", strerror(ret)); > - return ret; > - } > - > - if (strncmp(name, BOOTVAR_TRANSACTION, strlen(name) + 1) == 0 && > - strncmp(value, get_state_string(STATE_IN_PROGRESS), > - strlen(get_state_string(STATE_IN_PROGRESS)) + 1) == 0) { > - DEBUG("Setting %s=%s in bootloader environment", name, value); > - /* Open or create a new environment to reflect > - * EFI Boot Guard's representation of SWUpdate's > - * recovery_status=in_progress. */ > - if ((ret = libebg.env_create_new(&ebgenv)) != 0) { > - ERROR("Cannot open/create new bootloader environment: %s.", > - strerror(ret)); > - } > - } else if (strncmp(name, (char *)STATE_KEY, strlen((char *)STATE_KEY) + 1) == 0) { > - /* Map update_state_t to EFI Boot Guard's API. */ > - switch (*value) { > - case STATE_IN_PROGRESS: > - case STATE_FAILED: > - case STATE_TESTING: > - /* Fall-through for update_state_t values destined either > - * for BOOTVAR_TRANSACTION or handled by EBG internally. */ > - break; > - case STATE_OK: > - case STATE_INSTALLED: > - DEBUG("Setting %s=%s in bootloader environment", name, value); > - if ((ret = libebg.env_setglobalstate(&ebgenv, *value - '0')) != 0) { > - ERROR("Cannot set %s=%s in bootloader environment.", STATE_KEY, value); > - } > - break; > - default: > - ret = -EINVAL; > - ERROR("Unsupported bootloader environment assignment %s=%s.", STATE_KEY, value); > - } > - } else { > - /* A new environment is created if EFI Boot Guard's > - * representation of SWUpdate's recovery_status is > - * not in_progress. */ > - DEBUG("Setting %s=%s in bootloader environment", name, value); > - if ((ret = libebg.env_create_new(&ebgenv)) != 0) { > - ERROR("Cannot open/create new bootloader environment: %s.", > - strerror(ret)); > - return ret; > - } > - if ((ret = libebg.env_set(&ebgenv, (char *)name, (char *)value)) != 0) { > - ERROR("Cannot set %s=%s in bootloader environment: %s.", > - name, value, strerror(ret)); > - } > + char *value = malloc(size); > + if (value == NULL) { > + ERROR("Error allocating memory"); > + return NULL; > + } > + > + int result = libebg.env_get(&ebgenv, (char *)name, value); > + if (result != 0) { > + ERROR("Cannot get %s: %s", name, strerror(-result)); > + free(value); > + return NULL; > + } > + return value; > +} > + > +/* Note: EFI Boot Guard Environment integers are at most uint32_t. */ > +static inline uint32_t _env_to_uint32(char *value) > +{ > + if (!value) { > + return UINT_MAX; > } > - (void)libebg.env_close(&ebgenv); > + errno = 0; > + uint32_t result = strtoul(value, NULL, 10); > + free(value); > + return errno != 0 ? UINT_MAX : result; > +} > > - return ret; > +static inline uint8_t ascii_to_uint8(unsigned char value) > +{ > + return value - '0'; > } > > -static int do_env_unset(const char *name) > +static inline unsigned char uint8_to_ascii(uint8_t value) > { > - int ret; > + return value + '0'; > +} > > +static char *do_env_get(const char *name) > +{ > + errno = 0; > libebg.beverbose(&ebgenv, loglevel > INFOLEVEL ? true : false); > > - if ((ret = libebg.env_open_current(&ebgenv)) != 0) { > - ERROR("Cannot open current bootloader environment: %s.", strerror(ret)); > - return ret; > + int result = libebg.env_open_current(&ebgenv); > + if (result != 0) { > + ERROR("Cannot open bootloader environment: %s", strerror(result)); > + return NULL; > } > > - DEBUG("Unsetting %s in bootloader environment", name); > - if (strncmp(name, BOOTVAR_TRANSACTION, strlen(name) + 1) == 0) { > - ret = libebg.env_finalize_update(&ebgenv); > - if (ret) { > - ERROR("Cannot unset %s in bootloader environment: %s.", BOOTVAR_TRANSACTION, strerror(ret)); > + if (!inflight && is(name, EBGENV_USTATE)) { > + /* > + * When not in an in-flight transaction, get the "global significant" > + * EBGENV_USTATE value: > + * If rolled-back and successfully booted, there's an alternative > + * boot path that has > + * EBGENV_REVISION == 0 and > + * EBGENV_USTATE == STATE_FAILED > + * which is how EFI Boot Guard encodes a rolled-back condition. > + * To act on this condition, e.g., report and clear it, STATE_FAILED > + * as the global significant EBGENV_USTATE value is returned. > + * > + * If not rolled-back, the current boot path's EBGENV_USTATE value > + * is returned. > + */ > + char *value = NULL; > + if (asprintf(&value, "%u", libebg.env_getglobalstate(&ebgenv)) == -1) { > + ERROR("Error allocating memory"); > + return NULL; > } > - } else if (strncmp(name, (char *)STATE_KEY, strlen((char *)STATE_KEY) + 1) == 0) { > - /* Unsetting STATE_KEY is semantically equivalent to setting it to STATE_OK. */ > - if ((ret = libebg.env_setglobalstate(&ebgenv, STATE_OK - '0')) != 0) { > - ERROR("Cannot unset %s in bootloader environment.", STATE_KEY); > - } > - } else { > - ret = libebg.env_set_ex(&ebgenv, (char *)name, USERVAR_TYPE_DELETED, (uint8_t *)"", 1); > + return value; > } > - (void)libebg.env_close(&ebgenv); > > - return ret; > + return _env_get(name); > } > > -static char *do_env_get(const char *name) > +static int create_new_environment(void) > { > - char *value = NULL; > - size_t size; > + uint32_t revision = _env_to_uint32(_env_get(EBGENV_REVISION)); > + uint8_t in_progress = (uint8_t)_env_to_uint32(_env_get(EBGENV_IN_PROGRESS)); > + if ((revision == UINT_MAX) || (in_progress == UINT8_MAX)) { > + ERROR("Cannot get environment revision or in-progress marker"); > + return -EIO; > + } > + if (in_progress == 1) { > + return 0; > + } > + int result = libebg.env_create_new(&ebgenv); > + if (result != 0) { > + ERROR("Cannot create new environment revision: %s", strerror(result)); > + return -result; > + } > + /* > + * libebgenv has now set: > + * EBG_ENVDATA->in_progress = 1 > + * EBG_ENVDATA->revision = <current boot path's revision>++ > + */ > + uint32_t new_revision = _env_to_uint32(_env_get(EBGENV_REVISION)); > + if (new_revision == UINT_MAX) { > + return -EIO; > + } > + if (++revision != new_revision) { > + ERROR("No new environment revision was created!"); > + return -ENOENT; > + } > + inflight = true; > + DEBUG("Created new environment revision %d, starting transaction", > + new_revision); > + return 0; > +} > > +static int do_env_set(const char *name, const char *value) > +{ > errno = 0; > libebg.beverbose(&ebgenv, loglevel > INFOLEVEL ? true : false); > > - int ret; > - if ((ret = libebg.env_open_current(&ebgenv)) != 0) { > - ERROR("Cannot open current bootloader environment: %s.", > - strerror(ret)); > - return NULL; > + if (!inflight) { > + /* > + * Without an in-flight transaction, only allow > + * (1) starting a transaction or > + * (2) acknowledging an update. > + */ > + if (!is(name, BOOTVAR_TRANSACTION) && !is(name, EBGENV_USTATE)) { > + ERROR("Not setting %s=%s w/o in-flight transaction", > + name, value); > + return -EINVAL; > + } > + if (is(name, BOOTVAR_TRANSACTION) && > + !is(value, get_state_string(STATE_IN_PROGRESS))) { > + ERROR("Not setting %s=%s w/o in-flight transaction", > + name, value); > + return -EINVAL; > + } > + > + if (is(name, EBGENV_USTATE)) { > + switch (*value) { > + case STATE_OK: /* Acknowledging an update. */ > + case STATE_IN_PROGRESS: /* Starting a transaction. */ > + break; > + default: > + ERROR("Not setting %s=%s w/o in-flight transaction", > + name, value); > + return -EINVAL; > + } > + } > + } > + > + int result = libebg.env_open_current(&ebgenv); > + if (result != 0) { > + ERROR("Cannot open bootloader environment: %s", strerror(result)); > + return -result; > } > > - if (strncmp(name, (char *)STATE_KEY, strlen((char *)STATE_KEY) + 1) == 0) { > - value = (char *)malloc(sizeof(char)); > - *value = libebg.env_getglobalstate(&ebgenv); > - /* Map EFI Boot Guard's int return to update_state_t's char value */ > - *value = *value + '0'; > - } else { > - if ((size = libebg.env_get(&ebgenv, (char *)name, NULL)) != 0) { > - value = malloc(size); > - if (value) { > - if (libebg.env_get(&ebgenv, (char *)name, value) != 0) { > - value = NULL; > + if (is(name, BOOTVAR_TRANSACTION)) { > + /* > + * Note: This gets called by core/stream_interface.c's > + * update_transaction_state() with the update > + * state's string representation. > + */ > + if (is(value, get_state_string(STATE_IN_PROGRESS))) { > + return create_new_environment(); > + } > + > + if (is(value, get_state_string(STATE_FAILED)) || > + is(value, get_state_string(STATE_INSTALLED)) || > + is(value, get_state_string(STATE_OK))) { > + /* > + * Irrespective of the value, set EBGENV_IN_PROGRESS = 0, > + * else EFI Boot Guard will *NOT* consider this environment > + * for booting at all. > + */ > + if ((result = libebg.env_set(&ebgenv, EBGENV_IN_PROGRESS, > + (char *)"0")) != 0) { > + ERROR("Error setting %s=0: %s", EBGENV_IN_PROGRESS, > + strerror(-result)); > + return result; > + } > + return 0; > + } > + > + /* Fall-through for invalid EBGENV_IN_PROGRESS values. */ > + ERROR("Unsupported setting %s=%s", EBGENV_IN_PROGRESS, value); > + return -EINVAL; > + } > + > + if (!is(name, EBGENV_USTATE)) { > + if ((result = libebg.env_set(&ebgenv, (char *)name, (char *)value)) != 0) { > + ERROR("Error setting %s=%s: %s", name, value, strerror(-result)); > + return result; > + } > + return 0; > + } > + > + switch (*value) { > + case STATE_IN_PROGRESS: > + return create_new_environment(); > + case STATE_OK: > + if (inflight) { > + /* > + * Environment modification within the in-flight transaction, > + * i.e., the in-memory working copy, just set it. > + */ > + if ((result = libebg.env_set(&ebgenv, EBGENV_USTATE, > + (char *)value)) != 0) { > + ERROR("Error setting %s=%s: %s", EBGENV_USTATE, > + get_state_string(STATE_OK), > + strerror(-result)); > + return result; > + } > + return 0; > + } > + > + unsigned char global_ustate = uint8_to_ascii((uint8_t) > + libebg.env_getglobalstate(&ebgenv)); > + if (global_ustate == STATE_NOT_AVAILABLE) { > + ERROR("Cannot read global %s", EBGENV_USTATE); > + return -EIO; > + } > + unsigned char current_ustate = uint8_to_ascii((uint8_t) > + _env_to_uint32( > + _env_get(EBGENV_USTATE))); > + if (!is_valid_state(current_ustate)) { > + ERROR("Cannot read current %s", EBGENV_USTATE); > + return -EIO; > + } > + > + if (global_ustate == STATE_FAILED) { > + TRACE("Found rolled-back condition, clearing marker"); > + /* > + * Clear rolled-back condition by setting > + * EBGENV_USTATE = STATE_OK on all alternative > + * boot paths having EBGENV_USTATE != STATE_OK > + * and write them to disk. > + * Note: Does not write to the current boot path (to > + * which was rolled-back to). > + */ > + if ((result = libebg.env_setglobalstate(&ebgenv, > + ascii_to_uint8(STATE_OK))) != 0) { > + ERROR("Error resetting failure condition: %s", > + strerror(-result)); > + return result; > + } > + /* > + * Restore prior current boot path's EBGENV_USTATE value > + * (as there's no way to reload it from disk). > + * Should be STATE_OK anyway but better play it safe... > + */ > + if (current_ustate != STATE_OK) { > + if ((result = libebg.env_set(&ebgenv, EBGENV_USTATE, > + (char[2]){ current_ustate, '\0' })) != 0) { > + ERROR("Error restoring %s: %s", EBGENV_USTATE, > + strerror(-result)); > + return result; > } > } > + return 0; > + } > + > + if (current_ustate == STATE_TESTING) { > + TRACE("Found successful update, blessing it"); > + /* > + * Acknowledge, update the /current/ boot path on disk. > + */ > + if ((result = libebg.env_set(&ebgenv, EBGENV_USTATE, > + (char *)value)) != 0) { > + ERROR("Error setting %s=%s: %s", EBGENV_USTATE, > + value, strerror(-result)); > + return result; > + } > + > + if ((result = libebg.env_close(&ebgenv)) != 0) { > + ERROR("Error persisting environment: %s", > + strerror(result)); > + return -result; > + } > + return 0; > } > + > + WARN("Unsupported state for setting %s=%s", EBGENV_USTATE, > + get_state_string(*value)); > + return -EINVAL; > + case STATE_INSTALLED: > + if ((result = libebg.env_finalize_update(&ebgenv)) != 0) { > + ERROR("Error finalizing environment: %s", strerror(result)); > + return -result; > + } > + /* > + * libebgenv has now set: > + * EBG_ENVDATA->in_progress = 0 > + * EBG_ENVDATA->ustate = USTATE_INSTALLED. > + * Now, persist the in-memory working copy environment as new > + * current boot path and terminate the transaction. > + * Note: Does write to an alternative boot path "upcycled" to > + * the new current boot path; Does *NOT* write to the current > + * boot path. > + */ > + if ((result = libebg.env_close(&ebgenv)) != 0) { > + ERROR("Error persisting environment: %s", strerror(result)); > + return -result; > + } > + inflight = false; > + return 0; > + case STATE_FAILED: > + /* > + * SWUpdate sets this if the installation has failed. In this case, > + * the transaction is simply not committed, so nothing to do. > + */ > + return 0; > + default: > + break; > } > > - (void)libebg.env_close(&ebgenv); > + /* > + * Fall-through for invalid or EBGENV_USTATE values handled > + * by EFI Boot Guard internally. > + */ > + WARN("Unsupported setting %s=%s", EBGENV_USTATE, get_state_string(*value)); > + return -EINVAL; > +} > + > +static int do_env_unset(const char *name) > +{ > + errno = 0; > + libebg.beverbose(&ebgenv, loglevel > INFOLEVEL ? true : false); > > - if (value == NULL) { > - ERROR("Cannot get %s from bootloader environment: %s", > - name, strerror(errno)); > + int result = libebg.env_open_current(&ebgenv); > + if (result != 0) { > + ERROR("Cannot open bootloader environment: %s", strerror(result)); > + return -result; > } > > - return value; > + if (is(name, EBGENV_USTATE)) { > + /* > + * Unsetting EBGENV_USTATE is semantically equivalent to > + * setting EBGENV_USTATE = STATE_OK. > + */ > + return do_env_set(EBGENV_USTATE, (char[2]){ STATE_OK, '\0' }); > + } > + > + if (is(name, BOOTVAR_TRANSACTION)) { > + /* > + * Unsetting BOOTVAR_TRANSACTION is semantically equivalent to > + * setting EBGENV_IN_PROGRESS = 0. > + */ > + return do_env_set(EBGENV_IN_PROGRESS, (char *)"0"); > + } > + > + if ((result = libebg.env_set_ex(&ebgenv, (char *)name, USERVAR_TYPE_DELETED, > + (uint8_t *)"", 1)) != 0) { > + ERROR("Error unsetting %s: %s", name, strerror(-result)); > + return result; > + } > + return 0; > } > > static int do_apply_list(const char *filename) > { > - FILE *fp = NULL; > - char *line = NULL; > - char *key; > - char *value; > - size_t len = 0; > - int ret = 0; > - > errno = 0; > libebg.beverbose(&ebgenv, loglevel > INFOLEVEL ? true : false); > > - if (!(fp = fopen(filename, "rb"))) { > - ERROR("Failed to open bootloader environment file %s: %s", > - filename, strerror(errno)); > - return -1; > + FILE *file = fopen(filename, "rb"); > + if (!file) { > + ERROR("Cannot open bootloader environment source file %s: %s", > + filename, strerror(errno)); > + return -EIO; > } > > - while ((getline(&line, &len, fp)) != -1) { > - key = strtok(line, "="); > - value = strtok(NULL, "\t\n"); > - if (value != NULL && key != NULL) { > - if ((ret = do_env_set(key, value)) != 0) { > + char *line = NULL; > + size_t length = 0; > + int result = 0; > + while ((getline(&line, &length, file)) != -1) { > + char *key = strtok(line, "="); > + char *value = strtok(NULL, "\t\n"); > + if (key != NULL && value != NULL) { > + if ((result = do_env_set(key, value)) != 0) { > break; > } > } > } > > - if (fp) { > - fclose(fp); > - } > - if (line) { > - free(line); > - } > - return ret; > + fclose(file); > + free(line); > + return result; > } > > static bootloader ebg = { > @@ -212,9 +509,15 @@ static bootloader ebg = { > .apply_list = &do_apply_list > }; > > -static bootloader* probe(void) > +static bootloader *probe(void) > { > - void* handle = dlopen("libebgenv.so.0", RTLD_NOW | RTLD_GLOBAL); > + if (!is(STATE_KEY, EBGENV_USTATE)) { > + ERROR("CONFIG_UPDATE_STATE_BOOTLOADER=%s is required for " > + "EFI Boot Guard support", EBGENV_USTATE); > + return NULL; > + } > + Reason is here due to the introduced check - our test in CI does not foresee a valid environment. I do not see the ERROR as well, I think because bootloader are loaded before initializing the tracing inside SWUpdate. Anyway, with this code: [TRACE] : SWUPDATE running : [print_registered_bootloaders] : ebg shared lib not found. Could you check ? > + void *handle = dlopen("libebgenv.so.0", RTLD_NOW | RTLD_GLOBAL); > if (!handle) { > return NULL; > } > diff --git a/core/stream_interface.c b/core/stream_interface.c > index 81c26c3..9ba6332 100644 > --- a/core/stream_interface.c > +++ b/core/stream_interface.c > @@ -611,6 +611,40 @@ void *network_initializer(void *data) > if (!(inst.fd < 0)) > close(inst.fd); > > + if (!software->parms.dry_run && is_bootloader(BOOTLOADER_EBG)) { > + if (!software->bootloader_transaction_marker) { > + /* > + * EFI Boot Guard's "in_progress" environment variable > + * has special semantics hard-coded, hence the > + * bootloader transaction marker cannot be disabled. > + */ > + TRACE("Note: Setting EFI Boot Guard's 'in_progress' " > + "environment variable cannot be disabled."); > + } > + if (!software->bootloader_state_marker) { > + /* > + * With a disabled update state marker, there's no > + * transaction auto-commit via > + * save_state(STATE_INSTALLED) > + * which effectively calls > + * bootloader_env_set(STATE_KEY, STATE_INSTALLED). > + * Hence, manually calling save_state(STATE_INSTALLED) > + * or equivalent is required to commit the transaction. > + * This can be useful to, e.g., terminate the transaction > + * from an according progress interface client or an > + * SWUpdate suricatta module after it has received an > + * update activation request from the remote server. > + */ > + TRACE("Note: EFI Boot Guard environment transaction " > + "will not be auto-committed."); > + } > + if (!software->bootloader_transaction_marker && > + !software->bootloader_state_marker) { > + WARN("EFI Boot Guard environment modifications will " > + "not be persisted."); > + } > + } > + So all this code is just to track that variable is not stored persistently. > /* do carry out the installation (flash programming) */ > if (ret == 0) { > TRACE("Valid image found: copying to FLASH"); Best regards, Stefano
Hi Stefano, > CI fails, see: > > https://source.denx.de/swupdate/swupdate/-/pipelines/14326 Sorry, my bad, see below. > On 28.11.22 12:23, Christian Storm wrote: > > Completely rework the EFI Boot Guard Binding accommodating to > > libebgenv's implicit assumptions and actions. To this end, > > environment modification transactions are introduced. > > The assumptions and rationale is documented and in addition > > log messages are emitted for particular combinations of > > bootloader_transaction_marker and bootloader_state_marker. > > > > Signed-off-by: Christian Storm <christian.storm@siemens.com> > > --- > > bootloader/ebg.c | 567 ++++++++++++++++++++++++++++++---------- > > core/stream_interface.c | 34 +++ > > 2 files changed, 469 insertions(+), 132 deletions(-) > > > > diff --git a/bootloader/ebg.c b/bootloader/ebg.c > > index 4194a38..99ee40b 100644 > > --- a/bootloader/ebg.c > > +++ b/bootloader/ebg.c > > @@ -1,22 +1,22 @@ > > /* > > * Author: Christian Storm > > - * Author: Andreas Reichel > > - * Copyright (C) 2018, Siemens AG > > + * Copyright (C) 2022, Siemens AG > > * > > * SPDX-License-Identifier: GPL-2.0-only > > */ > > +#include <stdint.h> > > #include <unistd.h> > > #include <stdlib.h> > > #include <limits.h> > > #include <stdbool.h> > > #include <stdio.h> > > #include <string.h> > > -#include <util.h> > > +#include <dlfcn.h> > > #include <efibootguard/ebgenv.h> > > -#include <generated/autoconf.h> > > -#include <state.h> > > -#include "dlfcn.h" > > +#include "generated/autoconf.h" > > +#include "util.h" > > +#include "state.h" > > #include "bootloader.h" > > static struct { > > @@ -25,184 +25,481 @@ static struct { > > int (*env_open_current)(ebgenv_t *e); > > int (*env_get)(ebgenv_t *e, char *key, char* buffer); > > int (*env_set)(ebgenv_t *e, char *key, char *value); > > - int (*env_set_ex)(ebgenv_t *e, char *key, uint64_t datatype, uint8_t *value, uint32_t datalen); > > + int (*env_set_ex)(ebgenv_t *e, char *key, uint64_t datatype, > > + uint8_t *value, uint32_t datalen); > > uint16_t (*env_getglobalstate)(ebgenv_t *e); > > int (*env_setglobalstate)(ebgenv_t *e, uint16_t ustate); > > int (*env_close)(ebgenv_t *e); > > int (*env_finalize_update)(ebgenv_t *e); > > } libebg; > > -static ebgenv_t ebgenv = {0}; > > -static int do_env_set(const char *name, const char *value) > > +/* > > + * ---------------------------------------------------------------------------- > > + * | Logics, Assumptions & Rationale | > > + * ---------------------------------------------------------------------------- > > + * > > + * EFI Boot Guard boots the environment having `EBGENV_IN_PROGRESS == 0` > > + * and the highest revision number. If multiple environments have the highest > > + * revision number, environment probing order is decisive. > > + * This environment is called the *current boot path*. > > + * Sorted descending on revision numbers and arbitrated by probing order, the > > + * other environments are termed *alternative boot paths*. > > + * > > + * Environment modifications ― except blessing a successful update ― must not > > + * touch the current boot path. Instead, a new boot path is created by > > + * "upcycling" the least recent alternative boot path. > > + * More specifically, environment modifications are captured in a *transaction*: > > + * An in-memory working copy of the current boot path environment is created > > + * which has a by one incremented higher revision than the current boot path. > > + * Modifications are performed on this working copy environment. > > + * When committing the transaction, i.e., writing it to disk, the new current > > + * boot path is persisted and booted next. > > + * > > + * A transaction is started by setting > > + * `EBGENV_USTATE = STATE_IN_PROGRESS` or > > + * `BOOTVAR_TRANSACTION = STATE_IN_PROGRESS` > > + * which is idempotent. Then, `libebgenv` sets > > + * - `EBGENV_IN_PROGRESS = 1` and > > + * - `EBGENV_REVISION` to the current boot path's revision plus one, and > > + * - the transaction `inflight` marker is set to `true`. > > + * > > + * A transaction is committed when setting `EBGENV_USTATE = STATE_INSTALLED`. > > + * Then, `libebgenv` sets > > + * - `EBGENV_IN_PROGRESS = 0` and > > + * - `EBGENV_USTATE = USTATE_INSTALLED`, > > + * - the new current boot path is persisted to disk, and > > + * - the transaction `inflight` marker is reset to `false`. > > + * With this, the current boot path becomes the most recent alternative boot > > + * path serving as rollback boot path if the new current boot path fails to > > + * boot. In this case, the failed boot path is marked with > > + * - `EBGENV_USTATE = USTATE_FAILED` and > > + * - `EBGENV_REVISION = 0` > > + * by which the rollback boot path becomes the current boot path (again). > > + * If the new current boot path boots successfully, the /current/ boot path > > + * needs to be written to acknowledge the successful update via setting > > + * `EBGENV_USTATE = USTATE_OK`. > > + * > > + * Note: The modification of EFI Boot Guard's environment variable > > + * `EBGENV_IN_PROGRESS` cannot be disabled as it's hard-wired in EFI > > + * Boot Guard with particular semantics. > > + * Note: Successive calls to libebgenv's `env_open_current()` after the first > > + * invocation are no-ops and select the in-memory working copy's current > > + * environment. > > + * Note: libebgenv's `env_close()` first *writes* to the current environment and > > + * then closes it. There's currently no way to just close it, e.g., for re- > > + * loading the environment from disk. > > + * > > + */ > > + > > + > > +/* EFI Boot Guard hard-coded environment variable names. */ > > +#define EBGENV_IN_PROGRESS (char *)"in_progress" > > +#define EBGENV_REVISION (char *)"revision" > > +#define EBGENV_USTATE (char *)"ustate" > > + > > +static ebgenv_t ebgenv = { 0 }; > > +static bool inflight = false; > > + > > +static inline bool is(const char *s1, const char *s2) > > { > > - int ret; > > + return strcmp(s1, s2) == 0; > > +} > > - errno = 0; > > - libebg.beverbose(&ebgenv, loglevel > INFOLEVEL ? true : false); > > +static char *_env_get(const char *name) > > +{ > > + /* > > + * libebgenv's env_get() is two-staged: The first call yields the > > + * value's size in bytes, the second call, with an accordingly > > + * sized buffer, yields the actual value. > > + */ > > + size_t size = libebg.env_get(&ebgenv, (char *)name, NULL); > > + if (size == 0) { > > + WARN("Cannot find key %s", name); > > + return NULL; > > + } > > - if ((ret = libebg.env_open_current(&ebgenv)) != 0) { > > - ERROR("Cannot open current bootloader environment: %s.", strerror(ret)); > > - return ret; > > - } > > - > > - if (strncmp(name, BOOTVAR_TRANSACTION, strlen(name) + 1) == 0 && > > - strncmp(value, get_state_string(STATE_IN_PROGRESS), > > - strlen(get_state_string(STATE_IN_PROGRESS)) + 1) == 0) { > > - DEBUG("Setting %s=%s in bootloader environment", name, value); > > - /* Open or create a new environment to reflect > > - * EFI Boot Guard's representation of SWUpdate's > > - * recovery_status=in_progress. */ > > - if ((ret = libebg.env_create_new(&ebgenv)) != 0) { > > - ERROR("Cannot open/create new bootloader environment: %s.", > > - strerror(ret)); > > - } > > - } else if (strncmp(name, (char *)STATE_KEY, strlen((char *)STATE_KEY) + 1) == 0) { > > - /* Map update_state_t to EFI Boot Guard's API. */ > > - switch (*value) { > > - case STATE_IN_PROGRESS: > > - case STATE_FAILED: > > - case STATE_TESTING: > > - /* Fall-through for update_state_t values destined either > > - * for BOOTVAR_TRANSACTION or handled by EBG internally. */ > > - break; > > - case STATE_OK: > > - case STATE_INSTALLED: > > - DEBUG("Setting %s=%s in bootloader environment", name, value); > > - if ((ret = libebg.env_setglobalstate(&ebgenv, *value - '0')) != 0) { > > - ERROR("Cannot set %s=%s in bootloader environment.", STATE_KEY, value); > > - } > > - break; > > - default: > > - ret = -EINVAL; > > - ERROR("Unsupported bootloader environment assignment %s=%s.", STATE_KEY, value); > > - } > > - } else { > > - /* A new environment is created if EFI Boot Guard's > > - * representation of SWUpdate's recovery_status is > > - * not in_progress. */ > > - DEBUG("Setting %s=%s in bootloader environment", name, value); > > - if ((ret = libebg.env_create_new(&ebgenv)) != 0) { > > - ERROR("Cannot open/create new bootloader environment: %s.", > > - strerror(ret)); > > - return ret; > > - } > > - if ((ret = libebg.env_set(&ebgenv, (char *)name, (char *)value)) != 0) { > > - ERROR("Cannot set %s=%s in bootloader environment: %s.", > > - name, value, strerror(ret)); > > - } > > + char *value = malloc(size); > > + if (value == NULL) { > > + ERROR("Error allocating memory"); > > + return NULL; > > + } > > + > > + int result = libebg.env_get(&ebgenv, (char *)name, value); > > + if (result != 0) { > > + ERROR("Cannot get %s: %s", name, strerror(-result)); > > + free(value); > > + return NULL; > > + } > > + return value; > > +} > > + > > +/* Note: EFI Boot Guard Environment integers are at most uint32_t. */ > > +static inline uint32_t _env_to_uint32(char *value) > > +{ > > + if (!value) { > > + return UINT_MAX; > > } > > - (void)libebg.env_close(&ebgenv); > > + errno = 0; > > + uint32_t result = strtoul(value, NULL, 10); > > + free(value); > > + return errno != 0 ? UINT_MAX : result; > > +} > > - return ret; > > +static inline uint8_t ascii_to_uint8(unsigned char value) > > +{ > > + return value - '0'; > > } > > -static int do_env_unset(const char *name) > > +static inline unsigned char uint8_to_ascii(uint8_t value) > > { > > - int ret; > > + return value + '0'; > > +} > > +static char *do_env_get(const char *name) > > +{ > > + errno = 0; > > libebg.beverbose(&ebgenv, loglevel > INFOLEVEL ? true : false); > > - if ((ret = libebg.env_open_current(&ebgenv)) != 0) { > > - ERROR("Cannot open current bootloader environment: %s.", strerror(ret)); > > - return ret; > > + int result = libebg.env_open_current(&ebgenv); > > + if (result != 0) { > > + ERROR("Cannot open bootloader environment: %s", strerror(result)); > > + return NULL; > > } > > - DEBUG("Unsetting %s in bootloader environment", name); > > - if (strncmp(name, BOOTVAR_TRANSACTION, strlen(name) + 1) == 0) { > > - ret = libebg.env_finalize_update(&ebgenv); > > - if (ret) { > > - ERROR("Cannot unset %s in bootloader environment: %s.", BOOTVAR_TRANSACTION, strerror(ret)); > > + if (!inflight && is(name, EBGENV_USTATE)) { > > + /* > > + * When not in an in-flight transaction, get the "global significant" > > + * EBGENV_USTATE value: > > + * If rolled-back and successfully booted, there's an alternative > > + * boot path that has > > + * EBGENV_REVISION == 0 and > > + * EBGENV_USTATE == STATE_FAILED > > + * which is how EFI Boot Guard encodes a rolled-back condition. > > + * To act on this condition, e.g., report and clear it, STATE_FAILED > > + * as the global significant EBGENV_USTATE value is returned. > > + * > > + * If not rolled-back, the current boot path's EBGENV_USTATE value > > + * is returned. > > + */ > > + char *value = NULL; > > + if (asprintf(&value, "%u", libebg.env_getglobalstate(&ebgenv)) == -1) { > > + ERROR("Error allocating memory"); > > + return NULL; > > } > > - } else if (strncmp(name, (char *)STATE_KEY, strlen((char *)STATE_KEY) + 1) == 0) { > > - /* Unsetting STATE_KEY is semantically equivalent to setting it to STATE_OK. */ > > - if ((ret = libebg.env_setglobalstate(&ebgenv, STATE_OK - '0')) != 0) { > > - ERROR("Cannot unset %s in bootloader environment.", STATE_KEY); > > - } > > - } else { > > - ret = libebg.env_set_ex(&ebgenv, (char *)name, USERVAR_TYPE_DELETED, (uint8_t *)"", 1); > > + return value; > > } > > - (void)libebg.env_close(&ebgenv); > > - return ret; > > + return _env_get(name); > > } > > -static char *do_env_get(const char *name) > > +static int create_new_environment(void) > > { > > - char *value = NULL; > > - size_t size; > > + uint32_t revision = _env_to_uint32(_env_get(EBGENV_REVISION)); > > + uint8_t in_progress = (uint8_t)_env_to_uint32(_env_get(EBGENV_IN_PROGRESS)); > > + if ((revision == UINT_MAX) || (in_progress == UINT8_MAX)) { > > + ERROR("Cannot get environment revision or in-progress marker"); > > + return -EIO; > > + } > > + if (in_progress == 1) { > > + return 0; > > + } > > + int result = libebg.env_create_new(&ebgenv); > > + if (result != 0) { > > + ERROR("Cannot create new environment revision: %s", strerror(result)); > > + return -result; > > + } > > + /* > > + * libebgenv has now set: > > + * EBG_ENVDATA->in_progress = 1 > > + * EBG_ENVDATA->revision = <current boot path's revision>++ > > + */ > > + uint32_t new_revision = _env_to_uint32(_env_get(EBGENV_REVISION)); > > + if (new_revision == UINT_MAX) { > > + return -EIO; > > + } > > + if (++revision != new_revision) { > > + ERROR("No new environment revision was created!"); > > + return -ENOENT; > > + } > > + inflight = true; > > + DEBUG("Created new environment revision %d, starting transaction", > > + new_revision); > > + return 0; > > +} > > +static int do_env_set(const char *name, const char *value) > > +{ > > errno = 0; > > libebg.beverbose(&ebgenv, loglevel > INFOLEVEL ? true : false); > > - int ret; > > - if ((ret = libebg.env_open_current(&ebgenv)) != 0) { > > - ERROR("Cannot open current bootloader environment: %s.", > > - strerror(ret)); > > - return NULL; > > + if (!inflight) { > > + /* > > + * Without an in-flight transaction, only allow > > + * (1) starting a transaction or > > + * (2) acknowledging an update. > > + */ > > + if (!is(name, BOOTVAR_TRANSACTION) && !is(name, EBGENV_USTATE)) { > > + ERROR("Not setting %s=%s w/o in-flight transaction", > > + name, value); > > + return -EINVAL; > > + } > > + if (is(name, BOOTVAR_TRANSACTION) && > > + !is(value, get_state_string(STATE_IN_PROGRESS))) { > > + ERROR("Not setting %s=%s w/o in-flight transaction", > > + name, value); > > + return -EINVAL; > > + } > > + > > + if (is(name, EBGENV_USTATE)) { > > + switch (*value) { > > + case STATE_OK: /* Acknowledging an update. */ > > + case STATE_IN_PROGRESS: /* Starting a transaction. */ > > + break; > > + default: > > + ERROR("Not setting %s=%s w/o in-flight transaction", > > + name, value); > > + return -EINVAL; > > + } > > + } > > + } > > + > > + int result = libebg.env_open_current(&ebgenv); > > + if (result != 0) { > > + ERROR("Cannot open bootloader environment: %s", strerror(result)); > > + return -result; > > } > > - if (strncmp(name, (char *)STATE_KEY, strlen((char *)STATE_KEY) + 1) == 0) { > > - value = (char *)malloc(sizeof(char)); > > - *value = libebg.env_getglobalstate(&ebgenv); > > - /* Map EFI Boot Guard's int return to update_state_t's char value */ > > - *value = *value + '0'; > > - } else { > > - if ((size = libebg.env_get(&ebgenv, (char *)name, NULL)) != 0) { > > - value = malloc(size); > > - if (value) { > > - if (libebg.env_get(&ebgenv, (char *)name, value) != 0) { > > - value = NULL; > > + if (is(name, BOOTVAR_TRANSACTION)) { > > + /* > > + * Note: This gets called by core/stream_interface.c's > > + * update_transaction_state() with the update > > + * state's string representation. > > + */ > > + if (is(value, get_state_string(STATE_IN_PROGRESS))) { > > + return create_new_environment(); > > + } > > + > > + if (is(value, get_state_string(STATE_FAILED)) || > > + is(value, get_state_string(STATE_INSTALLED)) || > > + is(value, get_state_string(STATE_OK))) { > > + /* > > + * Irrespective of the value, set EBGENV_IN_PROGRESS = 0, > > + * else EFI Boot Guard will *NOT* consider this environment > > + * for booting at all. > > + */ > > + if ((result = libebg.env_set(&ebgenv, EBGENV_IN_PROGRESS, > > + (char *)"0")) != 0) { > > + ERROR("Error setting %s=0: %s", EBGENV_IN_PROGRESS, > > + strerror(-result)); > > + return result; > > + } > > + return 0; > > + } > > + > > + /* Fall-through for invalid EBGENV_IN_PROGRESS values. */ > > + ERROR("Unsupported setting %s=%s", EBGENV_IN_PROGRESS, value); > > + return -EINVAL; > > + } > > + > > + if (!is(name, EBGENV_USTATE)) { > > + if ((result = libebg.env_set(&ebgenv, (char *)name, (char *)value)) != 0) { > > + ERROR("Error setting %s=%s: %s", name, value, strerror(-result)); > > + return result; > > + } > > + return 0; > > + } > > + > > + switch (*value) { > > + case STATE_IN_PROGRESS: > > + return create_new_environment(); > > + case STATE_OK: > > + if (inflight) { > > + /* > > + * Environment modification within the in-flight transaction, > > + * i.e., the in-memory working copy, just set it. > > + */ > > + if ((result = libebg.env_set(&ebgenv, EBGENV_USTATE, > > + (char *)value)) != 0) { > > + ERROR("Error setting %s=%s: %s", EBGENV_USTATE, > > + get_state_string(STATE_OK), > > + strerror(-result)); > > + return result; > > + } > > + return 0; > > + } > > + > > + unsigned char global_ustate = uint8_to_ascii((uint8_t) > > + libebg.env_getglobalstate(&ebgenv)); > > + if (global_ustate == STATE_NOT_AVAILABLE) { > > + ERROR("Cannot read global %s", EBGENV_USTATE); > > + return -EIO; > > + } > > + unsigned char current_ustate = uint8_to_ascii((uint8_t) > > + _env_to_uint32( > > + _env_get(EBGENV_USTATE))); > > + if (!is_valid_state(current_ustate)) { > > + ERROR("Cannot read current %s", EBGENV_USTATE); > > + return -EIO; > > + } > > + > > + if (global_ustate == STATE_FAILED) { > > + TRACE("Found rolled-back condition, clearing marker"); > > + /* > > + * Clear rolled-back condition by setting > > + * EBGENV_USTATE = STATE_OK on all alternative > > + * boot paths having EBGENV_USTATE != STATE_OK > > + * and write them to disk. > > + * Note: Does not write to the current boot path (to > > + * which was rolled-back to). > > + */ > > + if ((result = libebg.env_setglobalstate(&ebgenv, > > + ascii_to_uint8(STATE_OK))) != 0) { > > + ERROR("Error resetting failure condition: %s", > > + strerror(-result)); > > + return result; > > + } > > + /* > > + * Restore prior current boot path's EBGENV_USTATE value > > + * (as there's no way to reload it from disk). > > + * Should be STATE_OK anyway but better play it safe... > > + */ > > + if (current_ustate != STATE_OK) { > > + if ((result = libebg.env_set(&ebgenv, EBGENV_USTATE, > > + (char[2]){ current_ustate, '\0' })) != 0) { > > + ERROR("Error restoring %s: %s", EBGENV_USTATE, > > + strerror(-result)); > > + return result; > > } > > } > > + return 0; > > + } > > + > > + if (current_ustate == STATE_TESTING) { > > + TRACE("Found successful update, blessing it"); > > + /* > > + * Acknowledge, update the /current/ boot path on disk. > > + */ > > + if ((result = libebg.env_set(&ebgenv, EBGENV_USTATE, > > + (char *)value)) != 0) { > > + ERROR("Error setting %s=%s: %s", EBGENV_USTATE, > > + value, strerror(-result)); > > + return result; > > + } > > + > > + if ((result = libebg.env_close(&ebgenv)) != 0) { > > + ERROR("Error persisting environment: %s", > > + strerror(result)); > > + return -result; > > + } > > + return 0; > > } > > + > > + WARN("Unsupported state for setting %s=%s", EBGENV_USTATE, > > + get_state_string(*value)); > > + return -EINVAL; > > + case STATE_INSTALLED: > > + if ((result = libebg.env_finalize_update(&ebgenv)) != 0) { > > + ERROR("Error finalizing environment: %s", strerror(result)); > > + return -result; > > + } > > + /* > > + * libebgenv has now set: > > + * EBG_ENVDATA->in_progress = 0 > > + * EBG_ENVDATA->ustate = USTATE_INSTALLED. > > + * Now, persist the in-memory working copy environment as new > > + * current boot path and terminate the transaction. > > + * Note: Does write to an alternative boot path "upcycled" to > > + * the new current boot path; Does *NOT* write to the current > > + * boot path. > > + */ > > + if ((result = libebg.env_close(&ebgenv)) != 0) { > > + ERROR("Error persisting environment: %s", strerror(result)); > > + return -result; > > + } > > + inflight = false; > > + return 0; > > + case STATE_FAILED: > > + /* > > + * SWUpdate sets this if the installation has failed. In this case, > > + * the transaction is simply not committed, so nothing to do. > > + */ > > + return 0; > > + default: > > + break; > > } > > - (void)libebg.env_close(&ebgenv); > > + /* > > + * Fall-through for invalid or EBGENV_USTATE values handled > > + * by EFI Boot Guard internally. > > + */ > > + WARN("Unsupported setting %s=%s", EBGENV_USTATE, get_state_string(*value)); > > + return -EINVAL; > > +} > > + > > +static int do_env_unset(const char *name) > > +{ > > + errno = 0; > > + libebg.beverbose(&ebgenv, loglevel > INFOLEVEL ? true : false); > > - if (value == NULL) { > > - ERROR("Cannot get %s from bootloader environment: %s", > > - name, strerror(errno)); > > + int result = libebg.env_open_current(&ebgenv); > > + if (result != 0) { > > + ERROR("Cannot open bootloader environment: %s", strerror(result)); > > + return -result; > > } > > - return value; > > + if (is(name, EBGENV_USTATE)) { > > + /* > > + * Unsetting EBGENV_USTATE is semantically equivalent to > > + * setting EBGENV_USTATE = STATE_OK. > > + */ > > + return do_env_set(EBGENV_USTATE, (char[2]){ STATE_OK, '\0' }); > > + } > > + > > + if (is(name, BOOTVAR_TRANSACTION)) { > > + /* > > + * Unsetting BOOTVAR_TRANSACTION is semantically equivalent to > > + * setting EBGENV_IN_PROGRESS = 0. > > + */ > > + return do_env_set(EBGENV_IN_PROGRESS, (char *)"0"); > > + } > > + > > + if ((result = libebg.env_set_ex(&ebgenv, (char *)name, USERVAR_TYPE_DELETED, > > + (uint8_t *)"", 1)) != 0) { > > + ERROR("Error unsetting %s: %s", name, strerror(-result)); > > + return result; > > + } > > + return 0; > > } > > static int do_apply_list(const char *filename) > > { > > - FILE *fp = NULL; > > - char *line = NULL; > > - char *key; > > - char *value; > > - size_t len = 0; > > - int ret = 0; > > - > > errno = 0; > > libebg.beverbose(&ebgenv, loglevel > INFOLEVEL ? true : false); > > - if (!(fp = fopen(filename, "rb"))) { > > - ERROR("Failed to open bootloader environment file %s: %s", > > - filename, strerror(errno)); > > - return -1; > > + FILE *file = fopen(filename, "rb"); > > + if (!file) { > > + ERROR("Cannot open bootloader environment source file %s: %s", > > + filename, strerror(errno)); > > + return -EIO; > > } > > - while ((getline(&line, &len, fp)) != -1) { > > - key = strtok(line, "="); > > - value = strtok(NULL, "\t\n"); > > - if (value != NULL && key != NULL) { > > - if ((ret = do_env_set(key, value)) != 0) { > > + char *line = NULL; > > + size_t length = 0; > > + int result = 0; > > + while ((getline(&line, &length, file)) != -1) { > > + char *key = strtok(line, "="); > > + char *value = strtok(NULL, "\t\n"); > > + if (key != NULL && value != NULL) { > > + if ((result = do_env_set(key, value)) != 0) { > > break; > > } > > } > > } > > - if (fp) { > > - fclose(fp); > > - } > > - if (line) { > > - free(line); > > - } > > - return ret; > > + fclose(file); > > + free(line); > > + return result; > > } > > static bootloader ebg = { > > @@ -212,9 +509,15 @@ static bootloader ebg = { > > .apply_list = &do_apply_list > > }; > > -static bootloader* probe(void) > > +static bootloader *probe(void) > > { > > - void* handle = dlopen("libebgenv.so.0", RTLD_NOW | RTLD_GLOBAL); > > + if (!is(STATE_KEY, EBGENV_USTATE)) { > > + ERROR("CONFIG_UPDATE_STATE_BOOTLOADER=%s is required for " > > + "EFI Boot Guard support", EBGENV_USTATE); > > + return NULL; > > + } > > + > > Reason is here due to the introduced check - our test in CI does not foresee a > valid environment. I do not see the ERROR as well, I think because bootloader > are loaded before initializing the tracing inside SWUpdate. Anyway, with this > code: > > [TRACE] : SWUPDATE running : [print_registered_bootloaders] : ebg shared lib > not found. > > Could you check ? My bad, many thanks for analyzing! I reproduced the error and will look into it... The reason I introduced the check there is that I wanted to fail early in case this required assumption does not hold (and to not clutter the methods by having this check first in each of them). The failure reason is indeed that those checks run with the default bootloader which they don't find as it's not deployed. So, this should fix it (I will test it and submit a patch) as it uses the default NONE bootloader instead of a specific one: --- a/scripts/acceptance-tests/CheckImage.mk +++ b/scripts/acceptance-tests/CheckImage.mk @@ -6,7 +6,7 @@ # # test commands for --check command-line option # -SWU_CHECK_BASE = ./swupdate -l 5 -c $(if $(CONFIG_SIGALG_CMS),-k $(obj)/cacert.pem) $(if $(strip $(filter %.cfg, $^)), -f $(filter %.cfg, $^)) +SWU_CHECK_BASE = ./swupdate -B none -l 5 -c $(if $(CONFIG_SIGALG_CMS),-k $(obj)/cacert.pem) $(if $(strip $(filter %.cfg, $^)), -f $(filter %.cfg, $^)) SWU_CHECK = $(SWU_CHECK_BASE) $(if $(CONFIG_HW_COMPATIBILITY),-H test:1) $(if $(strip $(filter-out FORCE,$<)),-i $<) $(if $(strip $(KBUILD_VERBOSE:0=)),,>/dev/null 2>&1) quiet_cmd_swu_check_assert_false = RUN $@ > > + void *handle = dlopen("libebgenv.so.0", RTLD_NOW | RTLD_GLOBAL); > > if (!handle) { > > return NULL; > > } > > diff --git a/core/stream_interface.c b/core/stream_interface.c > > index 81c26c3..9ba6332 100644 > > --- a/core/stream_interface.c > > +++ b/core/stream_interface.c > > @@ -611,6 +611,40 @@ void *network_initializer(void *data) > > if (!(inst.fd < 0)) > > close(inst.fd); > > + if (!software->parms.dry_run && is_bootloader(BOOTLOADER_EBG)) { > > + if (!software->bootloader_transaction_marker) { > > + /* > > + * EFI Boot Guard's "in_progress" environment variable > > + * has special semantics hard-coded, hence the > > + * bootloader transaction marker cannot be disabled. > > + */ > > + TRACE("Note: Setting EFI Boot Guard's 'in_progress' " > > + "environment variable cannot be disabled."); > > + } > > + if (!software->bootloader_state_marker) { > > + /* > > + * With a disabled update state marker, there's no > > + * transaction auto-commit via > > + * save_state(STATE_INSTALLED) > > + * which effectively calls > > + * bootloader_env_set(STATE_KEY, STATE_INSTALLED). > > + * Hence, manually calling save_state(STATE_INSTALLED) > > + * or equivalent is required to commit the transaction. > > + * This can be useful to, e.g., terminate the transaction > > + * from an according progress interface client or an > > + * SWUpdate suricatta module after it has received an > > + * update activation request from the remote server. > > + */ > > + TRACE("Note: EFI Boot Guard environment transaction " > > + "will not be auto-committed."); > > + } > > + if (!software->bootloader_transaction_marker && > > + !software->bootloader_state_marker) { > > + WARN("EFI Boot Guard environment modifications will " > > + "not be persisted."); > > + } > > + } > > + > > So all this code is just to track that variable is not stored persistently. Yes, but that's just a means to an end: For one, it works around writing intermediate boot configurations which are not/might not be complete and hence may not boot correctly and second, it works around writing the environment(s) too often including unnecessarily touching the current boot path under some conditions. The whole idea is to capture modifications in a "transaction" and only flush to disk once a configuration is "complete". The same is true for acknowledging an update. This is what I tried to explain in the "Logics, Assumptions & Rationale" Section... Kind regards, Christian
diff --git a/bootloader/ebg.c b/bootloader/ebg.c index 4194a38..99ee40b 100644 --- a/bootloader/ebg.c +++ b/bootloader/ebg.c @@ -1,22 +1,22 @@ /* * Author: Christian Storm - * Author: Andreas Reichel - * Copyright (C) 2018, Siemens AG + * Copyright (C) 2022, Siemens AG * * SPDX-License-Identifier: GPL-2.0-only */ +#include <stdint.h> #include <unistd.h> #include <stdlib.h> #include <limits.h> #include <stdbool.h> #include <stdio.h> #include <string.h> -#include <util.h> +#include <dlfcn.h> #include <efibootguard/ebgenv.h> -#include <generated/autoconf.h> -#include <state.h> -#include "dlfcn.h" +#include "generated/autoconf.h" +#include "util.h" +#include "state.h" #include "bootloader.h" static struct { @@ -25,184 +25,481 @@ static struct { int (*env_open_current)(ebgenv_t *e); int (*env_get)(ebgenv_t *e, char *key, char* buffer); int (*env_set)(ebgenv_t *e, char *key, char *value); - int (*env_set_ex)(ebgenv_t *e, char *key, uint64_t datatype, uint8_t *value, uint32_t datalen); + int (*env_set_ex)(ebgenv_t *e, char *key, uint64_t datatype, + uint8_t *value, uint32_t datalen); uint16_t (*env_getglobalstate)(ebgenv_t *e); int (*env_setglobalstate)(ebgenv_t *e, uint16_t ustate); int (*env_close)(ebgenv_t *e); int (*env_finalize_update)(ebgenv_t *e); } libebg; -static ebgenv_t ebgenv = {0}; -static int do_env_set(const char *name, const char *value) +/* + * ---------------------------------------------------------------------------- + * | Logics, Assumptions & Rationale | + * ---------------------------------------------------------------------------- + * + * EFI Boot Guard boots the environment having `EBGENV_IN_PROGRESS == 0` + * and the highest revision number. If multiple environments have the highest + * revision number, environment probing order is decisive. + * This environment is called the *current boot path*. + * Sorted descending on revision numbers and arbitrated by probing order, the + * other environments are termed *alternative boot paths*. + * + * Environment modifications ― except blessing a successful update ― must not + * touch the current boot path. Instead, a new boot path is created by + * "upcycling" the least recent alternative boot path. + * More specifically, environment modifications are captured in a *transaction*: + * An in-memory working copy of the current boot path environment is created + * which has a by one incremented higher revision than the current boot path. + * Modifications are performed on this working copy environment. + * When committing the transaction, i.e., writing it to disk, the new current + * boot path is persisted and booted next. + * + * A transaction is started by setting + * `EBGENV_USTATE = STATE_IN_PROGRESS` or + * `BOOTVAR_TRANSACTION = STATE_IN_PROGRESS` + * which is idempotent. Then, `libebgenv` sets + * - `EBGENV_IN_PROGRESS = 1` and + * - `EBGENV_REVISION` to the current boot path's revision plus one, and + * - the transaction `inflight` marker is set to `true`. + * + * A transaction is committed when setting `EBGENV_USTATE = STATE_INSTALLED`. + * Then, `libebgenv` sets + * - `EBGENV_IN_PROGRESS = 0` and + * - `EBGENV_USTATE = USTATE_INSTALLED`, + * - the new current boot path is persisted to disk, and + * - the transaction `inflight` marker is reset to `false`. + * With this, the current boot path becomes the most recent alternative boot + * path serving as rollback boot path if the new current boot path fails to + * boot. In this case, the failed boot path is marked with + * - `EBGENV_USTATE = USTATE_FAILED` and + * - `EBGENV_REVISION = 0` + * by which the rollback boot path becomes the current boot path (again). + * If the new current boot path boots successfully, the /current/ boot path + * needs to be written to acknowledge the successful update via setting + * `EBGENV_USTATE = USTATE_OK`. + * + * Note: The modification of EFI Boot Guard's environment variable + * `EBGENV_IN_PROGRESS` cannot be disabled as it's hard-wired in EFI + * Boot Guard with particular semantics. + * Note: Successive calls to libebgenv's `env_open_current()` after the first + * invocation are no-ops and select the in-memory working copy's current + * environment. + * Note: libebgenv's `env_close()` first *writes* to the current environment and + * then closes it. There's currently no way to just close it, e.g., for re- + * loading the environment from disk. + * + */ + + +/* EFI Boot Guard hard-coded environment variable names. */ +#define EBGENV_IN_PROGRESS (char *)"in_progress" +#define EBGENV_REVISION (char *)"revision" +#define EBGENV_USTATE (char *)"ustate" + +static ebgenv_t ebgenv = { 0 }; +static bool inflight = false; + +static inline bool is(const char *s1, const char *s2) { - int ret; + return strcmp(s1, s2) == 0; +} - errno = 0; - libebg.beverbose(&ebgenv, loglevel > INFOLEVEL ? true : false); +static char *_env_get(const char *name) +{ + /* + * libebgenv's env_get() is two-staged: The first call yields the + * value's size in bytes, the second call, with an accordingly + * sized buffer, yields the actual value. + */ + size_t size = libebg.env_get(&ebgenv, (char *)name, NULL); + if (size == 0) { + WARN("Cannot find key %s", name); + return NULL; + } - if ((ret = libebg.env_open_current(&ebgenv)) != 0) { - ERROR("Cannot open current bootloader environment: %s.", strerror(ret)); - return ret; - } - - if (strncmp(name, BOOTVAR_TRANSACTION, strlen(name) + 1) == 0 && - strncmp(value, get_state_string(STATE_IN_PROGRESS), - strlen(get_state_string(STATE_IN_PROGRESS)) + 1) == 0) { - DEBUG("Setting %s=%s in bootloader environment", name, value); - /* Open or create a new environment to reflect - * EFI Boot Guard's representation of SWUpdate's - * recovery_status=in_progress. */ - if ((ret = libebg.env_create_new(&ebgenv)) != 0) { - ERROR("Cannot open/create new bootloader environment: %s.", - strerror(ret)); - } - } else if (strncmp(name, (char *)STATE_KEY, strlen((char *)STATE_KEY) + 1) == 0) { - /* Map update_state_t to EFI Boot Guard's API. */ - switch (*value) { - case STATE_IN_PROGRESS: - case STATE_FAILED: - case STATE_TESTING: - /* Fall-through for update_state_t values destined either - * for BOOTVAR_TRANSACTION or handled by EBG internally. */ - break; - case STATE_OK: - case STATE_INSTALLED: - DEBUG("Setting %s=%s in bootloader environment", name, value); - if ((ret = libebg.env_setglobalstate(&ebgenv, *value - '0')) != 0) { - ERROR("Cannot set %s=%s in bootloader environment.", STATE_KEY, value); - } - break; - default: - ret = -EINVAL; - ERROR("Unsupported bootloader environment assignment %s=%s.", STATE_KEY, value); - } - } else { - /* A new environment is created if EFI Boot Guard's - * representation of SWUpdate's recovery_status is - * not in_progress. */ - DEBUG("Setting %s=%s in bootloader environment", name, value); - if ((ret = libebg.env_create_new(&ebgenv)) != 0) { - ERROR("Cannot open/create new bootloader environment: %s.", - strerror(ret)); - return ret; - } - if ((ret = libebg.env_set(&ebgenv, (char *)name, (char *)value)) != 0) { - ERROR("Cannot set %s=%s in bootloader environment: %s.", - name, value, strerror(ret)); - } + char *value = malloc(size); + if (value == NULL) { + ERROR("Error allocating memory"); + return NULL; + } + + int result = libebg.env_get(&ebgenv, (char *)name, value); + if (result != 0) { + ERROR("Cannot get %s: %s", name, strerror(-result)); + free(value); + return NULL; + } + return value; +} + +/* Note: EFI Boot Guard Environment integers are at most uint32_t. */ +static inline uint32_t _env_to_uint32(char *value) +{ + if (!value) { + return UINT_MAX; } - (void)libebg.env_close(&ebgenv); + errno = 0; + uint32_t result = strtoul(value, NULL, 10); + free(value); + return errno != 0 ? UINT_MAX : result; +} - return ret; +static inline uint8_t ascii_to_uint8(unsigned char value) +{ + return value - '0'; } -static int do_env_unset(const char *name) +static inline unsigned char uint8_to_ascii(uint8_t value) { - int ret; + return value + '0'; +} +static char *do_env_get(const char *name) +{ + errno = 0; libebg.beverbose(&ebgenv, loglevel > INFOLEVEL ? true : false); - if ((ret = libebg.env_open_current(&ebgenv)) != 0) { - ERROR("Cannot open current bootloader environment: %s.", strerror(ret)); - return ret; + int result = libebg.env_open_current(&ebgenv); + if (result != 0) { + ERROR("Cannot open bootloader environment: %s", strerror(result)); + return NULL; } - DEBUG("Unsetting %s in bootloader environment", name); - if (strncmp(name, BOOTVAR_TRANSACTION, strlen(name) + 1) == 0) { - ret = libebg.env_finalize_update(&ebgenv); - if (ret) { - ERROR("Cannot unset %s in bootloader environment: %s.", BOOTVAR_TRANSACTION, strerror(ret)); + if (!inflight && is(name, EBGENV_USTATE)) { + /* + * When not in an in-flight transaction, get the "global significant" + * EBGENV_USTATE value: + * If rolled-back and successfully booted, there's an alternative + * boot path that has + * EBGENV_REVISION == 0 and + * EBGENV_USTATE == STATE_FAILED + * which is how EFI Boot Guard encodes a rolled-back condition. + * To act on this condition, e.g., report and clear it, STATE_FAILED + * as the global significant EBGENV_USTATE value is returned. + * + * If not rolled-back, the current boot path's EBGENV_USTATE value + * is returned. + */ + char *value = NULL; + if (asprintf(&value, "%u", libebg.env_getglobalstate(&ebgenv)) == -1) { + ERROR("Error allocating memory"); + return NULL; } - } else if (strncmp(name, (char *)STATE_KEY, strlen((char *)STATE_KEY) + 1) == 0) { - /* Unsetting STATE_KEY is semantically equivalent to setting it to STATE_OK. */ - if ((ret = libebg.env_setglobalstate(&ebgenv, STATE_OK - '0')) != 0) { - ERROR("Cannot unset %s in bootloader environment.", STATE_KEY); - } - } else { - ret = libebg.env_set_ex(&ebgenv, (char *)name, USERVAR_TYPE_DELETED, (uint8_t *)"", 1); + return value; } - (void)libebg.env_close(&ebgenv); - return ret; + return _env_get(name); } -static char *do_env_get(const char *name) +static int create_new_environment(void) { - char *value = NULL; - size_t size; + uint32_t revision = _env_to_uint32(_env_get(EBGENV_REVISION)); + uint8_t in_progress = (uint8_t)_env_to_uint32(_env_get(EBGENV_IN_PROGRESS)); + if ((revision == UINT_MAX) || (in_progress == UINT8_MAX)) { + ERROR("Cannot get environment revision or in-progress marker"); + return -EIO; + } + if (in_progress == 1) { + return 0; + } + int result = libebg.env_create_new(&ebgenv); + if (result != 0) { + ERROR("Cannot create new environment revision: %s", strerror(result)); + return -result; + } + /* + * libebgenv has now set: + * EBG_ENVDATA->in_progress = 1 + * EBG_ENVDATA->revision = <current boot path's revision>++ + */ + uint32_t new_revision = _env_to_uint32(_env_get(EBGENV_REVISION)); + if (new_revision == UINT_MAX) { + return -EIO; + } + if (++revision != new_revision) { + ERROR("No new environment revision was created!"); + return -ENOENT; + } + inflight = true; + DEBUG("Created new environment revision %d, starting transaction", + new_revision); + return 0; +} +static int do_env_set(const char *name, const char *value) +{ errno = 0; libebg.beverbose(&ebgenv, loglevel > INFOLEVEL ? true : false); - int ret; - if ((ret = libebg.env_open_current(&ebgenv)) != 0) { - ERROR("Cannot open current bootloader environment: %s.", - strerror(ret)); - return NULL; + if (!inflight) { + /* + * Without an in-flight transaction, only allow + * (1) starting a transaction or + * (2) acknowledging an update. + */ + if (!is(name, BOOTVAR_TRANSACTION) && !is(name, EBGENV_USTATE)) { + ERROR("Not setting %s=%s w/o in-flight transaction", + name, value); + return -EINVAL; + } + if (is(name, BOOTVAR_TRANSACTION) && + !is(value, get_state_string(STATE_IN_PROGRESS))) { + ERROR("Not setting %s=%s w/o in-flight transaction", + name, value); + return -EINVAL; + } + + if (is(name, EBGENV_USTATE)) { + switch (*value) { + case STATE_OK: /* Acknowledging an update. */ + case STATE_IN_PROGRESS: /* Starting a transaction. */ + break; + default: + ERROR("Not setting %s=%s w/o in-flight transaction", + name, value); + return -EINVAL; + } + } + } + + int result = libebg.env_open_current(&ebgenv); + if (result != 0) { + ERROR("Cannot open bootloader environment: %s", strerror(result)); + return -result; } - if (strncmp(name, (char *)STATE_KEY, strlen((char *)STATE_KEY) + 1) == 0) { - value = (char *)malloc(sizeof(char)); - *value = libebg.env_getglobalstate(&ebgenv); - /* Map EFI Boot Guard's int return to update_state_t's char value */ - *value = *value + '0'; - } else { - if ((size = libebg.env_get(&ebgenv, (char *)name, NULL)) != 0) { - value = malloc(size); - if (value) { - if (libebg.env_get(&ebgenv, (char *)name, value) != 0) { - value = NULL; + if (is(name, BOOTVAR_TRANSACTION)) { + /* + * Note: This gets called by core/stream_interface.c's + * update_transaction_state() with the update + * state's string representation. + */ + if (is(value, get_state_string(STATE_IN_PROGRESS))) { + return create_new_environment(); + } + + if (is(value, get_state_string(STATE_FAILED)) || + is(value, get_state_string(STATE_INSTALLED)) || + is(value, get_state_string(STATE_OK))) { + /* + * Irrespective of the value, set EBGENV_IN_PROGRESS = 0, + * else EFI Boot Guard will *NOT* consider this environment + * for booting at all. + */ + if ((result = libebg.env_set(&ebgenv, EBGENV_IN_PROGRESS, + (char *)"0")) != 0) { + ERROR("Error setting %s=0: %s", EBGENV_IN_PROGRESS, + strerror(-result)); + return result; + } + return 0; + } + + /* Fall-through for invalid EBGENV_IN_PROGRESS values. */ + ERROR("Unsupported setting %s=%s", EBGENV_IN_PROGRESS, value); + return -EINVAL; + } + + if (!is(name, EBGENV_USTATE)) { + if ((result = libebg.env_set(&ebgenv, (char *)name, (char *)value)) != 0) { + ERROR("Error setting %s=%s: %s", name, value, strerror(-result)); + return result; + } + return 0; + } + + switch (*value) { + case STATE_IN_PROGRESS: + return create_new_environment(); + case STATE_OK: + if (inflight) { + /* + * Environment modification within the in-flight transaction, + * i.e., the in-memory working copy, just set it. + */ + if ((result = libebg.env_set(&ebgenv, EBGENV_USTATE, + (char *)value)) != 0) { + ERROR("Error setting %s=%s: %s", EBGENV_USTATE, + get_state_string(STATE_OK), + strerror(-result)); + return result; + } + return 0; + } + + unsigned char global_ustate = uint8_to_ascii((uint8_t) + libebg.env_getglobalstate(&ebgenv)); + if (global_ustate == STATE_NOT_AVAILABLE) { + ERROR("Cannot read global %s", EBGENV_USTATE); + return -EIO; + } + unsigned char current_ustate = uint8_to_ascii((uint8_t) + _env_to_uint32( + _env_get(EBGENV_USTATE))); + if (!is_valid_state(current_ustate)) { + ERROR("Cannot read current %s", EBGENV_USTATE); + return -EIO; + } + + if (global_ustate == STATE_FAILED) { + TRACE("Found rolled-back condition, clearing marker"); + /* + * Clear rolled-back condition by setting + * EBGENV_USTATE = STATE_OK on all alternative + * boot paths having EBGENV_USTATE != STATE_OK + * and write them to disk. + * Note: Does not write to the current boot path (to + * which was rolled-back to). + */ + if ((result = libebg.env_setglobalstate(&ebgenv, + ascii_to_uint8(STATE_OK))) != 0) { + ERROR("Error resetting failure condition: %s", + strerror(-result)); + return result; + } + /* + * Restore prior current boot path's EBGENV_USTATE value + * (as there's no way to reload it from disk). + * Should be STATE_OK anyway but better play it safe... + */ + if (current_ustate != STATE_OK) { + if ((result = libebg.env_set(&ebgenv, EBGENV_USTATE, + (char[2]){ current_ustate, '\0' })) != 0) { + ERROR("Error restoring %s: %s", EBGENV_USTATE, + strerror(-result)); + return result; } } + return 0; + } + + if (current_ustate == STATE_TESTING) { + TRACE("Found successful update, blessing it"); + /* + * Acknowledge, update the /current/ boot path on disk. + */ + if ((result = libebg.env_set(&ebgenv, EBGENV_USTATE, + (char *)value)) != 0) { + ERROR("Error setting %s=%s: %s", EBGENV_USTATE, + value, strerror(-result)); + return result; + } + + if ((result = libebg.env_close(&ebgenv)) != 0) { + ERROR("Error persisting environment: %s", + strerror(result)); + return -result; + } + return 0; } + + WARN("Unsupported state for setting %s=%s", EBGENV_USTATE, + get_state_string(*value)); + return -EINVAL; + case STATE_INSTALLED: + if ((result = libebg.env_finalize_update(&ebgenv)) != 0) { + ERROR("Error finalizing environment: %s", strerror(result)); + return -result; + } + /* + * libebgenv has now set: + * EBG_ENVDATA->in_progress = 0 + * EBG_ENVDATA->ustate = USTATE_INSTALLED. + * Now, persist the in-memory working copy environment as new + * current boot path and terminate the transaction. + * Note: Does write to an alternative boot path "upcycled" to + * the new current boot path; Does *NOT* write to the current + * boot path. + */ + if ((result = libebg.env_close(&ebgenv)) != 0) { + ERROR("Error persisting environment: %s", strerror(result)); + return -result; + } + inflight = false; + return 0; + case STATE_FAILED: + /* + * SWUpdate sets this if the installation has failed. In this case, + * the transaction is simply not committed, so nothing to do. + */ + return 0; + default: + break; } - (void)libebg.env_close(&ebgenv); + /* + * Fall-through for invalid or EBGENV_USTATE values handled + * by EFI Boot Guard internally. + */ + WARN("Unsupported setting %s=%s", EBGENV_USTATE, get_state_string(*value)); + return -EINVAL; +} + +static int do_env_unset(const char *name) +{ + errno = 0; + libebg.beverbose(&ebgenv, loglevel > INFOLEVEL ? true : false); - if (value == NULL) { - ERROR("Cannot get %s from bootloader environment: %s", - name, strerror(errno)); + int result = libebg.env_open_current(&ebgenv); + if (result != 0) { + ERROR("Cannot open bootloader environment: %s", strerror(result)); + return -result; } - return value; + if (is(name, EBGENV_USTATE)) { + /* + * Unsetting EBGENV_USTATE is semantically equivalent to + * setting EBGENV_USTATE = STATE_OK. + */ + return do_env_set(EBGENV_USTATE, (char[2]){ STATE_OK, '\0' }); + } + + if (is(name, BOOTVAR_TRANSACTION)) { + /* + * Unsetting BOOTVAR_TRANSACTION is semantically equivalent to + * setting EBGENV_IN_PROGRESS = 0. + */ + return do_env_set(EBGENV_IN_PROGRESS, (char *)"0"); + } + + if ((result = libebg.env_set_ex(&ebgenv, (char *)name, USERVAR_TYPE_DELETED, + (uint8_t *)"", 1)) != 0) { + ERROR("Error unsetting %s: %s", name, strerror(-result)); + return result; + } + return 0; } static int do_apply_list(const char *filename) { - FILE *fp = NULL; - char *line = NULL; - char *key; - char *value; - size_t len = 0; - int ret = 0; - errno = 0; libebg.beverbose(&ebgenv, loglevel > INFOLEVEL ? true : false); - if (!(fp = fopen(filename, "rb"))) { - ERROR("Failed to open bootloader environment file %s: %s", - filename, strerror(errno)); - return -1; + FILE *file = fopen(filename, "rb"); + if (!file) { + ERROR("Cannot open bootloader environment source file %s: %s", + filename, strerror(errno)); + return -EIO; } - while ((getline(&line, &len, fp)) != -1) { - key = strtok(line, "="); - value = strtok(NULL, "\t\n"); - if (value != NULL && key != NULL) { - if ((ret = do_env_set(key, value)) != 0) { + char *line = NULL; + size_t length = 0; + int result = 0; + while ((getline(&line, &length, file)) != -1) { + char *key = strtok(line, "="); + char *value = strtok(NULL, "\t\n"); + if (key != NULL && value != NULL) { + if ((result = do_env_set(key, value)) != 0) { break; } } } - if (fp) { - fclose(fp); - } - if (line) { - free(line); - } - return ret; + fclose(file); + free(line); + return result; } static bootloader ebg = { @@ -212,9 +509,15 @@ static bootloader ebg = { .apply_list = &do_apply_list }; -static bootloader* probe(void) +static bootloader *probe(void) { - void* handle = dlopen("libebgenv.so.0", RTLD_NOW | RTLD_GLOBAL); + if (!is(STATE_KEY, EBGENV_USTATE)) { + ERROR("CONFIG_UPDATE_STATE_BOOTLOADER=%s is required for " + "EFI Boot Guard support", EBGENV_USTATE); + return NULL; + } + + void *handle = dlopen("libebgenv.so.0", RTLD_NOW | RTLD_GLOBAL); if (!handle) { return NULL; } diff --git a/core/stream_interface.c b/core/stream_interface.c index 81c26c3..9ba6332 100644 --- a/core/stream_interface.c +++ b/core/stream_interface.c @@ -611,6 +611,40 @@ void *network_initializer(void *data) if (!(inst.fd < 0)) close(inst.fd); + if (!software->parms.dry_run && is_bootloader(BOOTLOADER_EBG)) { + if (!software->bootloader_transaction_marker) { + /* + * EFI Boot Guard's "in_progress" environment variable + * has special semantics hard-coded, hence the + * bootloader transaction marker cannot be disabled. + */ + TRACE("Note: Setting EFI Boot Guard's 'in_progress' " + "environment variable cannot be disabled."); + } + if (!software->bootloader_state_marker) { + /* + * With a disabled update state marker, there's no + * transaction auto-commit via + * save_state(STATE_INSTALLED) + * which effectively calls + * bootloader_env_set(STATE_KEY, STATE_INSTALLED). + * Hence, manually calling save_state(STATE_INSTALLED) + * or equivalent is required to commit the transaction. + * This can be useful to, e.g., terminate the transaction + * from an according progress interface client or an + * SWUpdate suricatta module after it has received an + * update activation request from the remote server. + */ + TRACE("Note: EFI Boot Guard environment transaction " + "will not be auto-committed."); + } + if (!software->bootloader_transaction_marker && + !software->bootloader_state_marker) { + WARN("EFI Boot Guard environment modifications will " + "not be persisted."); + } + } + /* do carry out the installation (flash programming) */ if (ret == 0) { TRACE("Valid image found: copying to FLASH");
Completely rework the EFI Boot Guard Binding accommodating to libebgenv's implicit assumptions and actions. To this end, environment modification transactions are introduced. The assumptions and rationale is documented and in addition log messages are emitted for particular combinations of bootloader_transaction_marker and bootloader_state_marker. Signed-off-by: Christian Storm <christian.storm@siemens.com> --- bootloader/ebg.c | 567 ++++++++++++++++++++++++++++++---------- core/stream_interface.c | 34 +++ 2 files changed, 469 insertions(+), 132 deletions(-)