From 7fa23ab646a02c7299ff8257cb104ad8f83eda70 Mon Sep 17 00:00:00 2001 From: Spencer Michaels Date: Tue, 5 Nov 2019 21:23:40 -0800 Subject: [PATCH 1/6] boot: Make EFISTUB IDs use binaries' filenames. Change config_entry_add_linux() so that Linux EFISTUB entries' ids are simply the binaries' filenames, as is already the case with loader configs. This guarantees that EFISTUB binaries' IDs are mutually unique. --- src/boot/efi/boot.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index 849c3c63b6..217dfbad16 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -1965,12 +1965,11 @@ static VOID config_entry_add_linux( } if (os_name && os_id && (os_version || os_build)) { - _cleanup_freepool_ CHAR16 *conf = NULL, *path = NULL; + _cleanup_freepool_ CHAR16 *path = NULL; - conf = PoolPrint(L"%s-%s", os_id, os_version ? : os_build); path = PoolPrint(L"\\EFI\\Linux\\%s", f->FileName); - entry = config_entry_add_loader(config, device, LOADER_LINUX, conf, 'l', os_name, path); + entry = config_entry_add_loader(config, device, LOADER_LINUX, f->FileName, 'l', os_name, path); FreePool(content); content = NULL; From 65901c0fd164bdcf248f7c0317342c4f3fb761b7 Mon Sep 17 00:00:00 2001 From: Spencer Michaels Date: Tue, 5 Nov 2019 19:56:05 -0800 Subject: [PATCH 2/6] boot: Ignore EFISTUB binaries starting with "auto-". To further increase similarity with loader configs and provide global uniqueness, ignore filenames starting with auto- (see boot.c:1512). --- src/boot/efi/boot.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index 217dfbad16..39bad10eee 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -1927,6 +1927,8 @@ static VOID config_entry_add_linux( continue; if (StriCmp(f->FileName + len - 4, L".efi") != 0) continue; + if (StrnCmp(f->FileName, L"auto-", 5) == 0) + continue; /* look for .osrel and .cmdline sections in the .efi binary */ err = pe_file_locate_sections(linux_dir, f->FileName, sections, addrs, offs, szs); From 6cd12ebcfe459466257ea63022a32515d756e719 Mon Sep 17 00:00:00 2001 From: Spencer Michaels Date: Tue, 5 Nov 2019 19:55:39 -0800 Subject: [PATCH 3/6] boot: Retain ".conf" suffix for loader config IDs. Change config_entry_add_from_file() so that it does not remove the `.conf` ending from the filename that it uses for entry->id. This is necessary because otherwise an EFISTUB binary and a loader config entry might end up with the same ID, as the loader config IDs previously didn't include the config filename's .conf extension (see boot.c:1435). Consider, for instance, an EFISTUB /EFI/Linux/linux.efi and a loader config /loader/entries/linux.efi.conf; both would have the ID linux.efi. In addition, update a comment that previously stated that IDs are non-unique (which is no longer the case, as of this commit). --- src/boot/efi/boot.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index 39bad10eee..f6989ddb9b 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -32,7 +32,7 @@ enum loader_type { }; typedef struct { - CHAR16 *id; /* The identifier for this entry (note that this id is not necessarily unique though!) */ + CHAR16 *id; /* The unique identifier for this entry */ CHAR16 *title_show; CHAR16 *title; CHAR16 *version; @@ -1310,7 +1310,6 @@ static VOID config_entry_add_from_file( CHAR8 *line; UINTN pos = 0; CHAR8 *key, *value; - UINTN len; EFI_STATUS err; EFI_FILE_HANDLE handle; _cleanup_freepool_ CHAR16 *initrd = NULL; @@ -1431,10 +1430,6 @@ static VOID config_entry_add_from_file( entry->device = device; entry->id = StrDuplicate(file); - len = StrLen(entry->id); - /* remove ".conf" */ - if (len > 5) - entry->id[len - 5] = '\0'; StrLwr(entry->id); config_add_entry(config, entry); From 10d0024a07c8a919a5637c1295d26145fde7cd56 Mon Sep 17 00:00:00 2001 From: Spencer Michaels Date: Tue, 5 Nov 2019 21:28:05 -0800 Subject: [PATCH 4/6] boot: Improve EFISTUB name and version detection. Make two minor cosmetic changes to how config_entry_add_linux() extracts OS information from the embedded /etc/os-release in EFISTUB binaries. This increases the detail of information available to the user for EFISTUB entries. * entry->version, which was previously always null, is now set to the value of VERSION, or VERSION_ID, or BUILD_ID (preferred in that order). * entry->title, which was previously set to the value of PRETTY_NAME, can now fall back to NAME or ID if PRETTY_NAME is not present. In that case, NAME is preferred over ID. --- src/boot/efi/boot.c | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index f6989ddb9b..e86824ad31 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -1770,7 +1770,8 @@ static ConfigEntry *config_entry_add_loader( CHAR16 *id, CHAR16 key, CHAR16 *title, - CHAR16 *loader) { + CHAR16 *loader, + CHAR16 *version) { ConfigEntry *entry; @@ -1778,6 +1779,7 @@ static ConfigEntry *config_entry_add_loader( *entry = (ConfigEntry) { .type = type, .title = StrDuplicate(title), + .version = StrDuplicate(version), .device = device, .loader = StrDuplicate(loader), .id = StrDuplicate(id), @@ -1835,7 +1837,7 @@ static BOOLEAN config_entry_add_loader_auto( return FALSE; uefi_call_wrapper(handle->Close, 1, handle); - entry = config_entry_add_loader(config, device, LOADER_UNDEFINED, id, key, title, loader); + entry = config_entry_add_loader(config, device, LOADER_UNDEFINED, id, key, title, loader, NULL); if (!entry) return FALSE; @@ -1903,10 +1905,12 @@ static VOID config_entry_add_linux( CHAR8 *line; UINTN pos = 0; CHAR8 *key, *value; + CHAR16 *os_name_pretty = NULL; CHAR16 *os_name = NULL; CHAR16 *os_id = NULL; CHAR16 *os_version = NULL; - CHAR16 *os_build = NULL; + CHAR16 *os_version_id = NULL; + CHAR16 *os_build_id = NULL; err = uefi_call_wrapper(linux_dir->Read, 3, linux_dir, &bufsize, buf); if (bufsize == 0 || EFI_ERROR(err)) @@ -1937,6 +1941,12 @@ static VOID config_entry_add_linux( /* read properties from the embedded os-release file */ while ((line = line_get_key_value(content, (CHAR8 *)"=", &pos, &key, &value))) { if (strcmpa((CHAR8 *)"PRETTY_NAME", key) == 0) { + FreePool(os_name_pretty); + os_name_pretty = stra_to_str(value); + continue; + } + + if (strcmpa((CHAR8 *)"NAME", key) == 0) { FreePool(os_name); os_name = stra_to_str(value); continue; @@ -1954,19 +1964,27 @@ static VOID config_entry_add_linux( continue; } + if (strcmpa((CHAR8 *)"VERSION_ID", key) == 0) { + FreePool(os_version_id); + os_version_id = stra_to_str(value); + continue; + } + if (strcmpa((CHAR8 *)"BUILD_ID", key) == 0) { - FreePool(os_build); - os_build = stra_to_str(value); + FreePool(os_build_id); + os_build_id = stra_to_str(value); continue; } } - if (os_name && os_id && (os_version || os_build)) { + if ((os_name_pretty || os_name) && os_id && (os_version || os_version_id || os_build_id)) { _cleanup_freepool_ CHAR16 *path = NULL; path = PoolPrint(L"\\EFI\\Linux\\%s", f->FileName); - entry = config_entry_add_loader(config, device, LOADER_LINUX, f->FileName, 'l', os_name, path); + entry = config_entry_add_loader(config, device, LOADER_LINUX, f->FileName, 'l', + os_name_pretty ? : (os_name ? : os_id), path, + os_version ? : (os_version_id ? : os_build_id)); FreePool(content); content = NULL; @@ -1985,10 +2003,12 @@ static VOID config_entry_add_linux( config_entry_parse_tries(entry, L"\\EFI\\Linux", f->FileName, L".efi"); } + FreePool(os_name_pretty); FreePool(os_name); FreePool(os_id); FreePool(os_version); - FreePool(os_build); + FreePool(os_version_id); + FreePool(os_build_id); FreePool(content); } From ae474efc3f6508bc4a0e50505f205f0b0b7c7617 Mon Sep 17 00:00:00 2001 From: Spencer Michaels Date: Tue, 12 Nov 2019 18:45:50 -0800 Subject: [PATCH 5/6] boot: Update bootspec.c to match previous changes. bootspec.c, which is used by bootctl and systemctl, computes bootloaders entry IDs independently from systemd-boot. This commit updates the ID computation in bootspec.c to be in line with the previous few commits altering boot.c. --- src/shared/bootspec.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 4f750dc1da..ff6d5666bc 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -73,12 +73,12 @@ static int boot_entry_load( return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid loader entry file suffix: %s", path); b = basename(path); - tmp.id = strndup(b, c - b); + tmp.id = strdup(b); if (!tmp.id) return log_oom(); if (!efi_loader_entry_name_valid(tmp.id)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid loader entry filename: %s", path); + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid loader entry: %s", tmp.id); tmp.path = strdup(path); if (!tmp.path) @@ -283,12 +283,13 @@ static int boot_entry_load_unified( const char *cmdline, BootEntry *ret) { - _cleanup_free_ char *os_pretty_name = NULL, *os_id = NULL, *version_id = NULL, *build_id = NULL; + _cleanup_free_ char *os_pretty_name = NULL; _cleanup_(boot_entry_free) BootEntry tmp = { .type = BOOT_ENTRY_UNIFIED, }; _cleanup_fclose_ FILE *f = NULL; const char *k; + char *b; int r; assert(root); @@ -303,18 +304,15 @@ static int boot_entry_load_unified( if (!f) return log_error_errno(errno, "Failed to open os-release buffer: %m"); - r = parse_env_file(f, "os-release", - "PRETTY_NAME", &os_pretty_name, - "ID", &os_id, - "VERSION_ID", &version_id, - "BUILD_ID", &build_id); + r = parse_env_file(f, "os-release", "PRETTY_NAME", &os_pretty_name); if (r < 0) return log_error_errno(r, "Failed to parse os-release data from unified kernel image %s: %m", path); - if (!os_pretty_name || !os_id || !(version_id || build_id)) + if (!os_pretty_name) return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), "Missing fields in os-release data from unified kernel image %s, refusing.", path); - tmp.id = strjoin(os_id, "-", version_id ?: build_id); + b = basename(path); + tmp.id = strdup(b); if (!tmp.id) return log_oom(); From 15b82eecb6461b55c8a70abd3276318af8dee0b9 Mon Sep 17 00:00:00 2001 From: Spencer Michaels Date: Wed, 20 Nov 2019 14:18:32 -0800 Subject: [PATCH 6/6] boot: Deduplicate old-style loader entries. In cases where systemd (and thus bootctl) is updated to a version including the earlier unique-ID fix, but the corresponding new version of systemd-boot is not installed to the ESP and run at least once, the bootloader will report old-style entry IDs cached in the LoaderEntries EFI variable, while bootctl will report new-style IDs for the same entries, producing duplicate entries. This commit makes bootctl compute and retain old-style IDs for non-auto entries so that it can properly deduplicate entries even if the cache contains old-style IDs. --- src/shared/bootspec.c | 17 ++++++++++++----- src/shared/bootspec.h | 8 ++++++-- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index ff6d5666bc..699b101b39 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -36,6 +36,7 @@ static void boot_entry_free(BootEntry *entry) { assert(entry); free(entry->id); + free(entry->id_old); free(entry->path); free(entry->root); free(entry->title); @@ -74,7 +75,8 @@ static int boot_entry_load( b = basename(path); tmp.id = strdup(b); - if (!tmp.id) + tmp.id_old = strndup(b, c - b); + if (!tmp.id || !tmp.id_old) return log_oom(); if (!efi_loader_entry_name_valid(tmp.id)) @@ -283,7 +285,7 @@ static int boot_entry_load_unified( const char *cmdline, BootEntry *ret) { - _cleanup_free_ char *os_pretty_name = NULL; + _cleanup_free_ char *os_pretty_name = NULL, *os_id = NULL, *version_id = NULL, *build_id = NULL; _cleanup_(boot_entry_free) BootEntry tmp = { .type = BOOT_ENTRY_UNIFIED, }; @@ -304,16 +306,21 @@ static int boot_entry_load_unified( if (!f) return log_error_errno(errno, "Failed to open os-release buffer: %m"); - r = parse_env_file(f, "os-release", "PRETTY_NAME", &os_pretty_name); + r = parse_env_file(f, "os-release", + "PRETTY_NAME", &os_pretty_name, + "ID", &os_id, + "VERSION_ID", &version_id, + "BUILD_ID", &build_id); if (r < 0) return log_error_errno(r, "Failed to parse os-release data from unified kernel image %s: %m", path); - if (!os_pretty_name) + if (!os_pretty_name || !os_id || !(version_id || build_id)) return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), "Missing fields in os-release data from unified kernel image %s, refusing.", path); b = basename(path); tmp.id = strdup(b); - if (!tmp.id) + tmp.id_old = strjoin(os_id, "-", version_id ?: build_id); + if (!tmp.id || !tmp.id_old) return log_oom(); if (!efi_loader_entry_name_valid(tmp.id)) diff --git a/src/shared/bootspec.h b/src/shared/bootspec.h index c18d89494a..a825b35bc5 100644 --- a/src/shared/bootspec.h +++ b/src/shared/bootspec.h @@ -21,6 +21,7 @@ typedef enum BootEntryType { typedef struct BootEntry { BootEntryType type; char *id; /* This is the file basename without extension */ + char *id_old; /* Old-style ID, for deduplication purposes. */ char *path; /* This is the full path to the drop-in file */ char *root; /* The root path in which the drop-in was found, i.e. to which 'kernel', 'efi' and 'initrd' are relative */ char *title; @@ -54,9 +55,12 @@ typedef struct BootConfig { static inline bool boot_config_has_entry(BootConfig *config, const char *id) { size_t j; - for (j = 0; j < config->n_entries; j++) - if (streq(config->entries[j].id, id)) + for (j = 0; j < config->n_entries; j++) { + const char* entry_id_old = config->entries[j].id_old; + if (streq(config->entries[j].id, id) || + (entry_id_old && streq(entry_id_old, id))) return true; + } return false; }