From 8ac694710fdab2ed3ecce1dc819f818d68568977 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Oct 2018 13:31:41 +0200 Subject: [PATCH 01/11] sd-boot: timeout_sec is unsigned hence show it with %u --- 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 4ff7594a9a..79285bd6b9 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -385,10 +385,10 @@ static VOID print_status(Config *config, CHAR16 *loaded_image_path) { Print(L"\n--- press key ---\n\n"); console_key_read(&key, TRUE); - Print(L"timeout: %d\n", config->timeout_sec); + Print(L"timeout: %u\n", config->timeout_sec); if (config->timeout_sec_efivar >= 0) Print(L"timeout (EFI var): %d\n", config->timeout_sec_efivar); - Print(L"timeout (config): %d\n", config->timeout_sec_config); + Print(L"timeout (config): %u\n", config->timeout_sec_config); if (config->entry_default_pattern) Print(L"default pattern: '%s'\n", config->entry_default_pattern); Print(L"editor: %s\n", yes_no(config->editor)); @@ -403,7 +403,8 @@ static VOID print_status(Config *config, CHAR16 *loaded_image_path) { Print(L"\n"); if (efivar_get_int(L"LoaderConfigTimeout", &i) == EFI_SUCCESS) - Print(L"LoaderConfigTimeout: %d\n", i); + Print(L"LoaderConfigTimeout: %u\n", i); + if (config->entry_oneshot) Print(L"LoaderEntryOneShot: %s\n", config->entry_oneshot); if (efivar_get(L"LoaderDevicePartUUID", &partstr) == EFI_SUCCESS) From b49dd00f8c13fc32250981a0508adc4dddf06a08 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Oct 2018 13:32:59 +0200 Subject: [PATCH 02/11] efi: add simple macros for MAX values of EFI's UINTN/INTN types --- src/boot/efi/util.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h index daa386b769..169bd674d3 100644 --- a/src/boot/efi/util.h +++ b/src/boot/efi/util.h @@ -52,3 +52,6 @@ static inline void FileHandleClosep(EFI_FILE_HANDLE *handle) { } const EFI_GUID loader_guid; + +#define UINTN_MAX (~(UINTN)0) +#define INTN_MAX ((INTN)(UINTN_MAX>>1)) From 2366d923450e7722fdc743075d5f88550455e37a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Oct 2018 13:35:06 +0200 Subject: [PATCH 03/11] sd-boot: when converting menu timeout for UINTN to INTN saturate Let's be a bit more careful and handle overly long timeouts in a slightly more sensible way. --- src/boot/efi/boot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index 79285bd6b9..3190849b7e 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -1413,7 +1413,7 @@ static VOID config_load_defaults(Config *config, EFI_FILE *root_dir) { err = efivar_get_int(L"LoaderConfigTimeout", &sec); if (!EFI_ERROR(err)) { - config->timeout_sec_efivar = sec; + config->timeout_sec_efivar = sec > INTN_MAX ? INTN_MAX : sec; config->timeout_sec = sec; } else config->timeout_sec_efivar = -1; From fe2579dd9c526bf14e0f1275fea61594ba54dcf7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Oct 2018 13:40:51 +0200 Subject: [PATCH 04/11] sd-boot: introduce a one-time override for the boot menu timeout This is useful to allow userspace to request a "boot into boot menu" feature. --- man/systemd-boot.xml | 8 ++++++-- src/boot/efi/boot.c | 17 ++++++++++++++--- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/man/systemd-boot.xml b/man/systemd-boot.xml index 3b726e63a4..e1d1910c91 100644 --- a/man/systemd-boot.xml +++ b/man/systemd-boot.xml @@ -256,8 +256,12 @@ LoaderConfigTimeout - The menu time-out. Read by the boot loader. (Also, modified by it when the - t/T keys are used, see above.) + LoaderConfigTimeoutOneShot + The menu time-out in seconds. Read by the boot loader. LoaderConfigTimeout + is maintained persistently, while LoaderConfigTimeoutOneShot is a one-time override which is + read once (in which case it takes precedence over LoaderConfigTimeout) and then + removed. LoaderConfigTimeout may be manipulated with the + t/T keys, see above.) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index 3190849b7e..b2fa8f1e2a 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -62,6 +62,7 @@ typedef struct { BOOLEAN editor; BOOLEAN auto_entries; BOOLEAN auto_firmware; + BOOLEAN force_menu; UINTN console_mode; enum console_mode_change_type console_mode_change; } Config; @@ -1417,6 +1418,15 @@ static VOID config_load_defaults(Config *config, EFI_FILE *root_dir) { config->timeout_sec = sec; } else config->timeout_sec_efivar = -1; + + err = efivar_get_int(L"LoaderConfigTimeoutOneShot", &sec); + if (!EFI_ERROR(err)) { + /* Unset variable now, after all it's "one shot". */ + (void) efivar_set(L"LoaderConfigTimeoutOneShot", NULL, TRUE); + + config->timeout_sec = sec; + config->force_menu = TRUE; /* force the menu when this is set */ + } } static VOID config_load_entries( @@ -2167,7 +2177,9 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { } /* select entry or show menu when key is pressed or timeout is set */ - if (config.timeout_sec == 0) { + if (config.force_menu || config.timeout_sec > 0) + menu = TRUE; + else { UINT64 key; err = console_key_read(&key, FALSE); @@ -2181,8 +2193,7 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { else menu = TRUE; } - } else - menu = TRUE; + } for (;;) { ConfigEntry *entry; From 427ee7ec82c81188fe6226521a0486089005b332 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Oct 2018 16:43:53 +0200 Subject: [PATCH 05/11] efi: add 'const' to various util.h API parameters --- src/boot/efi/util.c | 22 +++++++++++----------- src/boot/efi/util.h | 14 +++++++------- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/boot/efi/util.c b/src/boot/efi/util.c index fd4be681a5..a7bba3120b 100644 --- a/src/boot/efi/util.c +++ b/src/boot/efi/util.c @@ -59,7 +59,7 @@ UINT64 time_usec(VOID) { return 1000 * 1000 * ticks / freq; } -EFI_STATUS parse_boolean(CHAR8 *v, BOOLEAN *b) { +EFI_STATUS parse_boolean(const CHAR8 *v, BOOLEAN *b) { if (strcmpa(v, (CHAR8 *)"1") == 0 || strcmpa(v, (CHAR8 *)"yes") == 0 || strcmpa(v, (CHAR8 *)"y") == 0 || @@ -79,28 +79,28 @@ EFI_STATUS parse_boolean(CHAR8 *v, BOOLEAN *b) { return EFI_INVALID_PARAMETER; } -EFI_STATUS efivar_set_raw(const EFI_GUID *vendor, CHAR16 *name, VOID *buf, UINTN size, BOOLEAN persistent) { +EFI_STATUS efivar_set_raw(const EFI_GUID *vendor, const CHAR16 *name, const VOID *buf, UINTN size, BOOLEAN persistent) { UINT32 flags; flags = EFI_VARIABLE_BOOTSERVICE_ACCESS|EFI_VARIABLE_RUNTIME_ACCESS; if (persistent) flags |= EFI_VARIABLE_NON_VOLATILE; - return uefi_call_wrapper(RT->SetVariable, 5, name, (EFI_GUID *)vendor, flags, size, buf); + return uefi_call_wrapper(RT->SetVariable, 5, (CHAR16*) name, (EFI_GUID *)vendor, flags, size, (VOID*) buf); } -EFI_STATUS efivar_set(CHAR16 *name, CHAR16 *value, BOOLEAN persistent) { +EFI_STATUS efivar_set(const CHAR16 *name, const CHAR16 *value, BOOLEAN persistent) { return efivar_set_raw(&loader_guid, name, value, value ? (StrLen(value)+1) * sizeof(CHAR16) : 0, persistent); } EFI_STATUS efivar_set_int(CHAR16 *name, UINTN i, BOOLEAN persistent) { CHAR16 str[32]; - SPrint(str, 32, L"%d", i); + SPrint(str, 32, L"%u", i); return efivar_set(name, str, persistent); } -EFI_STATUS efivar_get(CHAR16 *name, CHAR16 **value) { +EFI_STATUS efivar_get(const CHAR16 *name, CHAR16 **value) { _cleanup_freepool_ CHAR8 *buf = NULL; CHAR16 *val; UINTN size; @@ -118,7 +118,7 @@ EFI_STATUS efivar_get(CHAR16 *name, CHAR16 **value) { return EFI_SUCCESS; } -EFI_STATUS efivar_get_int(CHAR16 *name, UINTN *i) { +EFI_STATUS efivar_get_int(const CHAR16 *name, UINTN *i) { _cleanup_freepool_ CHAR16 *val = NULL; EFI_STATUS err; @@ -129,7 +129,7 @@ EFI_STATUS efivar_get_int(CHAR16 *name, UINTN *i) { return err; } -EFI_STATUS efivar_get_raw(const EFI_GUID *vendor, CHAR16 *name, CHAR8 **buffer, UINTN *size) { +EFI_STATUS efivar_get_raw(const EFI_GUID *vendor, const CHAR16 *name, CHAR8 **buffer, UINTN *size) { _cleanup_freepool_ CHAR8 *buf = NULL; UINTN l; EFI_STATUS err; @@ -139,7 +139,7 @@ EFI_STATUS efivar_get_raw(const EFI_GUID *vendor, CHAR16 *name, CHAR8 **buffer, if (!buf) return EFI_OUT_OF_RESOURCES; - err = uefi_call_wrapper(RT->GetVariable, 5, name, (EFI_GUID *)vendor, NULL, &l, buf); + err = uefi_call_wrapper(RT->GetVariable, 5, (CHAR16*) name, (EFI_GUID *)vendor, NULL, &l, buf); if (!EFI_ERROR(err)) { *buffer = buf; buf = NULL; @@ -287,12 +287,12 @@ CHAR8 *strchra(CHAR8 *s, CHAR8 c) { return NULL; } -EFI_STATUS file_read(EFI_FILE_HANDLE dir, CHAR16 *name, UINTN off, UINTN size, CHAR8 **content, UINTN *content_size) { +EFI_STATUS file_read(EFI_FILE_HANDLE dir, const CHAR16 *name, UINTN off, UINTN size, CHAR8 **content, UINTN *content_size) { EFI_FILE_HANDLE handle; _cleanup_freepool_ CHAR8 *buf = NULL; EFI_STATUS err; - err = uefi_call_wrapper(dir->Open, 5, dir, &handle, name, EFI_FILE_MODE_READ, 0ULL); + err = uefi_call_wrapper(dir->Open, 5, dir, &handle, (CHAR16*) name, EFI_FILE_MODE_READ, 0ULL); if (EFI_ERROR(err)) return err; diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h index 169bd674d3..f7e0e49c06 100644 --- a/src/boot/efi/util.h +++ b/src/boot/efi/util.h @@ -11,26 +11,26 @@ static inline const CHAR16 *yes_no(BOOLEAN b) { return b ? L"yes" : L"no"; } -EFI_STATUS parse_boolean(CHAR8 *v, BOOLEAN *b); +EFI_STATUS parse_boolean(const CHAR8 *v, BOOLEAN *b); UINT64 ticks_read(void); UINT64 ticks_freq(void); UINT64 time_usec(void); -EFI_STATUS efivar_set(CHAR16 *name, CHAR16 *value, BOOLEAN persistent); -EFI_STATUS efivar_set_raw(const EFI_GUID *vendor, CHAR16 *name, VOID *buf, UINTN size, BOOLEAN persistent); +EFI_STATUS efivar_set(const CHAR16 *name, const CHAR16 *value, BOOLEAN persistent); +EFI_STATUS efivar_set_raw(const EFI_GUID *vendor, const CHAR16 *name, const VOID *buf, UINTN size, BOOLEAN persistent); EFI_STATUS efivar_set_int(CHAR16 *name, UINTN i, BOOLEAN persistent); VOID efivar_set_time_usec(CHAR16 *name, UINT64 usec); -EFI_STATUS efivar_get(CHAR16 *name, CHAR16 **value); -EFI_STATUS efivar_get_raw(const EFI_GUID *vendor, CHAR16 *name, CHAR8 **buffer, UINTN *size); -EFI_STATUS efivar_get_int(CHAR16 *name, UINTN *i); +EFI_STATUS efivar_get(const CHAR16 *name, CHAR16 **value); +EFI_STATUS efivar_get_raw(const EFI_GUID *vendor, const CHAR16 *name, CHAR8 **buffer, UINTN *size); +EFI_STATUS efivar_get_int(const CHAR16 *name, UINTN *i); CHAR8 *strchra(CHAR8 *s, CHAR8 c); CHAR16 *stra_to_path(CHAR8 *stra); CHAR16 *stra_to_str(CHAR8 *stra); -EFI_STATUS file_read(EFI_FILE_HANDLE dir, CHAR16 *name, UINTN off, UINTN size, CHAR8 **content, UINTN *content_size); +EFI_STATUS file_read(EFI_FILE_HANDLE dir, const CHAR16 *name, UINTN off, UINTN size, CHAR8 **content, UINTN *content_size); static inline void FreePoolp(void *p) { void *q = *(void**) p; From 95a18e91db59028cf5e1848834fa754a8dab5b23 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Oct 2018 16:45:32 +0200 Subject: [PATCH 06/11] efi: make sure parse_boolean() does something useful on a NULL parameter --- src/boot/efi/util.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/boot/efi/util.c b/src/boot/efi/util.c index a7bba3120b..77d7b81fea 100644 --- a/src/boot/efi/util.c +++ b/src/boot/efi/util.c @@ -60,6 +60,9 @@ UINT64 time_usec(VOID) { } EFI_STATUS parse_boolean(const CHAR8 *v, BOOLEAN *b) { + if (!v) + return EFI_INVALID_PARAMETER; + if (strcmpa(v, (CHAR8 *)"1") == 0 || strcmpa(v, (CHAR8 *)"yes") == 0 || strcmpa(v, (CHAR8 *)"y") == 0 || From 5dd5f7cfa878a329fce5672a244ab190bfc8fcd9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Oct 2018 16:51:46 +0200 Subject: [PATCH 07/11] sd-boot: add new EFI variable exposing feature set of boot loader We keep adding new features, let's advertise to the host OS what these are in a new variable LoaderFeatures. It works a bit like OsIndicationsSupported, but is about Loader features. --- man/systemd-boot.xml | 8 ++++++++ src/boot/efi/boot.c | 10 ++++++++++ 2 files changed, 18 insertions(+) diff --git a/man/systemd-boot.xml b/man/systemd-boot.xml index e1d1910c91..e537426970 100644 --- a/man/systemd-boot.xml +++ b/man/systemd-boot.xml @@ -301,6 +301,14 @@ loader. + + LoaderFeatures + + A set of flags indicating the features the boot loader supports. Set by the boot loader. Use + bootctl1 to view this + data. + + LoaderFirmwareInfo LoaderFirmwareType diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index b2fa8f1e2a..909cf914a6 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -2072,6 +2072,14 @@ static VOID config_write_entries_to_variable(Config *config) { } EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { + static const UINT64 loader_features = + (1ULL << 0) | /* I honour the LoaderConfigTimeout variable */ + (1ULL << 1) | /* I honour the LoaderConfigTimeoutOneShot variable */ + (1ULL << 2) | /* I honour the LoaderEntryDefault variable */ + (1ULL << 3) | /* I honour the LoaderEntryOneShot variable */ + (1ULL << 4) | /* I support boot counting */ + 0; + _cleanup_freepool_ CHAR16 *infostr = NULL, *typestr = NULL; CHAR8 *b; UINTN size; @@ -2095,6 +2103,8 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { typestr = PoolPrint(L"UEFI %d.%02d", ST->Hdr.Revision >> 16, ST->Hdr.Revision & 0xffff); efivar_set(L"LoaderFirmwareType", typestr, FALSE); + (void) efivar_set_raw(&loader_guid, L"LoaderFeatures", &loader_features, sizeof(loader_features), FALSE); + err = uefi_call_wrapper(BS->OpenProtocol, 6, image, &LoadedImageProtocol, (VOID **)&loaded_image, image, NULL, EFI_OPEN_PROTOCOL_GET_PROTOCOL); if (EFI_ERROR(err)) { From b58c7351c098916ad45af38b1d34565b98055da0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 22 Oct 2018 21:51:47 +0200 Subject: [PATCH 08/11] sd-boot: change name of automatic entry for rebooting into firmware Let's stick to one nomenclature. In userspace we usually call this "reboot to firmware setup", hence use the same name in sd-boot too. This name was previously only relevant internally, but since the addition of the LoaderEntries EFI var is exposed to userspace, hence let's get this right with the first release adding this. --- src/boot/efi/boot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index 909cf914a6..b64c0376e0 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -2162,7 +2162,7 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { UINT64 osind = (UINT64)*b; if (osind & EFI_OS_INDICATIONS_BOOT_TO_FW_UI) - config_entry_add_call(&config, L"auto-reboot-into-firmware-ui", L"Reboot Into Firmware Interface", reboot_into_firmware); + config_entry_add_call(&config, L"auto-reboot-to-firmware-setup", L"Reboot Into Firmware Interface", reboot_into_firmware); FreePool(b); } From aec1443aeccc6ad74cd6a17e1dffb4dab37007c3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 23 Oct 2018 13:42:58 +0200 Subject: [PATCH 09/11] sd-boot: use structured initialization --- src/boot/efi/boot.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index b64c0376e0..e62c766335 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -1404,9 +1404,11 @@ static VOID config_load_defaults(Config *config, EFI_FILE *root_dir) { UINTN sec; EFI_STATUS err; - config->editor = TRUE; - config->auto_entries = TRUE; - config->auto_firmware = TRUE; + *config = (Config) { + .editor = TRUE, + .auto_entries = TRUE, + .auto_firmware = TRUE, + }; err = file_read(root_dir, L"\\loader\\loader.conf", 0, 0, &content, NULL); if (!EFI_ERROR(err)) @@ -2137,7 +2139,6 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { loaded_image_path = DevicePathToStr(loaded_image->FilePath); efivar_set(L"LoaderImageIdentifier", loaded_image_path, FALSE); - ZeroMem(&config, sizeof(Config)); config_load_defaults(&config, root_dir); /* scan /EFI/Linux/ directory */ From 996daf2fa9ee54065c7c14affae95682b5e744d9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 23 Oct 2018 13:43:10 +0200 Subject: [PATCH 10/11] sd-boot: make sure special menu items also work if menu is skipped While it doesn't really make much sense to set "auto-reboot-to-firmware" as oneshot boot item, let's still support it properly, by also dispatching such a menu item if selected. --- src/boot/efi/boot.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index e62c766335..d3a26fd7a0 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -2215,12 +2215,12 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { uefi_call_wrapper(BS->SetWatchdogTimer, 4, 0, 0x10000, 0, NULL); if (!menu_run(&config, &entry, loaded_image_path)) break; + } - /* run special entry like "reboot" */ - if (entry->call) { - entry->call(); - continue; - } + /* run special entry like "reboot" */ + if (entry->call) { + entry->call(); + continue; } config_entry_bump_counters(entry, root_dir); From 3b42f3491054c70335dbfd6cda789f88f5241228 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 23 Oct 2018 13:44:23 +0200 Subject: [PATCH 11/11] efi: NUL terminate strings read with efivar_get() let's be more careful and NUL terminate everything we read from EFI variables, in case it isn't already. --- src/boot/efi/util.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/boot/efi/util.c b/src/boot/efi/util.c index 77d7b81fea..320b12ef85 100644 --- a/src/boot/efi/util.c +++ b/src/boot/efi/util.c @@ -105,18 +105,33 @@ EFI_STATUS efivar_set_int(CHAR16 *name, UINTN i, BOOLEAN persistent) { EFI_STATUS efivar_get(const CHAR16 *name, CHAR16 **value) { _cleanup_freepool_ CHAR8 *buf = NULL; + EFI_STATUS err; CHAR16 *val; UINTN size; - EFI_STATUS err; err = efivar_get_raw(&loader_guid, name, &buf, &size); if (EFI_ERROR(err)) return err; - val = StrDuplicate((CHAR16 *)buf); + /* Make sure there are no incomplete characters in the buffer */ + if ((size % 2) != 0) + return EFI_INVALID_PARAMETER; + + /* Return buffer directly if it happens to be NUL terminated already */ + if (size >= 2 && buf[size-2] == 0 && buf[size-1] == 0) { + *value = (CHAR16*) buf; + buf = NULL; + return EFI_SUCCESS; + } + + /* Make sure a terminating NUL is available at the end */ + val = AllocatePool(size + 2); if (!val) return EFI_OUT_OF_RESOURCES; + CopyMem(val, buf, size); + val[size/2] = 0; /* NUL terminate */ + *value = val; return EFI_SUCCESS; }