From 9951736b7fe532f266ad8457a889047e1396fb76 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 3 Feb 2022 16:27:33 +0100 Subject: [PATCH 01/10] Revert "bootctl: Ignore boot entries (continue #22041)" --- src/boot/bootctl.c | 2 +- src/login/logind-dbus.c | 4 ++-- src/shared/bootspec.c | 10 +++------- src/shared/bootspec.h | 2 +- 4 files changed, 7 insertions(+), 11 deletions(-) diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index c76fea0a02..edc9ef4be9 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -1606,7 +1606,7 @@ static int verb_list(int argc, char *argv[], void *userdata) { else if (r < 0) log_warning_errno(r, "Failed to determine entries reported by boot loader, ignoring: %m"); else - (void) boot_entries_augment_from_loader(&config, efi_entries); + (void) boot_entries_augment_from_loader(&config, efi_entries, false); if (config.n_entries == 0) log_info("No boot loader entries found."); diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index e801f24532..72de58631a 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -2923,7 +2923,7 @@ static int boot_loader_entry_exists(Manager *m, const char *id) { r = manager_read_efi_boot_loader_entries(m); if (r >= 0) - (void) boot_entries_augment_from_loader(&config, m->efi_boot_loader_entries); + (void) boot_entries_augment_from_loader(&config, m->efi_boot_loader_entries, true); return boot_config_has_entry(&config, id); } @@ -3081,7 +3081,7 @@ static int property_get_boot_loader_entries( r = manager_read_efi_boot_loader_entries(m); if (r >= 0) - (void) boot_entries_augment_from_loader(&config, m->efi_boot_loader_entries); + (void) boot_entries_augment_from_loader(&config, m->efi_boot_loader_entries, true); r = sd_bus_message_open_container(reply, 'a', "s"); if (r < 0) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 52268f6041..0076092c2a 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -759,7 +759,8 @@ int boot_entries_load_config_auto( int boot_entries_augment_from_loader( BootConfig *config, - char **found_by_loader) { + char **found_by_loader, + bool only_auto) { static const char *const title_table[] = { /* Pretty names for a few well-known automatically discovered entries. */ @@ -784,12 +785,7 @@ int boot_entries_augment_from_loader( if (boot_config_has_entry(config, *i)) continue; - /* - * consider the 'auto-' entries only, because the others - * ones are detected scanning the 'esp' and 'xbootldr' - * directories by boot_entries_load_config() - */ - if (!startswith(*i, "auto-")) + if (only_auto && !startswith(*i, "auto-")) continue; c = strdup(*i); diff --git a/src/shared/bootspec.h b/src/shared/bootspec.h index 4a95e24e27..81845f47e3 100644 --- a/src/shared/bootspec.h +++ b/src/shared/bootspec.h @@ -76,7 +76,7 @@ static inline BootEntry* boot_config_default_entry(BootConfig *config) { void boot_config_free(BootConfig *config); int boot_entries_load_config(const char *esp_path, const char *xbootldr_path, BootConfig *config); int boot_entries_load_config_auto(const char *override_esp_path, const char *override_xbootldr_path, BootConfig *config); -int boot_entries_augment_from_loader(BootConfig *config, char **list); +int boot_entries_augment_from_loader(BootConfig *config, char **list, bool only_auto); static inline const char* boot_entry_title(const BootEntry *entry) { return entry->show_title ?: entry->title ?: entry->id; From 736783d42035e0e654508cf3ea0ae95e76dbc1f5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Feb 2022 14:05:01 +0100 Subject: [PATCH 02/10] bootspec: port one more use of basename() to path_extract_filename() --- src/shared/bootspec.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 0076092c2a..1f2bab3fb8 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -64,26 +64,28 @@ static int boot_entry_load( _cleanup_fclose_ FILE *f = NULL; unsigned line = 1; - char *b, *c; + char *c; int r; assert(root); assert(path); assert(entry); - c = endswith_no_case(path, ".conf"); - if (!c) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid loader entry file suffix: %s", path); + r = path_extract_filename(path, &tmp.id); + if (r < 0) + return log_error_errno(r, "Failed to extract file name from path '%s': %m", path); - b = basename(path); - tmp.id = strdup(b); - tmp.id_old = strndup(b, c - b); - if (!tmp.id || !tmp.id_old) - return log_oom(); + c = endswith_no_case(tmp.id, ".conf"); + if (!c) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid loader entry file suffix: %s", tmp.id); if (!efi_loader_entry_name_valid(tmp.id)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid loader entry name: %s", tmp.id); + tmp.id_old = strndup(tmp.id, c - tmp.id); + if (!tmp.id_old) + return log_oom(); + tmp.path = strdup(path); if (!tmp.path) return log_oom(); From 4cddc18d0ac8127b8cad47d250563dc791c41c10 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Feb 2022 14:05:15 +0100 Subject: [PATCH 03/10] update TODO --- TODO | 2 ++ 1 file changed, 2 insertions(+) diff --git a/TODO b/TODO index 777279f1ee..ea17d9ad9a 100644 --- a/TODO +++ b/TODO @@ -78,6 +78,8 @@ Janitorial Clean-ups: Features: +* bootspec: remove tries counter from boot entry ids + * automatically ignore threaded cgroups in cg_xyz(). * add linker script that implicitly adds symbol for build ID and new coredump From fdc5c0429982a0b072736f0031bd61caf48a4193 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Feb 2022 14:12:09 +0100 Subject: [PATCH 04/10] bootspec: parse/show devicetree-overlay field too It has been defined in the boot loader spec, and is the only field we currently don't parse, hence fix that. --- src/boot/bootctl.c | 8 ++++++++ src/shared/bootspec.c | 11 ++++++++++- src/shared/bootspec.h | 1 + 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index edc9ef4be9..d94a2a586b 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -450,6 +450,7 @@ static int boot_entry_show(const BootEntry *e, bool show_as_default) { e->root, *s, &status); + if (!strv_isempty(e->options)) { _cleanup_free_ char *t = NULL, *t2 = NULL; _cleanup_strv_free_ char **ts = NULL; @@ -468,9 +469,16 @@ static int boot_entry_show(const BootEntry *e, bool show_as_default) { printf(" options: %s\n", t2); } + if (e->device_tree) boot_entry_file_list("devicetree", e->root, e->device_tree, &status); + STRV_FOREACH(s, e->device_tree_overlay) + boot_entry_file_list(s == e->device_tree_overlay ? "devicetree-overlay" : NULL, + e->root, + *s, + &status); + return -status; } diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 1f2bab3fb8..cbc356e472 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -51,6 +51,7 @@ static void boot_entry_free(BootEntry *entry) { free(entry->efi); strv_free(entry->initrd); free(entry->device_tree); + strv_free(entry->device_tree_overlay); } static int boot_entry_load( @@ -144,7 +145,15 @@ static int boot_entry_load( r = strv_extend(&tmp.initrd, p); else if (streq(field, "devicetree")) r = free_and_strdup(&tmp.device_tree, p); - else { + else if (streq(field, "devicetree-overlay")) { + _cleanup_strv_free_ char **l = NULL; + + l = strv_split(p, NULL); + if (!l) + return log_oom(); + + r = strv_extend_strv(&tmp.device_tree_overlay, l, false); + } else { log_notice("%s:%u: Unknown line \"%s\", ignoring.", path, line, field); continue; } diff --git a/src/shared/bootspec.h b/src/shared/bootspec.h index 81845f47e3..8032c99ed5 100644 --- a/src/shared/bootspec.h +++ b/src/shared/bootspec.h @@ -34,6 +34,7 @@ typedef struct BootEntry { char *efi; char **initrd; char *device_tree; + char **device_tree_overlay; } BootEntry; typedef struct BootConfig { From d403d8f0d65bdcecf7555a0cf040aa6de3de666e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Feb 2022 14:18:18 +0100 Subject: [PATCH 05/10] bootspec: also parse new 'beep' loader.conf variable --- src/shared/bootspec.c | 3 +++ src/shared/bootspec.h | 1 + 2 files changed, 4 insertions(+) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index cbc356e472..65a040f26d 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -178,6 +178,7 @@ void boot_config_free(BootConfig *config) { free(config->auto_firmware); free(config->console_mode); free(config->random_seed_mode); + free(config->beep); free(config->entry_oneshot); free(config->entry_default); @@ -245,6 +246,8 @@ static int boot_loader_read_conf(const char *path, BootConfig *config) { r = free_and_strdup(&config->console_mode, p); else if (streq(field, "random-seed-mode")) r = free_and_strdup(&config->random_seed_mode, p); + else if (streq(field, "beep")) + r = free_and_strdup(&config->beep, p); else { log_notice("%s:%u: Unknown line \"%s\", ignoring.", path, line, field); continue; diff --git a/src/shared/bootspec.h b/src/shared/bootspec.h index 8032c99ed5..62b3f6ce5f 100644 --- a/src/shared/bootspec.h +++ b/src/shared/bootspec.h @@ -45,6 +45,7 @@ typedef struct BootConfig { char *auto_firmware; char *console_mode; char *random_seed_mode; + char *beep; char *entry_oneshot; char *entry_default; From a78e472dfd7832a13e3d52797e672d4e77fc2a49 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Feb 2022 14:41:00 +0100 Subject: [PATCH 06/10] bootspec: also collect/mark the "selected" boot entry (i.e. the one currently booted) it's helpful and easy, so let's do it --- src/boot/bootctl.c | 18 +++++++---- src/shared/bootspec.c | 70 +++++++++++++++++++++++++++++++++---------- src/shared/bootspec.h | 2 ++ 3 files changed, 70 insertions(+), 20 deletions(-) diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index d94a2a586b..adc66788f8 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -411,7 +411,11 @@ static void boot_entry_file_list(const char *field, const char *root, const char *ret_status = status; } -static int boot_entry_show(const BootEntry *e, bool show_as_default) { +static int boot_entry_show( + const BootEntry *e, + bool show_as_default, + bool show_as_selected) { + int status = 0; /* Returns 0 on success, negative on processing error, and positive if something is wrong with the @@ -419,9 +423,10 @@ static int boot_entry_show(const BootEntry *e, bool show_as_default) { assert(e); - printf(" title: %s%s%s" "%s%s%s\n", + printf(" title: %s%s%s" "%s%s%s" "%s%s%s\n", ansi_highlight(), boot_entry_title(e), ansi_normal(), - ansi_highlight_green(), show_as_default ? " (default)" : "", ansi_normal()); + ansi_highlight_green(), show_as_default ? " (default)" : "", ansi_normal(), + ansi_highlight_magenta(), show_as_selected ? " (selected)" : "", ansi_normal()); if (e->id) printf(" id: %s\n", e->id); @@ -519,7 +524,7 @@ static int status_entries( else { printf("Default Boot Loader Entry:\n"); - r = boot_entry_show(config.entries + config.default_entry, false); + r = boot_entry_show(config.entries + config.default_entry, /* show_as_default= */ false, /* show_as_selected= */ false); if (r > 0) /* < 0 is already logged by the function itself, let's just emit an extra warning if the default entry is broken */ @@ -1624,7 +1629,10 @@ static int verb_list(int argc, char *argv[], void *userdata) { printf("Boot Loader Entries:\n"); for (size_t n = 0; n < config.n_entries; n++) { - r = boot_entry_show(config.entries + n, n == (size_t) config.default_entry); + r = boot_entry_show( + config.entries + n, + n == (size_t) config.default_entry, + n == (size_t) config.selected_entry); if (r < 0) return r; diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 65a040f26d..2d1d2b440b 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -182,6 +182,7 @@ void boot_config_free(BootConfig *config) { free(config->entry_oneshot); free(config->entry_default); + free(config->entry_selected); for (i = 0; i < config->n_entries; i++) boot_entry_free(config->entries + i); @@ -669,6 +670,55 @@ static int boot_entries_select_default(const BootConfig *config) { return config->n_entries - 1; } +static int boot_entries_select_selected(const BootConfig *config) { + + assert(config); + assert(config->entries || config->n_entries == 0); + + if (!config->entry_selected || config->n_entries == 0) + return -1; + + for (int i = config->n_entries - 1; i >= 0; i--) + if (streq(config->entry_selected, config->entries[i].id)) + return i; + + return -1; +} + +static int boot_load_efi_entry_pointers(BootConfig *config) { + int r; + + assert(config); + + if (!is_efi_boot()) + return 0; + + /* Loads the three "pointers" to boot loader entries from their EFI variables */ + + r = efi_get_variable_string(EFI_LOADER_VARIABLE(LoaderEntryOneShot), &config->entry_oneshot); + if (r < 0 && !IN_SET(r, -ENOENT, -ENODATA)) { + log_warning_errno(r, "Failed to read EFI variable \"LoaderEntryOneShot\": %m"); + if (r == -ENOMEM) + return r; + } + + r = efi_get_variable_string(EFI_LOADER_VARIABLE(LoaderEntryDefault), &config->entry_default); + if (r < 0 && !IN_SET(r, -ENOENT, -ENODATA)) { + log_warning_errno(r, "Failed to read EFI variable \"LoaderEntryDefault\": %m"); + if (r == -ENOMEM) + return r; + } + + r = efi_get_variable_string(EFI_LOADER_VARIABLE(LoaderEntrySelected), &config->entry_selected); + if (r < 0 && !IN_SET(r, -ENOENT, -ENODATA)) { + log_warning_errno(r, "Failed to read EFI variable \"LoaderEntrySelected\": %m"); + if (r == -ENOMEM) + return r; + } + + return 1; +} + int boot_entries_load_config( const char *esp_path, const char *xbootldr_path, @@ -714,23 +764,13 @@ int boot_entries_load_config( if (r < 0) return log_error_errno(r, "Failed to uniquify boot entries: %m"); - if (is_efi_boot()) { - r = efi_get_variable_string(EFI_LOADER_VARIABLE(LoaderEntryOneShot), &config->entry_oneshot); - if (r < 0 && !IN_SET(r, -ENOENT, -ENODATA)) { - log_warning_errno(r, "Failed to read EFI variable \"LoaderEntryOneShot\": %m"); - if (r == -ENOMEM) - return r; - } - - r = efi_get_variable_string(EFI_LOADER_VARIABLE(LoaderEntryDefault), &config->entry_default); - if (r < 0 && !IN_SET(r, -ENOENT, -ENODATA)) { - log_warning_errno(r, "Failed to read EFI variable \"LoaderEntryDefault\": %m"); - if (r == -ENOMEM) - return r; - } - } + r = boot_load_efi_entry_pointers(config); + if (r < 0) + return r; config->default_entry = boot_entries_select_default(config); + config->selected_entry = boot_entries_select_selected(config); + return 0; } diff --git a/src/shared/bootspec.h b/src/shared/bootspec.h index 62b3f6ce5f..8649e93bce 100644 --- a/src/shared/bootspec.h +++ b/src/shared/bootspec.h @@ -49,10 +49,12 @@ typedef struct BootConfig { char *entry_oneshot; char *entry_default; + char *entry_selected; BootEntry *entries; size_t n_entries; ssize_t default_entry; + ssize_t selected_entry; } BootConfig; static inline bool boot_config_has_entry(BootConfig *config, const char *id) { From bb6820576870d0b38dbf9f6e489b126558fc87d9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Feb 2022 21:15:22 +0100 Subject: [PATCH 07/10] bootctl: show more information about boot entry state in list Let's improve display of boot entries and show what type they have (i.e. boot loader spec type 1, or type 2, or auto-discovered or reported by boot loader), and in particular mark entries the boot loader discovered but we can't find (i.e. that likely vanished, or possibly couldn't be found due to a misconfiguration) and that the boot loader didn't find but we see (which are new, or possibly also the result of misconfiguraiton). This is supposed to be a replacement for #22161, but instead of hiding vanished entries, highlights them, which I think is more appropriate for a low-level tool such bootctl. Replaces: #22161 #22398 --- src/boot/bootctl.c | 54 ++++++++++++++++++++++++++++++++++------- src/login/logind-dbus.c | 6 ++--- src/shared/bootspec.c | 11 ++++++--- src/shared/bootspec.h | 33 ++++++++++++++----------- 4 files changed, 75 insertions(+), 29 deletions(-) diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index adc66788f8..ab514d28ee 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -36,6 +36,7 @@ #include "rm-rf.h" #include "stat-util.h" #include "stdio-util.h" +#include "string-table.h" #include "string-util.h" #include "strv.h" #include "sync-util.h" @@ -411,10 +412,20 @@ static void boot_entry_file_list(const char *field, const char *root, const char *ret_status = status; } +static const char* const boot_entry_type_table[_BOOT_ENTRY_TYPE_MAX] = { + [BOOT_ENTRY_CONF] = "Boot Loader Specification Type #1 (.conf)", + [BOOT_ENTRY_UNIFIED] = "Boot Loader Specification Type #2 (.efi)", + [BOOT_ENTRY_LOADER] = "Reported by Boot Loader", + [BOOT_ENTRY_LOADER_AUTO] = "Automatic", +}; + +DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(boot_entry_type, BootEntryType); + static int boot_entry_show( const BootEntry *e, bool show_as_default, - bool show_as_selected) { + bool show_as_selected, + bool show_reported) { int status = 0; @@ -423,10 +434,30 @@ static int boot_entry_show( assert(e); - printf(" title: %s%s%s" "%s%s%s" "%s%s%s\n", - ansi_highlight(), boot_entry_title(e), ansi_normal(), - ansi_highlight_green(), show_as_default ? " (default)" : "", ansi_normal(), - ansi_highlight_magenta(), show_as_selected ? " (selected)" : "", ansi_normal()); + printf(" type: %s\n", + boot_entry_type_to_string(e->type)); + + printf(" title: %s%s%s", + ansi_highlight(), boot_entry_title(e), ansi_normal()); + + if (show_as_default) + printf(" %s(default)%s", + ansi_highlight_green(), ansi_normal()); + + if (show_as_selected) + printf(" %s(selected)%s", + ansi_highlight_magenta(), ansi_normal()); + + if (show_reported) { + if (e->type == BOOT_ENTRY_LOADER) + printf(" %s(reported/absent)%s", + ansi_highlight_red(), ansi_normal()); + else if (!e->reported_by_loader && e->type != BOOT_ENTRY_LOADER_AUTO) + printf(" %s(not reported/new)%s", + ansi_highlight_green(), ansi_normal()); + } + + putchar('\n'); if (e->id) printf(" id: %s\n", e->id); @@ -524,7 +555,11 @@ static int status_entries( else { printf("Default Boot Loader Entry:\n"); - r = boot_entry_show(config.entries + config.default_entry, /* show_as_default= */ false, /* show_as_selected= */ false); + r = boot_entry_show( + config.entries + config.default_entry, + /* show_as_default= */ false, + /* show_as_selected= */ false, + /* show_discovered= */ false); if (r > 0) /* < 0 is already logged by the function itself, let's just emit an extra warning if the default entry is broken */ @@ -1619,7 +1654,7 @@ static int verb_list(int argc, char *argv[], void *userdata) { else if (r < 0) log_warning_errno(r, "Failed to determine entries reported by boot loader, ignoring: %m"); else - (void) boot_entries_augment_from_loader(&config, efi_entries, false); + (void) boot_entries_augment_from_loader(&config, efi_entries, /* only_auto= */ false); if (config.n_entries == 0) log_info("No boot loader entries found."); @@ -1631,8 +1666,9 @@ static int verb_list(int argc, char *argv[], void *userdata) { for (size_t n = 0; n < config.n_entries; n++) { r = boot_entry_show( config.entries + n, - n == (size_t) config.default_entry, - n == (size_t) config.selected_entry); + /* show_as_default= */ n == (size_t) config.default_entry, + /* show_as_selected= */ n == (size_t) config.selected_entry, + /* show_discovered= */ true); if (r < 0) return r; diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 72de58631a..32d619eecf 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -2923,9 +2923,9 @@ static int boot_loader_entry_exists(Manager *m, const char *id) { r = manager_read_efi_boot_loader_entries(m); if (r >= 0) - (void) boot_entries_augment_from_loader(&config, m->efi_boot_loader_entries, true); + (void) boot_entries_augment_from_loader(&config, m->efi_boot_loader_entries, /* auto_only= */ true); - return boot_config_has_entry(&config, id); + return !!boot_config_find_entry(&config, id); } static int method_set_reboot_to_boot_loader_entry( @@ -3081,7 +3081,7 @@ static int property_get_boot_loader_entries( r = manager_read_efi_boot_loader_entries(m); if (r >= 0) - (void) boot_entries_augment_from_loader(&config, m->efi_boot_loader_entries, true); + (void) boot_entries_augment_from_loader(&config, m->efi_boot_loader_entries, /* auto_only= */ true); r = sd_bus_message_open_container(reply, 'a', "s"); if (r < 0) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 2d1d2b440b..15b77f01b2 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -561,7 +561,7 @@ static int boot_entries_find_unified( return 0; } -static bool find_nonunique(BootEntry *entries, size_t n_entries, bool *arr) { +static bool find_nonunique(const BootEntry *entries, size_t n_entries, bool arr[]) { size_t i, j; bool non_unique = false; @@ -833,11 +833,15 @@ int boot_entries_augment_from_loader( * already included there. */ STRV_FOREACH(i, found_by_loader) { + BootEntry *existing; _cleanup_free_ char *c = NULL, *t = NULL, *p = NULL; char **a, **b; - if (boot_config_has_entry(config, *i)) + existing = boot_config_find_entry(config, *i); + if (existing) { + existing->reported_by_loader = true; continue; + } if (only_auto && !startswith(*i, "auto-")) continue; @@ -862,10 +866,11 @@ int boot_entries_augment_from_loader( return log_oom(); config->entries[config->n_entries++] = (BootEntry) { - .type = BOOT_ENTRY_LOADER, + .type = startswith(*i, "auto-") ? BOOT_ENTRY_LOADER_AUTO : BOOT_ENTRY_LOADER, .id = TAKE_PTR(c), .title = TAKE_PTR(t), .path = TAKE_PTR(p), + .reported_by_loader = true, }; } diff --git a/src/shared/bootspec.h b/src/shared/bootspec.h index 8649e93bce..6f1014db4a 100644 --- a/src/shared/bootspec.h +++ b/src/shared/bootspec.h @@ -11,15 +11,17 @@ #include "string-util.h" typedef enum BootEntryType { - BOOT_ENTRY_CONF, /* Type #1 entries: *.conf files */ - BOOT_ENTRY_UNIFIED, /* Type #2 entries: *.efi files */ - BOOT_ENTRY_LOADER, /* Additional entries augmented from LoaderEntries EFI var */ - _BOOT_ENTRY_MAX, - _BOOT_ENTRY_INVALID = -EINVAL, + BOOT_ENTRY_CONF, /* Boot Loader Specification Type #1 entries: *.conf files */ + BOOT_ENTRY_UNIFIED, /* Boot Loader Specification Type #2 entries: *.efi files */ + BOOT_ENTRY_LOADER, /* Additional entries augmented from LoaderEntries EFI variable (regular entries) */ + BOOT_ENTRY_LOADER_AUTO, /* Additional entries augmented from LoaderEntries EFI variable (special "automatic" entries) */ + _BOOT_ENTRY_TYPE_MAX, + _BOOT_ENTRY_TYPE_INVALID = -EINVAL, } BootEntryType; typedef struct BootEntry { BootEntryType type; + bool reported_by_loader; char *id; /* This is the file basename (including extension!) */ char *id_old; /* Old-style ID, for deduplication purposes. */ char *path; /* This is the full path to the drop-in file */ @@ -57,20 +59,21 @@ typedef struct BootConfig { ssize_t selected_entry; } BootConfig; -static inline bool boot_config_has_entry(BootConfig *config, const char *id) { - size_t j; +static inline BootEntry* boot_config_find_entry(BootConfig *config, const char *id) { + assert(config); + assert(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; - } + for (size_t j = 0; j < config->n_entries; j++) + if (streq_ptr(config->entries[j].id, id) || + streq_ptr(config->entries[j].id_old, id)) + return config->entries + j; - return false; + return NULL; } static inline BootEntry* boot_config_default_entry(BootConfig *config) { + assert(config); + if (config->default_entry < 0) return NULL; @@ -83,6 +86,8 @@ int boot_entries_load_config_auto(const char *override_esp_path, const char *ove int boot_entries_augment_from_loader(BootConfig *config, char **list, bool only_auto); static inline const char* boot_entry_title(const BootEntry *entry) { + assert(entry); + return entry->show_title ?: entry->title ?: entry->id; } From 56350400918c6979c60d46b7825e9671ee31f09c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Feb 2022 22:19:35 +0100 Subject: [PATCH 08/10] bootspec: make sure all return values are initialized on return of find_esp_and_warn() THis makes sure that find_esp_and_warn() + find_xbootldr_and_warn() follow our usual coding style that on success all return values are initialized. We got that right in most successful codepaths out of these functions, but missed the one where the paths are manually overwritten via env vars. --- src/shared/bootspec.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 15b77f01b2..7c74cf6f18 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -1263,6 +1263,16 @@ int find_esp_and_warn( /* Note: when the user explicitly configured things with an env var we won't validate the mount * point. After all we want this to be useful for testing. */ + + if (ret_part) + *ret_part = 0; + if (ret_pstart) + *ret_pstart = 0; + if (ret_psize) + *ret_psize = 0; + if (ret_uuid) + *ret_uuid = SD_ID128_NULL; + goto found; } @@ -1489,6 +1499,9 @@ int find_xbootldr_and_warn( "$SYSTEMD_XBOOTLDR_PATH does not refer to absolute path, refusing to use it: %s", path); + if (ret_uuid) + *ret_uuid = SD_ID128_NULL; + goto found; } From f63b5ad93592ff64b5e8a83b63f2a3daade28114 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Feb 2022 22:23:37 +0100 Subject: [PATCH 09/10] boot: suppress XBOOTLDR if same device as ESP when enumerating entries On my local system I linked up the ESP and XBOOTLDR partitions, and ended up with duplicate entries being listed. Try hard to detect that and only enumerate entries in the ESP if it turns out that both dirs have the same dev_t. This should detect both bind mounted and symlinked cases and should make our list output less confusing. --- src/basic/stat-util.h | 6 ++++ src/boot/bless-boot.c | 12 ++++--- src/boot/bootctl.c | 54 ++++++++++++++++++++---------- src/shared/bootspec.c | 78 ++++++++++++++++++++++++++++++++++--------- src/shared/bootspec.h | 4 +-- 5 files changed, 115 insertions(+), 39 deletions(-) diff --git a/src/basic/stat-util.h b/src/basic/stat-util.h index f7d2f12aa9..f9b3d2fbed 100644 --- a/src/basic/stat-util.h +++ b/src/basic/stat-util.h @@ -114,3 +114,9 @@ int statx_fallback(int dfd, const char *path, int flags, unsigned mask, struct s struct new_statx nsx; \ } var #endif + +static inline bool devid_set_and_equal(dev_t a, dev_t b) { + /* Returns true if a and b definitely refer to the same device. If either is zero, this means "don't + * know" and we'll return false */ + return a == b && a != 0; +} diff --git a/src/boot/bless-boot.c b/src/boot/bless-boot.c index 9e4b0d1f72..2cbabc6c59 100644 --- a/src/boot/bless-boot.c +++ b/src/boot/bless-boot.c @@ -14,6 +14,7 @@ #include "parse-util.h" #include "path-util.h" #include "pretty-print.h" +#include "stat-util.h" #include "sync-util.h" #include "terminal-util.h" #include "util.h" @@ -98,17 +99,18 @@ static int parse_argv(int argc, char *argv[]) { static int acquire_path(void) { _cleanup_free_ char *esp_path = NULL, *xbootldr_path = NULL; + dev_t esp_devid = 0, xbootldr_devid = 0; char **a; int r; if (!strv_isempty(arg_path)) return 0; - r = find_esp_and_warn(NULL, false, &esp_path, NULL, NULL, NULL, NULL); + r = find_esp_and_warn(NULL, /* unprivileged_mode= */ false, &esp_path, NULL, NULL, NULL, NULL, &esp_devid); if (r < 0 && r != -ENOKEY) /* ENOKEY means not found, and is the only error the function won't log about on its own */ return r; - r = find_xbootldr_and_warn(NULL, false, &xbootldr_path, NULL); + r = find_xbootldr_and_warn(NULL, /* unprivileged_mode= */ false, &xbootldr_path, NULL, &xbootldr_devid); if (r < 0 && r != -ENOKEY) return r; @@ -117,8 +119,10 @@ static int acquire_path(void) { "Couldn't find $BOOT partition. It is recommended to mount it to /boot.\n" "Alternatively, use --path= to specify path to mount point."); - if (esp_path) + if (esp_path && xbootldr_path && !devid_set_and_equal(esp_devid, xbootldr_devid)) /* in case the two paths refer to the same inode, suppress one */ a = strv_new(esp_path, xbootldr_path); + else if (esp_path) + a = strv_new(esp_path); else a = strv_new(xbootldr_path); if (!a) @@ -130,7 +134,7 @@ static int acquire_path(void) { _cleanup_free_ char *j = NULL; j = strv_join(arg_path, ":"); - log_debug("Using %s as boot loader drop-in search path.", j); + log_debug("Using %s as boot loader drop-in search path.", strna(j)); } return 0; diff --git a/src/boot/bootctl.c b/src/boot/bootctl.c index ab514d28ee..c81411c857 100644 --- a/src/boot/bootctl.c +++ b/src/boot/bootctl.c @@ -75,7 +75,8 @@ static int acquire_esp( uint32_t *ret_part, uint64_t *ret_pstart, uint64_t *ret_psize, - sd_id128_t *ret_uuid) { + sd_id128_t *ret_uuid, + dev_t *ret_devid) { char *np; int r; @@ -86,7 +87,7 @@ static int acquire_esp( * we simply eat up the error here, so that --list and --status work too, without noise about * this). */ - r = find_esp_and_warn(arg_esp_path, unprivileged_mode, &np, ret_part, ret_pstart, ret_psize, ret_uuid); + r = find_esp_and_warn(arg_esp_path, unprivileged_mode, &np, ret_part, ret_pstart, ret_psize, ret_uuid, ret_devid); if (r == -ENOKEY) { if (graceful) return log_info_errno(r, "Couldn't find EFI system partition, skipping."); @@ -104,16 +105,23 @@ static int acquire_esp( return 1; } -static int acquire_xbootldr(bool unprivileged_mode, sd_id128_t *ret_uuid) { +static int acquire_xbootldr( + bool unprivileged_mode, + sd_id128_t *ret_uuid, + dev_t *ret_devid) { + char *np; int r; - r = find_xbootldr_and_warn(arg_xbootldr_path, unprivileged_mode, &np, ret_uuid); + r = find_xbootldr_and_warn(arg_xbootldr_path, unprivileged_mode, &np, ret_uuid, ret_devid); if (r == -ENOKEY) { log_debug_errno(r, "Didn't find an XBOOTLDR partition, using the ESP as $BOOT."); + arg_xbootldr_path = mfree(arg_xbootldr_path); + if (ret_uuid) *ret_uuid = SD_ID128_NULL; - arg_xbootldr_path = mfree(arg_xbootldr_path); + if (ret_devid) + *ret_devid = 0; return 0; } if (r < 0) @@ -1436,7 +1444,7 @@ static void print_yes_no_line(bool first, bool good, const char *name) { static int are_we_installed(void) { int r; - r = acquire_esp(/* privileged_mode= */ false, /* graceful= */ false, NULL, NULL, NULL, NULL); + r = acquire_esp(/* privileged_mode= */ false, /* graceful= */ false, NULL, NULL, NULL, NULL, NULL); if (r < 0) return r; @@ -1468,9 +1476,10 @@ static int are_we_installed(void) { static int verb_status(int argc, char *argv[], void *userdata) { sd_id128_t esp_uuid = SD_ID128_NULL, xbootldr_uuid = SD_ID128_NULL; + dev_t esp_devid = 0, xbootldr_devid = 0; int r, k; - r = acquire_esp(/* unprivileged_mode= */ geteuid() != 0, /* graceful= */ false, NULL, NULL, NULL, &esp_uuid); + r = acquire_esp(/* unprivileged_mode= */ geteuid() != 0, /* graceful= */ false, NULL, NULL, NULL, &esp_uuid, &esp_devid); if (arg_print_esp_path) { if (r == -EACCES) /* If we couldn't acquire the ESP path, log about access errors (which is the only * error the find_esp_and_warn() won't log on its own) */ @@ -1481,7 +1490,7 @@ static int verb_status(int argc, char *argv[], void *userdata) { puts(arg_esp_path); } - r = acquire_xbootldr(/* unprivileged_mode= */ geteuid() != 0, &xbootldr_uuid); + r = acquire_xbootldr(/* unprivileged_mode= */ geteuid() != 0, &xbootldr_uuid, &xbootldr_devid); if (arg_print_dollar_boot_path) { if (r == -EACCES) return log_error_errno(r, "Failed to determine XBOOTLDR location: %m"); @@ -1615,7 +1624,14 @@ static int verb_status(int argc, char *argv[], void *userdata) { } if (arg_esp_path || arg_xbootldr_path) { - k = status_entries(arg_esp_path, esp_uuid, arg_xbootldr_path, xbootldr_uuid); + /* If XBOOTLDR and ESP actually refer to the same block device, suppress XBOOTLDR, since it would find the same entries twice */ + bool same = arg_esp_path && arg_xbootldr_path && devid_set_and_equal(esp_devid, xbootldr_devid); + + k = status_entries( + arg_esp_path, + esp_uuid, + same ? NULL : arg_xbootldr_path, + same ? SD_ID128_NULL : xbootldr_uuid); if (k < 0) r = k; } @@ -1626,25 +1642,29 @@ static int verb_status(int argc, char *argv[], void *userdata) { static int verb_list(int argc, char *argv[], void *userdata) { _cleanup_(boot_config_free) BootConfig config = {}; _cleanup_strv_free_ char **efi_entries = NULL; + dev_t esp_devid = 0, xbootldr_devid = 0; int r; /* If we lack privileges we invoke find_esp_and_warn() in "unprivileged mode" here, which does two things: turn * off logging about access errors and turn off potentially privileged device probing. Here we're interested in * the latter but not the former, hence request the mode, and log about EACCES. */ - r = acquire_esp(/* unprivileged_mode= */ geteuid() != 0, /* graceful= */ false, NULL, NULL, NULL, NULL); + r = acquire_esp(/* unprivileged_mode= */ geteuid() != 0, /* graceful= */ false, NULL, NULL, NULL, NULL, &esp_devid); if (r == -EACCES) /* We really need the ESP path for this call, hence also log about access errors */ return log_error_errno(r, "Failed to determine ESP: %m"); if (r < 0) return r; - r = acquire_xbootldr(/* unprivileged_mode= */ geteuid() != 0, NULL); + r = acquire_xbootldr(/* unprivileged_mode= */ geteuid() != 0, NULL, &xbootldr_devid); if (r == -EACCES) return log_error_errno(r, "Failed to determine XBOOTLDR partition: %m"); if (r < 0) return r; - r = boot_entries_load_config(arg_esp_path, arg_xbootldr_path, &config); + /* If XBOOTLDR and ESP actually refer to the same block device, suppress XBOOTLDR, since it would find the same entries twice */ + bool same = arg_esp_path && arg_xbootldr_path && devid_set_and_equal(esp_devid, xbootldr_devid); + + r = boot_entries_load_config(arg_esp_path, same ? NULL : arg_xbootldr_path, &config); if (r < 0) return r; @@ -1840,7 +1860,7 @@ static int verb_install(int argc, char *argv[], void *userdata) { install = streq(argv[0], "install"); graceful = !install && arg_graceful; /* support graceful mode for updates */ - r = acquire_esp(/* unprivileged_mode= */ false, graceful, &part, &pstart, &psize, &uuid); + r = acquire_esp(/* unprivileged_mode= */ false, graceful, &part, &pstart, &psize, &uuid, NULL); if (graceful && r == -ENOKEY) return 0; /* If --graceful is specified and we can't find an ESP, handle this cleanly */ if (r < 0) @@ -1857,7 +1877,7 @@ static int verb_install(int argc, char *argv[], void *userdata) { } } - r = acquire_xbootldr(/* unprivileged_mode= */ false, NULL); + r = acquire_xbootldr(/* unprivileged_mode= */ false, NULL, NULL); if (r < 0) return r; @@ -1917,11 +1937,11 @@ static int verb_remove(int argc, char *argv[], void *userdata) { sd_id128_t uuid = SD_ID128_NULL; int r, q; - r = acquire_esp(/* unprivileged_mode= */ false, /* graceful= */ false, NULL, NULL, NULL, &uuid); + r = acquire_esp(/* unprivileged_mode= */ false, /* graceful= */ false, NULL, NULL, NULL, &uuid, NULL); if (r < 0) return r; - r = acquire_xbootldr(/* unprivileged_mode= */ false, NULL); + r = acquire_xbootldr(/* unprivileged_mode= */ false, NULL, NULL); if (r < 0) return r; @@ -2130,7 +2150,7 @@ static int verb_set_efivar(int argc, char *argv[], void *userdata) { static int verb_random_seed(int argc, char *argv[], void *userdata) { int r; - r = find_esp_and_warn(arg_esp_path, false, &arg_esp_path, NULL, NULL, NULL, NULL); + r = find_esp_and_warn(arg_esp_path, false, &arg_esp_path, NULL, NULL, NULL, NULL, NULL); if (r == -ENOKEY) { /* find_esp_and_warn() doesn't warn about ENOKEY, so let's do that on our own */ if (!arg_graceful) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 7c74cf6f18..1dbeda23d9 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -780,6 +780,7 @@ int boot_entries_load_config_auto( BootConfig *config) { _cleanup_free_ char *esp_where = NULL, *xbootldr_where = NULL; + dev_t esp_devid = 0, xbootldr_devid = 0; int r; assert(config); @@ -800,14 +801,18 @@ int boot_entries_load_config_auto( "Failed to determine whether /run/boot-loader-entries/ exists: %m"); } - r = find_esp_and_warn(override_esp_path, false, &esp_where, NULL, NULL, NULL, NULL); + r = find_esp_and_warn(override_esp_path, /* unprivileged_mode= */ false, &esp_where, NULL, NULL, NULL, NULL, &esp_devid); if (r < 0) /* we don't log about ENOKEY here, but propagate it, leaving it to the caller to log */ return r; - r = find_xbootldr_and_warn(override_xbootldr_path, false, &xbootldr_where, NULL); + r = find_xbootldr_and_warn(override_xbootldr_path, /* unprivileged_mode= */ false, &xbootldr_where, NULL, &xbootldr_devid); if (r < 0 && r != -ENOKEY) return r; /* It's fine if the XBOOTLDR partition doesn't exist, hence we ignore ENOKEY here */ + /* If both paths actually refer to the same inode, suppress the xbootldr path */ + if (esp_where && xbootldr_where && devid_set_and_equal(esp_devid, xbootldr_devid)) + xbootldr_where = mfree(xbootldr_where); + return boot_entries_load_config(esp_where, xbootldr_where, config); } @@ -1162,7 +1167,8 @@ static int verify_esp( uint32_t *ret_part, uint64_t *ret_pstart, uint64_t *ret_psize, - sd_id128_t *ret_uuid) { + sd_id128_t *ret_uuid, + dev_t *ret_devid) { bool relax_checks; dev_t devid; @@ -1212,9 +1218,16 @@ static int verify_esp( * however blkid can't work if we have no privileges to access block devices directly, which is why * we use udev in that case. */ if (unprivileged_mode) - return verify_esp_udev(devid, searching, ret_part, ret_pstart, ret_psize, ret_uuid); + r = verify_esp_udev(devid, searching, ret_part, ret_pstart, ret_psize, ret_uuid); else - return verify_esp_blkid(devid, searching, ret_part, ret_pstart, ret_psize, ret_uuid); + r = verify_esp_blkid(devid, searching, ret_part, ret_pstart, ret_psize, ret_uuid); + if (r < 0) + return r; + + if (ret_devid) + *ret_devid = devid; + + return 0; finish: if (ret_part) @@ -1225,6 +1238,8 @@ finish: *ret_psize = 0; if (ret_uuid) *ret_uuid = SD_ID128_NULL; + if (ret_devid) + *ret_devid = 0; return 0; } @@ -1236,7 +1251,8 @@ int find_esp_and_warn( uint32_t *ret_part, uint64_t *ret_pstart, uint64_t *ret_psize, - sd_id128_t *ret_uuid) { + sd_id128_t *ret_uuid, + dev_t *ret_devid) { int r; @@ -1247,7 +1263,7 @@ int find_esp_and_warn( */ if (path) { - r = verify_esp(path, false, unprivileged_mode, ret_part, ret_pstart, ret_psize, ret_uuid); + r = verify_esp(path, /* searching= */ false, unprivileged_mode, ret_part, ret_pstart, ret_psize, ret_uuid, ret_devid); if (r < 0) return r; @@ -1256,13 +1272,21 @@ int find_esp_and_warn( path = getenv("SYSTEMD_ESP_PATH"); if (path) { + struct stat st; + if (!path_is_valid(path) || !path_is_absolute(path)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "$SYSTEMD_ESP_PATH does not refer to absolute path, refusing to use it: %s", path); - /* Note: when the user explicitly configured things with an env var we won't validate the mount - * point. After all we want this to be useful for testing. */ + /* Note: when the user explicitly configured things with an env var we won't validate the + * path beyond checking it refers to a directory. After all we want this to be useful for + * testing. */ + + if (stat(path, &st) < 0) + return log_error_errno(errno, "Failed to stat '%s': %m", path); + if (!S_ISDIR(st.st_mode)) + return log_error_errno(SYNTHETIC_ERRNO(ENOTDIR), "ESP path '%s' is not a directory.", path); if (ret_part) *ret_part = 0; @@ -1272,13 +1296,15 @@ int find_esp_and_warn( *ret_psize = 0; if (ret_uuid) *ret_uuid = SD_ID128_NULL; + if (ret_devid) + *ret_devid = st.st_dev; goto found; } FOREACH_STRING(path, "/efi", "/boot", "/boot/efi") { - r = verify_esp(path, true, unprivileged_mode, ret_part, ret_pstart, ret_psize, ret_uuid); + r = verify_esp(path, /* searching= */ true, unprivileged_mode, ret_part, ret_pstart, ret_psize, ret_uuid, ret_devid); if (r >= 0) goto found; if (!IN_SET(r, -ENOENT, -EADDRNOTAVAIL)) /* This one is not it */ @@ -1445,7 +1471,8 @@ static int verify_xbootldr( const char *p, bool searching, bool unprivileged_mode, - sd_id128_t *ret_uuid) { + sd_id128_t *ret_uuid, + dev_t *ret_devid) { bool relax_checks; dev_t devid; @@ -1463,13 +1490,22 @@ static int verify_xbootldr( goto finish; if (unprivileged_mode) - return verify_xbootldr_udev(devid, searching, ret_uuid); + r = verify_xbootldr_udev(devid, searching, ret_uuid); else - return verify_xbootldr_blkid(devid, searching, ret_uuid); + r = verify_xbootldr_blkid(devid, searching, ret_uuid); + if (r < 0) + return r; + + if (ret_devid) + *ret_devid = devid; + + return 0; finish: if (ret_uuid) *ret_uuid = SD_ID128_NULL; + if (ret_devid) + *ret_devid = 0; return 0; } @@ -1478,14 +1514,15 @@ int find_xbootldr_and_warn( const char *path, bool unprivileged_mode, char **ret_path, - sd_id128_t *ret_uuid) { + sd_id128_t *ret_uuid, + dev_t *ret_devid) { int r; /* Similar to find_esp_and_warn(), but finds the XBOOTLDR partition. Returns the same errors. */ if (path) { - r = verify_xbootldr(path, false, unprivileged_mode, ret_uuid); + r = verify_xbootldr(path, /* searching= */ false, unprivileged_mode, ret_uuid, ret_devid); if (r < 0) return r; @@ -1494,18 +1531,27 @@ int find_xbootldr_and_warn( path = getenv("SYSTEMD_XBOOTLDR_PATH"); if (path) { + struct stat st; + if (!path_is_valid(path) || !path_is_absolute(path)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "$SYSTEMD_XBOOTLDR_PATH does not refer to absolute path, refusing to use it: %s", path); + if (stat(path, &st) < 0) + return log_error_errno(errno, "Failed to stat '%s': %m", path); + if (!S_ISDIR(st.st_mode)) + return log_error_errno(SYNTHETIC_ERRNO(ENOTDIR), "XBOOTLDR path '%s' is not a directory.", path); + if (ret_uuid) *ret_uuid = SD_ID128_NULL; + if (ret_devid) + *ret_devid = st.st_dev; goto found; } - r = verify_xbootldr("/boot", true, unprivileged_mode, ret_uuid); + r = verify_xbootldr("/boot", true, unprivileged_mode, ret_uuid, ret_devid); if (r >= 0) { path = "/boot"; goto found; diff --git a/src/shared/bootspec.h b/src/shared/bootspec.h index 6f1014db4a..16353746ae 100644 --- a/src/shared/bootspec.h +++ b/src/shared/bootspec.h @@ -91,5 +91,5 @@ static inline const char* boot_entry_title(const BootEntry *entry) { return entry->show_title ?: entry->title ?: entry->id; } -int find_esp_and_warn(const char *path, bool unprivileged_mode, char **ret_path, uint32_t *ret_part, uint64_t *ret_pstart, uint64_t *ret_psize, sd_id128_t *ret_uuid); -int find_xbootldr_and_warn(const char *path, bool unprivileged_mode, char **ret_path,sd_id128_t *ret_uuid); +int find_esp_and_warn(const char *path, bool unprivileged_mode, char **ret_path, uint32_t *ret_part, uint64_t *ret_pstart, uint64_t *ret_psize, sd_id128_t *ret_uuid, dev_t *ret_devid); +int find_xbootldr_and_warn(const char *path, bool unprivileged_mode, char **ret_path,sd_id128_t *ret_uuid, dev_t *ret_devid); From d5ac1d4e10e6bec3ab63cd95fb3b729e3e5d1d96 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 11 Feb 2022 22:36:00 +0100 Subject: [PATCH 10/10] bootspec: avoid zero size VLA apparently some checkers don't like that. Let's be entirely safe here, and use malloc() based allocation, given that the entries are user controlled. --- src/shared/bootspec.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/shared/bootspec.c b/src/shared/bootspec.c index 1dbeda23d9..5cf3495021 100644 --- a/src/shared/bootspec.c +++ b/src/shared/bootspec.c @@ -562,17 +562,16 @@ static int boot_entries_find_unified( } static bool find_nonunique(const BootEntry *entries, size_t n_entries, bool arr[]) { - size_t i, j; bool non_unique = false; assert(entries || n_entries == 0); assert(arr || n_entries == 0); - for (i = 0; i < n_entries; i++) + for (size_t i = 0; i < n_entries; i++) arr[i] = false; - for (i = 0; i < n_entries; i++) - for (j = 0; j < n_entries; j++) + for (size_t i = 0; i < n_entries; i++) + for (size_t j = 0; j < n_entries; j++) if (i != j && streq(boot_entry_title(entries + i), boot_entry_title(entries + j))) non_unique = arr[i] = arr[j] = true; @@ -581,22 +580,26 @@ static bool find_nonunique(const BootEntry *entries, size_t n_entries, bool arr[ } static int boot_entries_uniquify(BootEntry *entries, size_t n_entries) { + _cleanup_free_ bool *arr = NULL; char *s; - size_t i; - int r; - bool arr[n_entries]; assert(entries || n_entries == 0); + if (n_entries == 0) + return 0; + + arr = new(bool, n_entries); + if (!arr) + return -ENOMEM; + /* Find _all_ non-unique titles */ if (!find_nonunique(entries, n_entries, arr)) return 0; /* Add version to non-unique titles */ - for (i = 0; i < n_entries; i++) + for (size_t i = 0; i < n_entries; i++) if (arr[i] && entries[i].version) { - r = asprintf(&s, "%s (%s)", boot_entry_title(entries + i), entries[i].version); - if (r < 0) + if (asprintf(&s, "%s (%s)", boot_entry_title(entries + i), entries[i].version) < 0) return -ENOMEM; free_and_replace(entries[i].show_title, s); @@ -606,10 +609,9 @@ static int boot_entries_uniquify(BootEntry *entries, size_t n_entries) { return 0; /* Add machine-id to non-unique titles */ - for (i = 0; i < n_entries; i++) + for (size_t i = 0; i < n_entries; i++) if (arr[i] && entries[i].machine_id) { - r = asprintf(&s, "%s (%s)", boot_entry_title(entries + i), entries[i].machine_id); - if (r < 0) + if (asprintf(&s, "%s (%s)", boot_entry_title(entries + i), entries[i].machine_id) < 0) return -ENOMEM; free_and_replace(entries[i].show_title, s); @@ -619,10 +621,9 @@ static int boot_entries_uniquify(BootEntry *entries, size_t n_entries) { return 0; /* Add file name to non-unique titles */ - for (i = 0; i < n_entries; i++) + for (size_t i = 0; i < n_entries; i++) if (arr[i]) { - r = asprintf(&s, "%s (%s)", boot_entry_title(entries + i), entries[i].id); - if (r < 0) + if (asprintf(&s, "%s (%s)", boot_entry_title(entries + i), entries[i].id) < 0) return -ENOMEM; free_and_replace(entries[i].show_title, s);