sd-boot: introduce and use efivar_unset()

Currently some of the code base check for the variable presence before
removing it, and some do not.

More so, in all cases (being updated) we're dealing with non-volatile
variables where changing those attribute to NVRAM wear out.

From what information I could find, there is no definitive answer if the
UEFI implementation will write to the NVRAM even when the variable is
missing.

So add a simple helper that checks for the variable presence before
removing it. While also having a bit cleaner API than the current
efivar_set(..., NULL, ...);

efivar_unset() follows the design from efivar_set*() where it returns an
EFI_STATUS even though its (presently) unused.

v2:
 - add inline comment, use early return

v3:
 - typos? typos!

Signed-off-by: Emil Velikov <emil.velikov@collabora.com>

diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c
index 6dad2bc3c4ab19bd7d5110aa5a15b26ea304ef5c..7b13cd21db4ae2f625445c1964b8ae2f77563bdd 100644 (file)
--- a/src/boot/efi/boot.c
+++ b/src/boot/efi/boot.c
@@ -1058,7 +1058,7 @@ static bool menu_run(
 
         if (console_mode_efivar_saved != config->console_mode_efivar) {
                 if (config->console_mode_efivar == CONSOLE_MODE_KEEP)
-                        efivar_set(MAKE_GUID_PTR(LOADER), u"LoaderConfigConsoleMode", NULL, EFI_VARIABLE_NON_VOLATILE);
+                        efivar_unset(MAKE_GUID_PTR(LOADER), u"LoaderConfigConsoleMode", EFI_VARIABLE_NON_VOLATILE);
                 else
                         efivar_set_uint_string(MAKE_GUID_PTR(LOADER), u"LoaderConfigConsoleMode",
                                                config->console_mode_efivar, EFI_VARIABLE_NON_VOLATILE);
@@ -1067,7 +1067,7 @@ static bool menu_run(
         if (timeout_efivar_saved != config->timeout_sec_efivar) {
                 switch (config->timeout_sec_efivar) {
                 case TIMEOUT_UNSET:
-                        efivar_set(MAKE_GUID_PTR(LOADER), u"LoaderConfigTimeout", NULL, EFI_VARIABLE_NON_VOLATILE);
+                        efivar_unset(MAKE_GUID_PTR(LOADER), u"LoaderConfigTimeout", EFI_VARIABLE_NON_VOLATILE);
                         break;
                 case TIMEOUT_MENU_FORCE:
                         efivar_set(MAKE_GUID_PTR(LOADER), u"LoaderConfigTimeout", u"menu-force", EFI_VARIABLE_NON_VOLATILE);
@@ -1618,7 +1618,7 @@ static void config_load_defaults(Config *config, EFI_FILE *root_dir) {
         err = efivar_get_timeout(u"LoaderConfigTimeoutOneShot", &config->timeout_sec);
         if (err == EFI_SUCCESS) {
                 /* Unset variable now, after all it's "one shot". */
-                (void) efivar_set(MAKE_GUID_PTR(LOADER), u"LoaderConfigTimeoutOneShot", NULL, EFI_VARIABLE_NON_VOLATILE);
+                (void) efivar_unset(MAKE_GUID_PTR(LOADER), u"LoaderConfigTimeoutOneShot", EFI_VARIABLE_NON_VOLATILE);
 
                 config->force_menu = true; /* force the menu when this is set */
         } else if (err != EFI_NOT_FOUND)
@@ -1631,7 +1631,7 @@ static void config_load_defaults(Config *config, EFI_FILE *root_dir) {
         err = efivar_get(MAKE_GUID_PTR(LOADER), u"LoaderEntryOneShot", &config->entry_oneshot);
         if (err == EFI_SUCCESS)
                 /* Unset variable now, after all it's "one shot". */
-                (void) efivar_set(MAKE_GUID_PTR(LOADER), u"LoaderEntryOneShot", NULL, EFI_VARIABLE_NON_VOLATILE);
+                (void) efivar_unset(MAKE_GUID_PTR(LOADER), u"LoaderEntryOneShot", EFI_VARIABLE_NON_VOLATILE);
 
         (void) efivar_get(MAKE_GUID_PTR(LOADER), u"LoaderEntryDefault", &config->entry_default_efivar);
 
@@ -2503,7 +2503,7 @@ static void save_selected_entry(const Config *config, const ConfigEntry *entry)
                 (void) efivar_set(MAKE_GUID_PTR(LOADER), u"LoaderEntryLastBooted", entry->id, EFI_VARIABLE_NON_VOLATILE);
         } else
                 /* Delete the non-volatile var if not needed. */
-                (void) efivar_set(MAKE_GUID_PTR(LOADER), u"LoaderEntryLastBooted", NULL, EFI_VARIABLE_NON_VOLATILE);
+                (void) efivar_unset(MAKE_GUID_PTR(LOADER), u"LoaderEntryLastBooted", EFI_VARIABLE_NON_VOLATILE);
 }
 
 static EFI_STATUS secure_boot_discover_keys(Config *config, EFI_FILE *root_dir) {

diff --git a/src/boot/efi/util.c b/src/boot/efi/util.c
index 60991d471821a350b85b81d9bd509bdfa52204ee..bb3ccb8280647b00ccbbf0ac1d0acc4bd58c52f3 100644 (file)
--- a/src/boot/efi/util.c
+++ b/src/boot/efi/util.c
@@ -85,6 +85,21 @@ EFI_STATUS efivar_set_uint64_le(const EFI_GUID *vendor, const char16_t *name, ui
         return efivar_set_raw(vendor, name, buf, sizeof(buf), flags);
 }
 
+EFI_STATUS efivar_unset(const EFI_GUID *vendor, const char16_t *name, uint32_t flags) {
+        EFI_STATUS err;
+
+        assert(vendor);
+        assert(name);
+
+        /* We could be wiping a non-volatile variable here and the spec makes no guarantees that won't incur
+         * in an extra write (and thus wear out). So check and clear only if needed. */
+        err = efivar_get_raw(vendor, name, NULL, NULL);
+        if (err == EFI_SUCCESS)
+                return efivar_set_raw(vendor, name, NULL, 0, flags);
+
+        return err;
+}
+
 EFI_STATUS efivar_get(const EFI_GUID *vendor, const char16_t *name, char16_t **ret) {
         _cleanup_free_ char16_t *buf = NULL;
         EFI_STATUS err;

diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h
index 10620dabcabcd41cfc7c6f931460e49f59f7b899..2f8c96d0ac67f05a3cb1cd1039085cf6d220efe2 100644 (file)
--- a/src/boot/efi/util.h
+++ b/src/boot/efi/util.h
@@ -95,6 +95,8 @@ EFI_STATUS efivar_set_uint32_le(const EFI_GUID *vendor, const char16_t *NAME, ui
 EFI_STATUS efivar_set_uint64_le(const EFI_GUID *vendor, const char16_t *name, uint64_t value, uint32_t flags);
 void efivar_set_time_usec(const EFI_GUID *vendor, const char16_t *name, uint64_t usec);
 
+EFI_STATUS efivar_unset(const EFI_GUID *vendor, const char16_t *name, uint32_t flags);
+
 EFI_STATUS efivar_get(const EFI_GUID *vendor, const char16_t *name, char16_t **ret);
 EFI_STATUS efivar_get_raw(const EFI_GUID *vendor, const char16_t *name, char **ret, size_t *ret_size);
 EFI_STATUS efivar_get_uint_string(const EFI_GUID *vendor, const char16_t *name, size_t *ret);