From 92bb46c464ffc6f04b633f7d82686a766663ad69 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 5 Jul 2023 15:38:38 +0200 Subject: [PATCH 1/3] efi: add a bunch of reported EFI loader/stub feature flags We gained a bunch of new features that deserve reporting to userspace, hence add matching flags for each. This allows userspace to determine if installing addons in the ESP even makes sense. This is inspired by a similar changes in #28057 --- src/boot/bootctl-status.c | 4 ++++ src/boot/efi/boot.c | 2 ++ src/boot/efi/stub.c | 2 ++ src/fundamental/efivars-fundamental.h | 4 ++++ 4 files changed, 12 insertions(+) diff --git a/src/boot/bootctl-status.c b/src/boot/bootctl-status.c index dc841153cc..0bfbf86e10 100644 --- a/src/boot/bootctl-status.c +++ b/src/boot/bootctl-status.c @@ -367,6 +367,8 @@ int verb_status(int argc, char *argv[], void *userdata) { { EFI_LOADER_FEATURE_SORT_KEY, "Support Type #1 sort-key field" }, { EFI_LOADER_FEATURE_SAVED_ENTRY, "Support @saved pseudo-entry" }, { EFI_LOADER_FEATURE_DEVICETREE, "Support Type #1 devicetree field" }, + { EFI_LOADER_FEATURE_SECUREBOOT_ENROLL, "Enroll SecureBoot keys" }, + { EFI_LOADER_FEATURE_RETAIN_SHIM, "Retain SHIM protocols" }, }; static const struct { uint64_t flag; @@ -377,6 +379,8 @@ int verb_status(int argc, char *argv[], void *userdata) { { EFI_STUB_FEATURE_PICK_UP_SYSEXTS, "Picks up system extension images from boot partition" }, { EFI_STUB_FEATURE_THREE_PCRS, "Measures kernel+command line+sysexts" }, { EFI_STUB_FEATURE_RANDOM_SEED, "Support for passing random seed to OS" }, + { EFI_STUB_FEATURE_CMDLINE_ADDONS, "Pick up .cmdline from addons" }, + { EFI_STUB_FEATURE_CMDLINE_SMBIOS, "Pick up .cmdline from SMBIOS Type 11" }, }; _cleanup_free_ char *fw_type = NULL, *fw_info = NULL, *loader = NULL, *loader_path = NULL, *stub = NULL; sd_id128_t loader_part_uuid = SD_ID128_NULL; diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index cda6f56426..bde5d49c45 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -2537,6 +2537,8 @@ static void export_variables( EFI_LOADER_FEATURE_SORT_KEY | EFI_LOADER_FEATURE_SAVED_ENTRY | EFI_LOADER_FEATURE_DEVICETREE | + EFI_LOADER_FEATURE_SECUREBOOT_ENROLL | + EFI_LOADER_FEATURE_RETAIN_SHIM | 0; _cleanup_free_ char16_t *infostr = NULL, *typestr = NULL; diff --git a/src/boot/efi/stub.c b/src/boot/efi/stub.c index 93a3641424..fd4f668eb6 100644 --- a/src/boot/efi/stub.c +++ b/src/boot/efi/stub.c @@ -88,6 +88,8 @@ static void export_variables(EFI_LOADED_IMAGE_PROTOCOL *loaded_image) { EFI_STUB_FEATURE_PICK_UP_SYSEXTS | /* We pick up system extensions from the boot partition */ EFI_STUB_FEATURE_THREE_PCRS | /* We can measure kernel image, parameters and sysext */ EFI_STUB_FEATURE_RANDOM_SEED | /* We pass a random seed to the kernel */ + EFI_STUB_FEATURE_CMDLINE_ADDONS | /* We pick up .cmdline addons */ + EFI_STUB_FEATURE_CMDLINE_SMBIOS | /* We support extending kernel cmdline from SMBIOS Type #11 */ 0; assert(loaded_image); diff --git a/src/fundamental/efivars-fundamental.h b/src/fundamental/efivars-fundamental.h index 3bad79b036..569f5eeceb 100644 --- a/src/fundamental/efivars-fundamental.h +++ b/src/fundamental/efivars-fundamental.h @@ -20,6 +20,8 @@ #define EFI_LOADER_FEATURE_SORT_KEY (UINT64_C(1) << 8) #define EFI_LOADER_FEATURE_SAVED_ENTRY (UINT64_C(1) << 9) #define EFI_LOADER_FEATURE_DEVICETREE (UINT64_C(1) << 10) +#define EFI_LOADER_FEATURE_SECUREBOOT_ENROLL (UINT64_C(1) << 11) +#define EFI_LOADER_FEATURE_RETAIN_SHIM (UINT64_C(1) << 12) /* Features of the stub, i.e. systemd-stub */ #define EFI_STUB_FEATURE_REPORT_BOOT_PARTITION (UINT64_C(1) << 0) @@ -27,6 +29,8 @@ #define EFI_STUB_FEATURE_PICK_UP_SYSEXTS (UINT64_C(1) << 2) #define EFI_STUB_FEATURE_THREE_PCRS (UINT64_C(1) << 3) #define EFI_STUB_FEATURE_RANDOM_SEED (UINT64_C(1) << 4) +#define EFI_STUB_FEATURE_CMDLINE_ADDONS (UINT64_C(1) << 5) +#define EFI_STUB_FEATURE_CMDLINE_SMBIOS (UINT64_C(1) << 6) typedef enum SecureBootMode { SECURE_BOOT_UNSUPPORTED, From 3a59c55f670d90377e300d0bb03e96ba8ddb6c87 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 5 Jul 2023 15:41:26 +0200 Subject: [PATCH 2/3] =?UTF-8?q?boot:=20rename=20entry=5Fcount=20=E2=86=92?= =?UTF-8?q?=20n=5Fentries?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While we don't strictly follow the rule, most of our userspace names these fields that count entries in some array n_xyz, hence let's do so in the EFI boot code too, to make things less special. --- src/boot/efi/boot.c | 92 ++++++++++++++++++++++----------------------- 1 file changed, 46 insertions(+), 46 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index bde5d49c45..f90f0891a8 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -67,7 +67,7 @@ typedef struct { typedef struct { ConfigEntry **entries; - size_t entry_count; + size_t n_entries; size_t idx_default; size_t idx_default_efivar; uint32_t timeout_sec; /* Actual timeout used (efi_main() override > efivar > config). */ @@ -363,13 +363,13 @@ static size_t entry_lookup_key(Config *config, size_t start, char16_t key) { /* select entry by number key */ if (key >= '1' && key <= '9') { size_t i = key - '0'; - if (i > config->entry_count) - i = config->entry_count; + if (i > config->n_entries) + i = config->n_entries; return i-1; } /* find matching key in config entries */ - for (size_t i = start; i < config->entry_count; i++) + for (size_t i = start; i < config->n_entries; i++) if (config->entries[i]->key == key) return i; @@ -548,7 +548,7 @@ static void print_status(Config *config, char16_t *loaded_image_path) { if (!ps_continue()) return; - for (size_t i = 0; i < config->entry_count; i++) { + for (size_t i = 0; i < config->n_entries; i++) { ConfigEntry *entry = config->entries[i]; EFI_DEVICE_PATH *dp = NULL; _cleanup_free_ char16_t *dp_str = NULL; @@ -558,7 +558,7 @@ static void print_status(Config *config, char16_t *loaded_image_path) { EFI_SUCCESS) (void) device_path_to_str(dp, &dp_str); - printf(" config entry: %zu/%zu\n", i + 1, config->entry_count); + printf(" config entry: %zu/%zu\n", i + 1, config->n_entries); printf(" id: %ls\n", entry->id); if (entry->title) printf(" title: %ls\n", entry->title); @@ -663,38 +663,38 @@ static bool menu_run( * sure that idx_highlight is centered, but not if we are close to the * beginning/end of the entry list. Otherwise we would have a half-empty * screen. */ - if (config->entry_count <= visible_max || idx_highlight <= visible_max / 2) + if (config->n_entries <= visible_max || idx_highlight <= visible_max / 2) idx_first = 0; - else if (idx_highlight >= config->entry_count - (visible_max / 2)) - idx_first = config->entry_count - visible_max; + else if (idx_highlight >= config->n_entries - (visible_max / 2)) + idx_first = config->n_entries - visible_max; else idx_first = idx_highlight - (visible_max / 2); idx_last = idx_first + visible_max - 1; /* length of the longest entry */ line_width = 0; - for (size_t i = 0; i < config->entry_count; i++) + for (size_t i = 0; i < config->n_entries; i++) line_width = MAX(line_width, strlen16(config->entries[i]->title_show)); line_width = MIN(line_width + 2 * entry_padding, x_max); /* offsets to center the entries on the screen */ x_start = (x_max - (line_width)) / 2; - if (config->entry_count < visible_max) - y_start = ((visible_max - config->entry_count) / 2) + 1; + if (config->n_entries < visible_max) + y_start = ((visible_max - config->n_entries) / 2) + 1; else y_start = 0; /* Put status line after the entry list, but give it some breathing room. */ - y_status = MIN(y_start + MIN(visible_max, config->entry_count) + 1, y_max - 1); + y_status = MIN(y_start + MIN(visible_max, config->n_entries) + 1, y_max - 1); lines = strv_free(lines); clearline = mfree(clearline); separator = mfree(separator); /* menu entries title lines */ - lines = xnew(char16_t *, config->entry_count + 1); + lines = xnew(char16_t *, config->n_entries + 1); - for (size_t i = 0; i < config->entry_count; i++) { + for (size_t i = 0; i < config->n_entries; i++) { size_t j, padding; lines[i] = xnew(char16_t, line_width + 1); @@ -710,7 +710,7 @@ static bool menu_run( lines[i][j] = ' '; lines[i][line_width] = '\0'; } - lines[config->entry_count] = NULL; + lines[config->n_entries] = NULL; clearline = xnew(char16_t, x_max + 1); separator = xnew(char16_t, x_max + 1); @@ -732,7 +732,7 @@ static bool menu_run( } if (refresh) { - for (size_t i = idx_first; i <= idx_last && i < config->entry_count; i++) { + for (size_t i = idx_first; i <= idx_last && i < config->n_entries; i++) { print_at(x_start, y_start + i - idx_first, i == idx_highlight ? COLOR_HIGHLIGHT : COLOR_ENTRY, lines[i]); @@ -834,7 +834,7 @@ static bool menu_run( case KEYPRESS(0, SCAN_DOWN, 0): case KEYPRESS(0, 0, 'j'): case KEYPRESS(0, 0, 'J'): - if (idx_highlight < config->entry_count-1) + if (idx_highlight < config->n_entries-1) idx_highlight++; break; @@ -848,9 +848,9 @@ static bool menu_run( case KEYPRESS(0, SCAN_END, 0): case KEYPRESS(EFI_ALT_PRESSED, 0, '>'): - if (idx_highlight < config->entry_count-1) { + if (idx_highlight < config->n_entries-1) { refresh = true; - idx_highlight = config->entry_count-1; + idx_highlight = config->n_entries-1; } break; @@ -863,8 +863,8 @@ static bool menu_run( case KEYPRESS(0, SCAN_PAGE_DOWN, 0): idx_highlight += visible_max; - if (idx_highlight > config->entry_count-1) - idx_highlight = config->entry_count-1; + if (idx_highlight > config->n_entries-1) + idx_highlight = config->n_entries-1; break; case KEYPRESS(0, 0, '\n'): @@ -1067,15 +1067,15 @@ static void config_add_entry(Config *config, ConfigEntry *entry) { assert(entry); /* This is just for paranoia. */ - assert(config->entry_count < IDX_MAX); + assert(config->n_entries < IDX_MAX); - if ((config->entry_count & 15) == 0) { + if ((config->n_entries & 15) == 0) { config->entries = xrealloc( config->entries, - sizeof(void *) * config->entry_count, - sizeof(void *) * (config->entry_count + 16)); + sizeof(void *) * config->n_entries, + sizeof(void *) * (config->n_entries + 16)); } - config->entries[config->entry_count++] = entry; + config->entries[config->n_entries++] = entry; } static void config_entry_free(ConfigEntry *entry) { @@ -1722,7 +1722,7 @@ static size_t config_entry_find(Config *config, const char16_t *pattern) { if (!pattern) return IDX_INVALID; - for (size_t i = 0; i < config->entry_count; i++) + for (size_t i = 0; i < config->n_entries; i++) if (efi_fnmatch(pattern, config->entries[i]->id)) return i; @@ -1758,7 +1758,7 @@ static void config_default_entry_select(Config *config) { } /* select the first suitable entry */ - for (i = 0; i < config->entry_count; i++) { + for (i = 0; i < config->n_entries; i++) { if (config->entries[i]->type == LOADER_AUTO || config->entries[i]->call) continue; config->idx_default = i; @@ -1771,14 +1771,14 @@ static void config_default_entry_select(Config *config) { config->timeout_sec = 10; } -static bool entries_unique(ConfigEntry **entries, bool *unique, size_t entry_count) { +static bool entries_unique(ConfigEntry **entries, bool *unique, size_t n_entries) { bool is_unique = true; assert(entries); assert(unique); - for (size_t i = 0; i < entry_count; i++) - for (size_t k = i + 1; k < entry_count; k++) { + for (size_t i = 0; i < n_entries; i++) + for (size_t k = i + 1; k < n_entries; k++) { if (!streq16(entries[i]->title_show, entries[k]->title_show)) continue; @@ -1792,20 +1792,20 @@ static bool entries_unique(ConfigEntry **entries, bool *unique, size_t entry_cou static void config_title_generate(Config *config) { assert(config); - bool unique[config->entry_count]; + bool unique[config->n_entries]; /* set title */ - for (size_t i = 0; i < config->entry_count; i++) { + for (size_t i = 0; i < config->n_entries; i++) { assert(!config->entries[i]->title_show); unique[i] = true; config->entries[i]->title_show = xstrdup16(config->entries[i]->title ?: config->entries[i]->id); } - if (entries_unique(config->entries, unique, config->entry_count)) + if (entries_unique(config->entries, unique, config->n_entries)) return; /* add version to non-unique titles */ - for (size_t i = 0; i < config->entry_count; i++) { + for (size_t i = 0; i < config->n_entries; i++) { if (unique[i]) continue; @@ -1818,11 +1818,11 @@ static void config_title_generate(Config *config) { config->entries[i]->title_show = xasprintf("%ls (%ls)", t, config->entries[i]->version); } - if (entries_unique(config->entries, unique, config->entry_count)) + if (entries_unique(config->entries, unique, config->n_entries)) return; /* add machine-id to non-unique titles */ - for (size_t i = 0; i < config->entry_count; i++) { + for (size_t i = 0; i < config->n_entries; i++) { if (unique[i]) continue; @@ -1835,11 +1835,11 @@ static void config_title_generate(Config *config) { config->entries[i]->title_show = xasprintf("%ls (%.8ls)", t, config->entries[i]->machine_id); } - if (entries_unique(config->entries, unique, config->entry_count)) + if (entries_unique(config->entries, unique, config->n_entries)) return; /* add file name to non-unique titles */ - for (size_t i = 0; i < config->entry_count; i++) { + for (size_t i = 0; i < config->n_entries; i++) { if (unique[i]) continue; @@ -2417,7 +2417,7 @@ static EFI_STATUS image_start( static void config_free(Config *config) { assert(config); - for (size_t i = 0; i < config->entry_count; i++) + for (size_t i = 0; i < config->n_entries; i++) config_entry_free(config->entries[i]); free(config->entries); free(config->entry_default_config); @@ -2433,12 +2433,12 @@ static void config_write_entries_to_variable(Config *config) { assert(config); - for (size_t i = 0; i < config->entry_count; i++) + for (size_t i = 0; i < config->n_entries; i++) sz += strsize16(config->entries[i]->id); p = buffer = xmalloc(sz); - for (size_t i = 0; i < config->entry_count; i++) + for (size_t i = 0; i < config->n_entries; i++) p = mempcpy(p, config->entries[i]->id, strsize16(config->entries[i]->id)); assert(p == buffer + sz); @@ -2587,7 +2587,7 @@ static void config_load_all_entries( config_load_xbootldr(config, loaded_image->DeviceHandle); /* sort entries after version number */ - sort_pointer_array((void **) config->entries, config->entry_count, (compare_pointer_func_t) config_entry_compare); + sort_pointer_array((void **) config->entries, config->n_entries, (compare_pointer_func_t) config_entry_compare); /* if we find some well-known loaders, add them to the end of the list */ config_entry_add_osx(config); @@ -2616,7 +2616,7 @@ static void config_load_all_entries( if (config->secure_boot_enroll != ENROLL_OFF) secure_boot_discover_keys(config, root_dir); - if (config->entry_count == 0) + if (config->n_entries == 0) return; config_write_entries_to_variable(config); @@ -2665,7 +2665,7 @@ static EFI_STATUS run(EFI_HANDLE image) { config_load_all_entries(&config, loaded_image, loaded_image_path, root_dir); - if (config.entry_count == 0) + if (config.n_entries == 0) return log_error_status( EFI_NOT_FOUND, "No loader found. Configuration files in \\loader\\entries\\*.conf are needed."); From e987d54baf7071da8a4b51cf5daf56663ad88d87 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 5 Jul 2023 15:42:30 +0200 Subject: [PATCH 3/3] boot: make LoaderType enum less special Usually (but not always) we use uppercase type naming, and do a typedef for enums like this. Do so here too. --- src/boot/efi/boot.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index f90f0891a8..1321658a01 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -34,14 +34,15 @@ _used_ _section_(".osrel") static const char osrel[] = "VERSION=\"" GIT_VERSION "\"\n" "NAME=\"systemd-boot " GIT_VERSION "\"\n"; -enum loader_type { +typedef enum LoaderType { LOADER_UNDEFINED, LOADER_AUTO, LOADER_EFI, LOADER_LINUX, /* Boot loader spec type #1 entries */ LOADER_UNIFIED_LINUX, /* Boot loader spec type #2 entries */ LOADER_SECURE_BOOT_KEYS, -}; + _LOADER_TYPE_MAX, +} LoaderType; typedef struct { char16_t *id; /* The unique identifier for this entry (typically the filename of the file defining the entry) */ @@ -51,7 +52,7 @@ typedef struct { char16_t *version; /* The raw (human readable) version string of the entry */ char16_t *machine_id; EFI_HANDLE *device; - enum loader_type type; + LoaderType type; char16_t *loader; char16_t *devicetree; char16_t *options;