From 18fde343a845a909260e3443f0a0734ba00cabe0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Nov 2021 23:54:10 +0100 Subject: boot: clean up unified boot loader entry name/version extraction Let's make sure IMAGE_ID/IMAGE_VERSION are properly honoured, and explain in a long comment why. Let's also use ID= field again, which was lost by accident. (While we are at it do some minimal OOM checks wherever we touch something) --- src/boot/efi/boot.c | 112 ++++++++++++++++++++------------- src/fundamental/bootspec-fundamental.c | 56 +++++++++++++++++ src/fundamental/bootspec-fundamental.h | 16 +++++ src/fundamental/meson.build | 4 +- 4 files changed, 142 insertions(+), 46 deletions(-) create mode 100644 src/fundamental/bootspec-fundamental.c create mode 100644 src/fundamental/bootspec-fundamental.h diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index 68fb94cbdc..89888c3aa9 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -4,6 +4,7 @@ #include #include +#include "bootspec-fundamental.h" #include "console.h" #include "devicetree.h" #include "disk.h" @@ -2040,8 +2041,10 @@ static void config_entry_add_linux( NULL, }; - _cleanup_freepool_ CHAR16 *os_name_pretty = NULL, *os_name = NULL, *os_id = NULL, - *os_version = NULL, *os_version_id = NULL, *os_build_id = NULL, *os_image_version = NULL; + _cleanup_freepool_ CHAR16 *os_pretty_name = NULL, *os_image_id = NULL, *os_name = NULL, *os_id = NULL, + *os_image_version = NULL, *os_version = NULL, *os_version_id = NULL, *os_build_id = NULL, + *path = NULL; + const CHAR16 *good_name, *good_version; _cleanup_freepool_ CHAR8 *content = NULL; UINTN offs[_SECTION_MAX] = {}; UINTN szs[_SECTION_MAX] = {}; @@ -2073,82 +2076,101 @@ 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); + if (strcmpa((const CHAR8*) "PRETTY_NAME", key) == 0) { + FreePool(os_pretty_name); + os_pretty_name = stra_to_str(value); continue; } - if (strcmpa((CHAR8 *)"NAME", key) == 0) { + if (strcmpa((const CHAR8*) "IMAGE_ID", key) == 0) { + FreePool(os_image_id); + os_image_id = stra_to_str(value); + continue; + } + + if (strcmpa((const CHAR8*) "NAME", key) == 0) { FreePool(os_name); os_name = stra_to_str(value); continue; } - if (strcmpa((CHAR8 *)"ID", key) == 0) { + if (strcmpa((const CHAR8*) "ID", key) == 0) { FreePool(os_id); os_id = stra_to_str(value); continue; } - if (strcmpa((CHAR8 *)"VERSION", key) == 0) { + if (strcmpa((const CHAR8*) "IMAGE_VERSION", key) == 0) { + FreePool(os_image_version); + os_image_version = stra_to_str(value); + continue; + } + + if (strcmpa((const CHAR8*) "VERSION", key) == 0) { FreePool(os_version); os_version = stra_to_str(value); continue; } - if (strcmpa((CHAR8 *)"VERSION_ID", key) == 0) { + if (strcmpa((const CHAR8*) "VERSION_ID", key) == 0) { FreePool(os_version_id); os_version_id = stra_to_str(value); continue; } - if (strcmpa((CHAR8 *)"BUILD_ID", key) == 0) { + if (strcmpa((const CHAR8*) "BUILD_ID", key) == 0) { FreePool(os_build_id); os_build_id = stra_to_str(value); continue; } - - if (strcmpa((const CHAR8*) "IMAGE_VERSION", key) == 0) { - FreePool(os_image_version); - os_image_version = stra_to_str(value); - continue; - } } - if ((os_name_pretty || os_name) && os_id && (os_image_version || 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, - /* key= */ 'l', - os_name_pretty ?: os_name, - path, - os_image_version ?: (os_version ?: (os_version_id ? : os_build_id))); - - config_entry_parse_tries(entry, L"\\EFI\\Linux", f->FileName, L".efi"); - - if (szs[SECTION_CMDLINE] == 0) - continue; + if (!bootspec_pick_name_version( + os_pretty_name, + os_image_id, + os_name, + os_id, + os_image_version, + os_version, + os_version_id, + os_build_id, + &good_name, + &good_version)) + continue; - FreePool(content); - content = NULL; + path = PoolPrint(L"\\EFI\\Linux\\%s", f->FileName); + if (!path) + return (void) log_oom(); + + entry = config_entry_add_loader( + config, + device, + LOADER_LINUX, + f->FileName, + /* key= */ 'l', + good_name, + path, + good_version); + if (!entry) + return (void) log_oom(); + + config_entry_parse_tries(entry, L"\\EFI\\Linux", f->FileName, L".efi"); + + if (szs[SECTION_CMDLINE] == 0) + continue; - /* read the embedded cmdline file */ - err = file_read(linux_dir, f->FileName, offs[SECTION_CMDLINE], szs[SECTION_CMDLINE], &content, NULL); - if (!EFI_ERROR(err)) { + content = mfree(content); - /* chomp the newline */ - if (content[szs[SECTION_CMDLINE] - 1] == '\n') - content[szs[SECTION_CMDLINE] - 1] = '\0'; + /* read the embedded cmdline file */ + err = file_read(linux_dir, f->FileName, offs[SECTION_CMDLINE], szs[SECTION_CMDLINE], &content, NULL); + if (!EFI_ERROR(err)) { + /* chomp the newline */ + if (content[szs[SECTION_CMDLINE] - 1] == '\n') + content[szs[SECTION_CMDLINE] - 1] = '\0'; - entry->options = stra_to_str(content); - } + entry->options = stra_to_str(content); + if (!entry->options) + return (void) log_oom(); } } } diff --git a/src/fundamental/bootspec-fundamental.c b/src/fundamental/bootspec-fundamental.c new file mode 100644 index 0000000000..51078397b6 --- /dev/null +++ b/src/fundamental/bootspec-fundamental.c @@ -0,0 +1,56 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "bootspec-fundamental.h" + +sd_bool bootspec_pick_name_version( + const sd_char *os_pretty_name, + const sd_char *os_image_id, + const sd_char *os_name, + const sd_char *os_id, + const sd_char *os_image_version, + const sd_char *os_version, + const sd_char *os_version_id, + const sd_char *os_build_id, + const sd_char **ret_name, + const sd_char **ret_version) { + + const sd_char *good_name, *good_version; + + /* Find the best human readable title and version string for a boot entry (using the os-release(5) + * fields). Precise is preferred over vague, and human readable over machine readable. Thus: + * + * 1. First priority gets the PRETTY_NAME field, which is the primary string intended for display, + * and should already contain both a nice description and a version indication (if that concept + * applies). + * + * 2. Otherwise we go for IMAGE_ID and IMAGE_VERSION (thus we show details about the image, + * i.e. specific combination of packages and configuration), if that concept applies. + * + * 3. Otherwise we go for NAME and VERSION (i.e. human readable OS name and version) + * + * 4. Otherwise we go for ID and VERSION_ID (i.e. machine readable OS name and version) + * + * 5. Finally, for the version we'll use BUILD_ID (i.e. a machine readable version that identifies + * the original OS build used during installation) + * + * Note that the display logic will show only the name by default, except if that isn't unique in + * which case the version is shown too. + * + * Note that name/version determined here are used only for display purposes. Boot entry preference + * sorting (i.e. algorithmic ordering of boot entries) is done based on the order of the entry "id" + * string (i.e. not on os-release(5) data). */ + + good_name = os_pretty_name ?: (os_image_id ?: (os_name ?: os_id)); + good_version = os_image_version ?: (os_version ?: (os_version_id ? : os_build_id)); + + if (!good_name || !good_version) + return false; + + if (ret_name) + *ret_name = good_name; + + if (ret_version) + *ret_version = good_version; + + return true; +} diff --git a/src/fundamental/bootspec-fundamental.h b/src/fundamental/bootspec-fundamental.h new file mode 100644 index 0000000000..0a1fe5c5ba --- /dev/null +++ b/src/fundamental/bootspec-fundamental.h @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +#pragma once + +#include "type.h" + +sd_bool bootspec_pick_name_version( + const sd_char *os_pretty_name, + const sd_char *os_image_id, + const sd_char *os_name, + const sd_char *os_id, + const sd_char *os_image_version, + const sd_char *os_version, + const sd_char *os_version_id, + const sd_char *os_build_id, + const sd_char **ret_name, + const sd_char **ret_version); diff --git a/src/fundamental/meson.build b/src/fundamental/meson.build index 3c9f07b191..26859653e6 100644 --- a/src/fundamental/meson.build +++ b/src/fundamental/meson.build @@ -3,13 +3,15 @@ fundamental_path = meson.current_source_dir() fundamental_headers = files( + 'bootspec-fundamental.h', 'efivars-fundamental.h', 'macro-fundamental.h', - 'string-util-fundamental.h', 'sha256.h', + 'string-util-fundamental.h', 'type.h') sources = ''' + bootspec-fundamental.c efivars-fundamental.c string-util-fundamental.c sha256.c -- cgit v1.2.1