From 0e8ecba96e72ecb57a5f2a668b645214e2f3b12b Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Thu, 9 Jun 2022 10:05:52 +0200 Subject: [PATCH 01/13] boot: Add efi_fnmatch Unlike MetaiMatch from the UEFI spec/EDK2 this implementation is intended to be compatible with POSIX fnmatch. --- src/boot/efi/efi-string.c | 105 +++++++++++++++++++++++++++++++++ src/boot/efi/efi-string.h | 2 + src/boot/efi/test-efi-string.c | 48 +++++++++++++++ 3 files changed, 155 insertions(+) diff --git a/src/boot/efi/efi-string.c b/src/boot/efi/efi-string.c index 4b405155a0..505830e310 100644 --- a/src/boot/efi/efi-string.c +++ b/src/boot/efi/efi-string.c @@ -139,6 +139,111 @@ DEFINE_STRCHR(char16_t, strchr16); DEFINE_STRNDUP(char, xstrndup8, strnlen8); DEFINE_STRNDUP(char16_t, xstrndup16, strnlen16); +/* Patterns are fnmatch-compatible (with reduced feature support). */ +static bool efi_fnmatch_internal(const char16_t *p, const char16_t *h, int max_depth) { + assert(p); + assert(h); + + if (max_depth == 0) + return false; + + for (;; p++, h++) + switch (*p) { + case '\0': + /* End of pattern. Check that haystack is now empty. */ + return *h == '\0'; + + case '\\': + p++; + if (*p == '\0' || *p != *h) + /* Trailing escape or no match. */ + return false; + break; + + case '?': + if (*h == '\0') + /* Early end of haystack. */ + return false; + break; + + case '*': + /* No need to recurse for consecutive '*'. */ + while (*p == '*') + p++; + + do { + /* Try matching haystack with remaining pattern. */ + if (efi_fnmatch_internal(p, h, max_depth - 1)) + return true; + + /* Otherwise, we match one char here. */ + h++; + } while (*h != '\0'); + + /* End of haystack. Pattern needs to be empty too for a match. */ + return *p == '\0'; + + case '[': + if (*h == '\0') + /* Early end of haystack. */ + return false; + + bool first = true, can_range = true, match = false; + for (;; first = false) { + p++; + if (*p == '\0') + return false; + + if (*p == '\\') { + p++; + if (*p == '\0') + return false; + match |= *p == *h; + can_range = true; + continue; + } + + /* End of set unless it's the first char. */ + if (*p == ']' && !first) + break; + + /* Range pattern if '-' is not first or last in set. */ + if (*p == '-' && can_range && !first && *(p + 1) != ']') { + char16_t low = *(p - 1); + p++; + if (*p == '\\') + p++; + if (*p == '\0') + return false; + + if (low <= *h && *h <= *p) + match = true; + + /* Ranges cannot be chained: [a-c-f] == [-abcf] */ + can_range = false; + continue; + } + + if (*p == *h) + match = true; + can_range = true; + } + + if (!match) + return false; + break; + + default: + if (*p != *h) + /* Single char mismatch. */ + return false; + } +} + +bool efi_fnmatch(const char16_t *pattern, const char16_t *haystack) { + return efi_fnmatch_internal(pattern, haystack, 32); +} + int efi_memcmp(const void *p1, const void *p2, size_t n) { const uint8_t *up1 = p1, *up2 = p2; int r; diff --git a/src/boot/efi/efi-string.h b/src/boot/efi/efi-string.h index b4870975bc..55c9c6e47a 100644 --- a/src/boot/efi/efi-string.h +++ b/src/boot/efi/efi-string.h @@ -99,6 +99,8 @@ static inline char16_t *xstrdup16(const char16_t *s) { return xstrndup16(s, SIZE_MAX); } +bool efi_fnmatch(const char16_t *pattern, const char16_t *haystack); + #ifdef SD_BOOT /* The compiler normally has knowledge about standard functions such as memcmp, but this is not the case when * compiling with -ffreestanding. By referring to builtins, the compiler can check arguments and do diff --git a/src/boot/efi/test-efi-string.c b/src/boot/efi/test-efi-string.c index b688c6ae41..5aaa1f713f 100644 --- a/src/boot/efi/test-efi-string.c +++ b/src/boot/efi/test-efi-string.c @@ -1,5 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include + #include "efi-string.h" #include "tests.h" @@ -322,6 +324,52 @@ TEST(xstrdup16) { free(s); } +#define TEST_FNMATCH_ONE(pattern, haystack, expect) \ + ({ \ + assert_se(fnmatch(pattern, haystack, 0) == (expect ? 0 : FNM_NOMATCH)); \ + assert_se(efi_fnmatch(u##pattern, u##haystack) == expect); \ + }) + +TEST(efi_fnmatch) { + TEST_FNMATCH_ONE("", "", true); + TEST_FNMATCH_ONE("abc", "abc", true); + TEST_FNMATCH_ONE("aBc", "abc", false); + TEST_FNMATCH_ONE("b", "a", false); + TEST_FNMATCH_ONE("b", "", false); + TEST_FNMATCH_ONE("abc", "a", false); + TEST_FNMATCH_ONE("a?c", "azc", true); + TEST_FNMATCH_ONE("???", "?.9", true); + TEST_FNMATCH_ONE("1?", "1", false); + TEST_FNMATCH_ONE("***", "", true); + TEST_FNMATCH_ONE("*", "123", true); + TEST_FNMATCH_ONE("**", "abcd", true); + TEST_FNMATCH_ONE("*b*", "abcd", true); + TEST_FNMATCH_ONE("*.conf", "arch.conf", true); + TEST_FNMATCH_ONE("debian-*.conf", "debian-wheezy.conf", true); + TEST_FNMATCH_ONE("debian-*.*", "debian-wheezy.efi", true); + TEST_FNMATCH_ONE("ab*cde", "abzcd", false); + TEST_FNMATCH_ONE("\\*\\a\\[", "*a[", true); + TEST_FNMATCH_ONE("[abc] [abc] [abc]", "a b c", true); + TEST_FNMATCH_ONE("abc]", "abc]", true); + TEST_FNMATCH_ONE("[abc]", "z", false); + TEST_FNMATCH_ONE("[abc", "a", false); + TEST_FNMATCH_ONE("[][!] [][!] [][!]", "[ ] !", true); + TEST_FNMATCH_ONE("[]-] []-]", "] -", true); + TEST_FNMATCH_ONE("[1\\]] [1\\]]", "1 ]", true); + TEST_FNMATCH_ONE("[$-\\+]", "&", true); + TEST_FNMATCH_ONE("[1-3A-C] [1-3A-C]", "2 B", true); + TEST_FNMATCH_ONE("[3-5] [3-5] [3-5]", "3 4 5", true); + TEST_FNMATCH_ONE("[f-h] [f-h] [f-h]", "f g h", true); + TEST_FNMATCH_ONE("[a-c-f] [a-c-f] [a-c-f] [a-c-f] [a-c-f]", "a b c - f", true); + TEST_FNMATCH_ONE("[a-c-f]", "e", false); + TEST_FNMATCH_ONE("[--0] [--0] [--0]", "- . 0", true); + TEST_FNMATCH_ONE("[+--] [+--] [+--]", "+ , -", true); + TEST_FNMATCH_ONE("[f-l]", "m", false); + TEST_FNMATCH_ONE("[b]", "z-a", false); + TEST_FNMATCH_ONE("[a\\-z]", "b", false); + TEST_FNMATCH_ONE("?a*b[.-0]c", "/a/b/c", true); +} + TEST(efi_memcmp) { assert_se(efi_memcmp(NULL, NULL, 0) == 0); assert_se(efi_memcmp(NULL, NULL, 1) == 0); From 23742af5225944b5ed7db4633038b4c025660945 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Thu, 9 Jun 2022 10:07:06 +0200 Subject: [PATCH 02/13] boot: Drop use of MetaiMatch A future commit will add support for unicode collation protocol that allows case folding and comparing strings with locale awareness. But it only operates on whole strings, so fnmatch cannot use those without a heavy cost. Instead we just case fold the patterns instead (the IDs we try to match are already lower case). --- man/loader.conf.xml | 9 ++++++++- src/boot/efi/boot.c | 15 +++++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/man/loader.conf.xml b/man/loader.conf.xml index 509412ec9d..43a115dcad 100644 --- a/man/loader.conf.xml +++ b/man/loader.conf.xml @@ -110,7 +110,14 @@ - + + + Supported glob wilcard patterns are ?, *, and + […] (including ranges). Note that these patterns use the same syntax as + glob7, but do not + support all features. In particular, set negation and named character classes are not supported. The + matching is done case-insensitively on the entry ID (as shown by bootctl + list). diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index 71f1227923..60e8786e3e 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -1170,7 +1170,7 @@ static void config_defaults_load_from_file(Config *config, CHAR8 *content) { } if (streq8((char *) key, "default")) { - if (value[0] == '@' && !streq8((char *) value, "@saved")) { + if (value[0] == '@' && !strcaseeq8((char *) value, "@saved")) { log_error_stall(L"Unsupported special entry identifier: %a", value); continue; } @@ -1606,6 +1606,11 @@ static void config_load_defaults(Config *config, EFI_FILE *root_dir) { (void) efivar_get(LOADER_GUID, L"LoaderEntryDefault", &config->entry_default_efivar); + strtolower16(config->entry_default_config); + strtolower16(config->entry_default_efivar); + strtolower16(config->entry_oneshot); + strtolower16(config->entry_saved); + config->use_saved_entry = streq16(config->entry_default_config, L"@saved"); config->use_saved_entry_efivar = streq16(config->entry_default_efivar, L"@saved"); if (config->use_saved_entry || config->use_saved_entry_efivar) @@ -1710,14 +1715,16 @@ static int config_entry_compare(const ConfigEntry *a, const ConfigEntry *b) { return CMP(a->tries_done, b->tries_done); } -static UINTN config_entry_find(Config *config, const CHAR16 *needle) { +static UINTN config_entry_find(Config *config, const CHAR16 *pattern) { assert(config); - if (!needle) + /* We expect pattern and entry IDs to be already case folded. */ + + if (!pattern) return IDX_INVALID; for (UINTN i = 0; i < config->entry_count; i++) - if (MetaiMatch(config->entries[i]->id, (CHAR16*) needle)) + if (efi_fnmatch(pattern, config->entries[i]->id)) return i; return IDX_INVALID; From 7ebce8145e627b04a976a5e60b91552321121293 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Thu, 26 May 2022 10:27:32 +0200 Subject: [PATCH 03/13] boot: Drop use of LibGetSystemConfigurationTable --- src/boot/efi/devicetree.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/boot/efi/devicetree.c b/src/boot/efi/devicetree.c index 81c6e60ca6..d5186d10d0 100644 --- a/src/boot/efi/devicetree.c +++ b/src/boot/efi/devicetree.c @@ -8,6 +8,13 @@ #define FDT_V1_SIZE (7*4) +static void *get_dtb_table(void) { + for (UINTN i = 0; i < ST->NumberOfTableEntries; i++) + if (memcmp(&EfiDtbTableGuid, &ST->ConfigurationTable[i].VendorGuid, sizeof(EfiDtbTableGuid)) == 0) + return ST->ConfigurationTable[i].VendorTable; + return NULL; +} + static EFI_STATUS devicetree_allocate(struct devicetree_state *state, UINTN size) { UINTN pages = DIV_ROUND_UP(size, EFI_PAGE_SIZE); EFI_STATUS err; @@ -74,8 +81,8 @@ EFI_STATUS devicetree_install(struct devicetree_state *state, EFI_FILE *root_dir assert(root_dir); assert(name); - err = LibGetSystemConfigurationTable(&EfiDtbTableGuid, &state->orig); - if (EFI_ERROR(err)) + state->orig = get_dtb_table(); + if (!state->orig) return EFI_UNSUPPORTED; err = root_dir->Open(root_dir, &handle, name, EFI_FILE_MODE_READ, EFI_FILE_READ_ONLY); @@ -114,8 +121,8 @@ EFI_STATUS devicetree_install_from_memory(struct devicetree_state *state, assert(state); assert(dtb_buffer && dtb_length > 0); - err = LibGetSystemConfigurationTable(&EfiDtbTableGuid, &state->orig); - if (EFI_ERROR(err)) + state->orig = get_dtb_table(); + if (!state->orig) return EFI_UNSUPPORTED; err = devicetree_allocate(state, dtb_length); From f747ca3ec3bdbdc55aecb5803dbd9f65bc0cd169 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Thu, 26 May 2022 10:46:58 +0200 Subject: [PATCH 04/13] boot: Drop use of LibOpenRoot --- src/boot/efi/boot.c | 20 +++++++++++--------- src/boot/efi/shim.c | 7 ++++--- src/boot/efi/util.c | 19 +++++++++++++++++++ src/boot/efi/util.h | 2 ++ src/boot/efi/xbootldr.c | 10 +++++----- 5 files changed, 41 insertions(+), 17 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index 60e8786e3e..dd5dad71f3 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -1947,8 +1947,9 @@ static void config_entry_add_osx(Config *config) { return; for (UINTN i = 0; i < n_handles; i++) { - _cleanup_(file_closep) EFI_FILE *root = LibOpenRoot(handles[i]); - if (!root) + _cleanup_(file_closep) EFI_FILE *root = NULL; + + if (open_volume(handles[i], &root) != EFI_SUCCESS) continue; if (config_entry_add_loader_auto( @@ -2249,7 +2250,7 @@ static void config_load_xbootldr( assert(device); err = xbootldr_open(device, &new_device, &root_dir); - if (EFI_ERROR(err)) + if (err != EFI_SUCCESS) return; config_entry_add_unified(config, new_device, root_dir); @@ -2344,9 +2345,10 @@ static EFI_STATUS image_start( if (entry->call) (void) entry->call(); - _cleanup_(file_closep) EFI_FILE *image_root = LibOpenRoot(entry->device); - if (!image_root) - return log_error_status_stall(EFI_DEVICE_ERROR, L"Error opening root path."); + _cleanup_(file_closep) EFI_FILE *image_root = NULL; + err = open_volume(entry->device, &image_root); + if (err != EFI_SUCCESS) + return log_error_status_stall(err, L"Error opening root path: %r", err); path = FileDevicePath(entry->device, entry->loader); if (!path) @@ -2602,9 +2604,9 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { export_variables(loaded_image, loaded_image_path, init_usec); - root_dir = LibOpenRoot(loaded_image->DeviceHandle); - if (!root_dir) - return log_error_status_stall(EFI_LOAD_ERROR, L"Unable to open root directory.", EFI_LOAD_ERROR); + err = open_volume(loaded_image->DeviceHandle, &root_dir); + if (err != EFI_SUCCESS) + return log_error_status_stall(err, L"Unable to open root directory: %r", err); if (secure_boot_enabled() && shim_loaded()) { err = security_policy_install(); diff --git a/src/boot/efi/shim.c b/src/boot/efi/shim.c index fb4aecaee0..663eafbae4 100644 --- a/src/boot/efi/shim.c +++ b/src/boot/efi/shim.c @@ -122,9 +122,10 @@ static EFIAPI EFI_STATUS security_policy_authentication (const EFI_SECURITY_PROT if (EFI_ERROR(status)) return status; - _cleanup_(file_closep) EFI_FILE *root = LibOpenRoot(h); - if (!root) - return EFI_NOT_FOUND; + _cleanup_(file_closep) EFI_FILE *root = NULL; + status = open_volume(h, &root); + if (status != EFI_SUCCESS) + return status; dev_path_str = DevicePathToStr(dp); if (!dev_path_str) diff --git a/src/boot/efi/util.c b/src/boot/efi/util.c index 06859d2f3a..7d607e04c7 100644 --- a/src/boot/efi/util.c +++ b/src/boot/efi/util.c @@ -682,3 +682,22 @@ void beep(UINTN beep_count) { } } #endif + +EFI_STATUS open_volume(EFI_HANDLE device, EFI_FILE **ret_file) { + EFI_STATUS err; + EFI_FILE *file; + EFI_SIMPLE_FILE_SYSTEM_PROTOCOL *volume; + + assert(ret_file); + + err = BS->HandleProtocol(device, &FileSystemProtocol, (void **) &volume); + if (err != EFI_SUCCESS) + return err; + + err = volume->OpenVolume(volume, &file); + if (err != EFI_SUCCESS) + return err; + + *ret_file = file; + return EFI_SUCCESS; +} diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h index 06c298d776..75d3e51415 100644 --- a/src/boot/efi/util.h +++ b/src/boot/efi/util.h @@ -165,3 +165,5 @@ void beep(UINTN beep_count); #else static inline void beep(UINTN beep_count) {} #endif + +EFI_STATUS open_volume(EFI_HANDLE device, EFI_FILE **ret_file); diff --git a/src/boot/efi/xbootldr.c b/src/boot/efi/xbootldr.c index bc82768715..583bc4216f 100644 --- a/src/boot/efi/xbootldr.c +++ b/src/boot/efi/xbootldr.c @@ -247,17 +247,17 @@ EFI_STATUS xbootldr_open(EFI_HANDLE *device, EFI_HANDLE *ret_device, EFI_FILE ** assert(ret_root_dir); err = find_device(device, &partition_path); - if (EFI_ERROR(err)) + if (err != EFI_SUCCESS) return err; EFI_DEVICE_PATH *dp = partition_path; err = BS->LocateDevicePath(&BlockIoProtocol, &dp, &new_device); - if (EFI_ERROR(err)) + if (err != EFI_SUCCESS) return err; - root_dir = LibOpenRoot(new_device); - if (!root_dir) - return EFI_NOT_FOUND; + err = open_volume(new_device, &root_dir); + if (err != EFI_SUCCESS) + return err; *ret_device = new_device; *ret_root_dir = root_dir; From 6a261332bc266e4b52e393dfacfdbb6383bc2bf3 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Thu, 26 May 2022 10:59:53 +0200 Subject: [PATCH 05/13] boot: Drop use of LibLocateProtocol --- src/boot/efi/console.c | 4 ++-- src/boot/efi/devicetree.c | 2 +- src/boot/efi/graphics.c | 2 +- src/boot/efi/measure.c | 4 ++-- src/boot/efi/random-seed.c | 2 +- src/boot/efi/splash.c | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/boot/efi/console.c b/src/boot/efi/console.c index 937ad7ddfd..009d3672b8 100644 --- a/src/boot/efi/console.c +++ b/src/boot/efi/console.c @@ -48,7 +48,7 @@ EFI_STATUS console_key_read(UINT64 *key, UINT64 timeout_usec) { if (!checked) { /* Get the *first* TextInputEx device.*/ - err = LibLocateProtocol(&SimpleTextInputExProtocol, (void **) &extraInEx); + err = BS->LocateProtocol(&SimpleTextInputExProtocol, NULL, (void **) &extraInEx); if (EFI_ERROR(err) || BS->CheckEvent(extraInEx->WaitForKeyEx) == EFI_INVALID_PARAMETER) /* If WaitForKeyEx fails here, the firmware pretends it talks this * protocol, but it really doesn't. */ @@ -185,7 +185,7 @@ EFI_STATUS query_screen_resolution(UINT32 *ret_w, UINT32 *ret_h) { EFI_STATUS err; EFI_GRAPHICS_OUTPUT_PROTOCOL *go; - err = LibLocateProtocol(&GraphicsOutputProtocol, (void **) &go); + err = BS->LocateProtocol(&GraphicsOutputProtocol, NULL, (void **) &go); if (EFI_ERROR(err)) return err; diff --git a/src/boot/efi/devicetree.c b/src/boot/efi/devicetree.c index d5186d10d0..15513a98e6 100644 --- a/src/boot/efi/devicetree.c +++ b/src/boot/efi/devicetree.c @@ -41,7 +41,7 @@ static EFI_STATUS devicetree_fixup(struct devicetree_state *state, UINTN len) { assert(state); - err = LibLocateProtocol(&EfiDtFixupProtocol, (void **)&fixup); + err = BS->LocateProtocol(&EfiDtFixupProtocol, NULL, (void **) &fixup); if (EFI_ERROR(err)) return log_error_status_stall(EFI_SUCCESS, L"Could not locate device tree fixup protocol, skipping."); diff --git a/src/boot/efi/graphics.c b/src/boot/efi/graphics.c index 62a3512fa3..9e69c2703d 100644 --- a/src/boot/efi/graphics.c +++ b/src/boot/efi/graphics.c @@ -19,7 +19,7 @@ EFI_STATUS graphics_mode(BOOLEAN on) { BOOLEAN stdin_locked; EFI_STATUS err; - err = LibLocateProtocol((EFI_GUID*) EFI_CONSOLE_CONTROL_GUID, (void **)&ConsoleControl); + err = BS->LocateProtocol((EFI_GUID *) EFI_CONSOLE_CONTROL_GUID, NULL, (void **) &ConsoleControl); if (EFI_ERROR(err)) /* console control protocol is nonstandard and might not exist. */ return err == EFI_NOT_FOUND ? EFI_SUCCESS : err; diff --git a/src/boot/efi/measure.c b/src/boot/efi/measure.c index c9ce8f19e0..b388d4bc40 100644 --- a/src/boot/efi/measure.c +++ b/src/boot/efi/measure.c @@ -84,7 +84,7 @@ static EFI_TCG *tcg1_interface_check(void) { UINT32 features; EFI_TCG *tcg; - status = LibLocateProtocol((EFI_GUID*) EFI_TCG_GUID, (void **) &tcg); + status = BS->LocateProtocol((EFI_GUID *) EFI_TCG_GUID, NULL, (void **) &tcg); if (EFI_ERROR(status)) return NULL; @@ -113,7 +113,7 @@ static EFI_TCG2 * tcg2_interface_check(void) { EFI_STATUS status; EFI_TCG2 *tcg; - status = LibLocateProtocol((EFI_GUID*) EFI_TCG2_GUID, (void **) &tcg); + status = BS->LocateProtocol((EFI_GUID *) EFI_TCG2_GUID, NULL, (void **) &tcg); if (EFI_ERROR(status)) return NULL; diff --git a/src/boot/efi/random-seed.c b/src/boot/efi/random-seed.c index a9ee273673..f1221ae2a7 100644 --- a/src/boot/efi/random-seed.c +++ b/src/boot/efi/random-seed.c @@ -26,7 +26,7 @@ static EFI_STATUS acquire_rng(UINTN size, void **ret) { /* Try to acquire the specified number of bytes from the UEFI RNG */ - err = LibLocateProtocol((EFI_GUID*) EFI_RNG_GUID, (void**) &rng); + err = BS->LocateProtocol((EFI_GUID *) EFI_RNG_GUID, NULL, (void **) &rng); if (EFI_ERROR(err)) return err; if (!rng) diff --git a/src/boot/efi/splash.c b/src/boot/efi/splash.c index e0d075c911..2fd2db6ba0 100644 --- a/src/boot/efi/splash.c +++ b/src/boot/efi/splash.c @@ -279,7 +279,7 @@ EFI_STATUS graphics_splash(const UINT8 *content, UINTN len, const EFI_GRAPHICS_O background = &pixel; } - err = LibLocateProtocol(&GraphicsOutputProtocol, (void **)&GraphicsOutput); + err = BS->LocateProtocol(&GraphicsOutputProtocol, NULL, (void **) &GraphicsOutput); if (EFI_ERROR(err)) return err; From 5594ebee99aaa2d66978b1c39d7e2084110dc76c Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Thu, 26 May 2022 13:07:30 +0200 Subject: [PATCH 06/13] boot: Drop use of DevicePathFromHandle --- src/boot/efi/disk.c | 10 ++++++---- src/boot/efi/xbootldr.c | 7 ++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/boot/efi/disk.c b/src/boot/efi/disk.c index b7beac3d08..fcd5812ff1 100644 --- a/src/boot/efi/disk.c +++ b/src/boot/efi/disk.c @@ -7,16 +7,18 @@ #include "util.h" EFI_STATUS disk_get_part_uuid(EFI_HANDLE *handle, CHAR16 uuid[static 37]) { + EFI_STATUS err; EFI_DEVICE_PATH *device_path; _cleanup_freepool_ EFI_DEVICE_PATH *paths = NULL; + /* export the device path this image is started from */ + if (!handle) return EFI_NOT_FOUND; - /* export the device path this image is started from */ - device_path = DevicePathFromHandle(handle); - if (!device_path) - return EFI_NOT_FOUND; + err = BS->HandleProtocol(handle, &DevicePathProtocol, (void **) &device_path); + if (err != EFI_SUCCESS) + return err; paths = UnpackDevicePath(device_path); for (EFI_DEVICE_PATH *path = paths; !IsDevicePathEnd(path); path = NextDevicePathNode(path)) { diff --git a/src/boot/efi/xbootldr.c b/src/boot/efi/xbootldr.c index 583bc4216f..84e443135c 100644 --- a/src/boot/efi/xbootldr.c +++ b/src/boot/efi/xbootldr.c @@ -154,9 +154,10 @@ static EFI_STATUS find_device(EFI_HANDLE *device, EFI_DEVICE_PATH **ret_device_p assert(device); assert(ret_device_path); - EFI_DEVICE_PATH *partition_path = DevicePathFromHandle(device); - if (!partition_path) - return EFI_NOT_FOUND; + EFI_DEVICE_PATH *partition_path; + err = BS->HandleProtocol(device, &DevicePathProtocol, (void **) &partition_path); + if (err != EFI_SUCCESS) + return err; /* Find the (last) partition node itself. */ EFI_DEVICE_PATH *part_node = NULL; From 9148312fab2cb67c7716c228886f6615beb3b6a7 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Sat, 28 May 2022 19:29:41 +0200 Subject: [PATCH 07/13] boot: Add xmalloc --- src/boot/efi/util.h | 53 +++++++++++++++++++++++------ src/fundamental/macro-fundamental.h | 3 +- 2 files changed, 43 insertions(+), 13 deletions(-) diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h index 75d3e51415..2245c21ce7 100644 --- a/src/boot/efi/util.h +++ b/src/boot/efi/util.h @@ -30,6 +30,48 @@ assert_cc(sizeof(int) == sizeof(UINT32)); # error "Unexpected pointer size" #endif +static inline void free(void *p) { + if (!p) + return; + + /* Debugging an invalid free requires trace logging to find the call site or a debugger attached. For + * release builds it is not worth the bother to even warn when we cannot even print a call stack. */ +#ifdef EFI_DEBUG + assert_se(BS->FreePool(p) == EFI_SUCCESS); +#else + (void) BS->FreePool(p); +#endif +} + +static inline void freep(void *p) { + free(*(void **) p); +} + +#define _cleanup_freepool_ _cleanup_free_ +#define _cleanup_free_ _cleanup_(freep) + +_malloc_ _alloc_(1) _returns_nonnull_ _warn_unused_result_ +static inline void *xmalloc(size_t size) { + void *p; + assert_se(BS->AllocatePool(EfiBootServicesData, size, &p) == EFI_SUCCESS); + return p; +} + +_malloc_ _alloc_(1, 2) _returns_nonnull_ _warn_unused_result_ +static inline void *xmalloc_multiply(size_t size, size_t n) { + assert_se(!__builtin_mul_overflow(size, n, &size)); + return xmalloc(size); +} + +/* Use malloc attribute as this never returns p like userspace realloc. */ +_malloc_ _alloc_(3) _returns_nonnull_ _warn_unused_result_ +static inline void *xrealloc(void *p, size_t old_size, size_t new_size) { + void *r = xmalloc(new_size); + memcpy(r, p, MIN(old_size, new_size)); + free(p); + return r; +} + #define xnew_alloc(type, n, alloc) \ ({ \ UINTN _alloc_size; \ @@ -65,17 +107,6 @@ CHAR16 *xstra_to_str(const CHAR8 *stra); EFI_STATUS file_read(EFI_FILE *dir, const CHAR16 *name, UINTN off, UINTN size, CHAR8 **content, UINTN *content_size); -static inline void free_poolp(void *p) { - void *q = *(void**) p; - - if (!q) - return; - - (void) BS->FreePool(q); -} - -#define _cleanup_freepool_ _cleanup_(free_poolp) - static inline void file_closep(EFI_FILE **handle) { if (!*handle) return; diff --git a/src/fundamental/macro-fundamental.h b/src/fundamental/macro-fundamental.h index f78c62ef25..18370ac46a 100644 --- a/src/fundamental/macro-fundamental.h +++ b/src/fundamental/macro-fundamental.h @@ -25,6 +25,7 @@ #define _public_ __attribute__((__visibility__("default"))) #define _pure_ __attribute__((__pure__)) #define _retain_ __attribute__((__retain__)) +#define _returns_nonnull_ __attribute__((__returns_nonnull__)) #define _section_(x) __attribute__((__section__(x))) #define _sentinel_ __attribute__((__sentinel__)) #define _unlikely_(x) (__builtin_expect(!!(x), 0)) @@ -76,8 +77,6 @@ #endif #define static_assert _Static_assert #define assert_se(expr) ({ _likely_(expr) ? VOID_0 : efi_assert(#expr, __FILE__, __LINE__, __PRETTY_FUNCTION__); }) - - #define free(a) FreePool(a) #endif /* This passes the argument through after (if asserts are enabled) checking that it is not null. */ From 0af26643d09bab0a886986c65d42408c8bc00853 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Sat, 28 May 2022 19:36:21 +0200 Subject: [PATCH 08/13] boot: Use xmalloc This drops the unused xnew0 and xallocate_zero_pool as there is only two users of it. _cleanup_freepool_ will be phased out once the types in the declarations are changed/renamed. --- src/boot/efi/boot.c | 85 +++++++++++++++++++------------------- src/boot/efi/cpio.c | 8 ++-- src/boot/efi/efi-string.c | 1 - src/boot/efi/initrd.c | 4 +- src/boot/efi/measure.c | 6 ++- src/boot/efi/pe.c | 2 +- src/boot/efi/random-seed.c | 6 +-- src/boot/efi/stub.c | 2 +- src/boot/efi/util.c | 24 +++++------ src/boot/efi/util.h | 13 +----- src/boot/efi/xbootldr.c | 4 +- 11 files changed, 72 insertions(+), 83 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index dd5dad71f3..f00f004eaa 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -318,7 +318,7 @@ static BOOLEAN line_edit( case KEYPRESS(0, CHAR_CARRIAGE_RETURN, 0): /* EZpad Mini 4s firmware sends malformed events */ case KEYPRESS(0, CHAR_CARRIAGE_RETURN, CHAR_CARRIAGE_RETURN): /* Teclast X98+ II firmware sends malformed events */ if (!streq16(line, *line_in)) { - FreePool(*line_in); + free(*line_in); *line_in = TAKE_PTR(line); } return TRUE; @@ -741,7 +741,7 @@ static BOOLEAN menu_run( } if (timeout_remain > 0) { - FreePool(status); + free(status); status = xpool_print(L"Boot in %u s.", timeout_remain); } @@ -872,7 +872,7 @@ static BOOLEAN menu_run( case KEYPRESS(0, 0, 'd'): case KEYPRESS(0, 0, 'D'): if (config->idx_default_efivar != idx_highlight) { - FreePool(config->entry_default_efivar); + free(config->entry_default_efivar); config->entry_default_efivar = xstrdup16(config->entries[idx_highlight]->id); config->idx_default_efivar = idx_highlight; status = xstrdup16(u"Default boot entry selected."); @@ -1037,11 +1037,10 @@ static void config_add_entry(Config *config, ConfigEntry *entry) { assert(config->entry_count < IDX_MAX); if ((config->entry_count & 15) == 0) { - UINTN i = config->entry_count + 16; - config->entries = xreallocate_pool( + config->entries = xrealloc( config->entries, sizeof(void *) * config->entry_count, - sizeof(void *) * i); + sizeof(void *) * (config->entry_count + 16)); } config->entries[config->entry_count++] = entry; } @@ -1050,20 +1049,20 @@ static void config_entry_free(ConfigEntry *entry) { if (!entry) return; - FreePool(entry->id); - FreePool(entry->title_show); - FreePool(entry->title); - FreePool(entry->sort_key); - FreePool(entry->version); - FreePool(entry->machine_id); - FreePool(entry->loader); - FreePool(entry->devicetree); - FreePool(entry->options); + free(entry->id); + free(entry->title_show); + free(entry->title); + free(entry->sort_key); + free(entry->version); + free(entry->machine_id); + free(entry->loader); + free(entry->devicetree); + free(entry->options); strv_free(entry->initrd); - FreePool(entry->path); - FreePool(entry->current_name); - FreePool(entry->next_name); - FreePool(entry); + free(entry->path); + free(entry->current_name); + free(entry->next_name); + free(entry); } static inline void config_entry_freep(ConfigEntry **entry) { @@ -1174,7 +1173,7 @@ static void config_defaults_load_from_file(Config *config, CHAR8 *content) { log_error_stall(L"Unsupported special entry identifier: %a", value); continue; } - FreePool(config->entry_default_config); + free(config->entry_default_config); config->entry_default_config = xstra_to_str(value); continue; } @@ -1416,7 +1415,7 @@ static void config_entry_bump_counters(ConfigEntry *entry, EFI_FILE *root_dir) { /* If the file we just renamed is the loader path, then let's update that. */ if (streq16(entry->loader, old_path)) { - FreePool(entry->loader); + free(entry->loader); entry->loader = TAKE_PTR(new_path); } } @@ -1451,31 +1450,31 @@ static void config_entry_add_type1( while ((line = line_get_key_value(content, (CHAR8 *)" \t", &pos, &key, &value))) { if (streq8((char *) key, "title")) { - FreePool(entry->title); + free(entry->title); entry->title = xstra_to_str(value); continue; } if (streq8((char *) key, "sort-key")) { - FreePool(entry->sort_key); + free(entry->sort_key); entry->sort_key = xstra_to_str(value); continue; } if (streq8((char *) key, "version")) { - FreePool(entry->version); + free(entry->version); entry->version = xstra_to_str(value); continue; } if (streq8((char *) key, "machine-id")) { - FreePool(entry->machine_id); + free(entry->machine_id); entry->machine_id = xstra_to_str(value); continue; } if (streq8((char *) key, "linux")) { - FreePool(entry->loader); + free(entry->loader); entry->type = LOADER_LINUX; entry->loader = xstra_to_path(value); entry->key = 'l'; @@ -1484,7 +1483,7 @@ static void config_entry_add_type1( if (streq8((char *) key, "efi")) { entry->type = LOADER_EFI; - FreePool(entry->loader); + free(entry->loader); entry->loader = xstra_to_path(value); /* do not add an entry for ourselves */ @@ -1505,13 +1504,13 @@ static void config_entry_add_type1( } if (streq8((char *) key, "devicetree")) { - FreePool(entry->devicetree); + free(entry->devicetree); entry->devicetree = xstra_to_path(value); continue; } if (streq8((char *) key, "initrd")) { - entry->initrd = xreallocate_pool( + entry->initrd = xrealloc( entry->initrd, n_initrd == 0 ? 0 : (n_initrd + 1) * sizeof(UINT16 *), (n_initrd + 2) * sizeof(UINT16 *)); @@ -1528,7 +1527,7 @@ static void config_entry_add_type1( CHAR16 *s; s = xpool_print(L"%s %s", entry->options, new); - FreePool(entry->options); + free(entry->options); entry->options = s; } else entry->options = TAKE_PTR(new); @@ -2141,49 +2140,49 @@ static void config_entry_add_unified( /* read properties from the embedded os-release file */ while ((line = line_get_key_value(content, (CHAR8 *)"=", &pos, &key, &value))) { if (streq8((char *) key, "PRETTY_NAME")) { - FreePool(os_pretty_name); + free(os_pretty_name); os_pretty_name = xstra_to_str(value); continue; } if (streq8((char *) key, "IMAGE_ID")) { - FreePool(os_image_id); + free(os_image_id); os_image_id = xstra_to_str(value); continue; } if (streq8((char *) key, "NAME")) { - FreePool(os_name); + free(os_name); os_name = xstra_to_str(value); continue; } if (streq8((char *) key, "ID")) { - FreePool(os_id); + free(os_id); os_id = xstra_to_str(value); continue; } if (streq8((char *) key, "IMAGE_VERSION")) { - FreePool(os_image_version); + free(os_image_version); os_image_version = xstra_to_str(value); continue; } if (streq8((char *) key, "VERSION")) { - FreePool(os_version); + free(os_version); os_version = xstra_to_str(value); continue; } if (streq8((char *) key, "VERSION_ID")) { - FreePool(os_version_id); + free(os_version_id); os_version_id = xstra_to_str(value); continue; } if (streq8((char *) key, "BUILD_ID")) { - FreePool(os_build_id); + free(os_build_id); os_build_id = xstra_to_str(value); continue; } @@ -2308,7 +2307,7 @@ static EFI_STATUS initrd_prepare( UINTN new_size, read_size = info->FileSize; if (__builtin_add_overflow(size, read_size, &new_size)) return EFI_OUT_OF_RESOURCES; - initrd = xreallocate_pool(initrd, size, new_size); + initrd = xrealloc(initrd, size, new_size); err = handle->Read(handle, &read_size, initrd + size); if (EFI_ERROR(err)) @@ -2422,9 +2421,9 @@ static void config_free(Config *config) { assert(config); for (UINTN i = 0; i < config->entry_count; i++) config_entry_free(config->entries[i]); - FreePool(config->entries); - FreePool(config->entry_default_config); - FreePool(config->entry_oneshot); + free(config->entries); + free(config->entry_default_config); + free(config->entry_oneshot); } static void config_write_entries_to_variable(Config *config) { @@ -2437,7 +2436,7 @@ static void config_write_entries_to_variable(Config *config) { for (UINTN i = 0; i < config->entry_count; i++) sz += strsize16(config->entries[i]->id); - p = buffer = xallocate_pool(sz); + p = buffer = xmalloc(sz); for (UINTN i = 0; i < config->entry_count; i++) { UINTN l; diff --git a/src/boot/efi/cpio.c b/src/boot/efi/cpio.c index aebffb51b7..454d79e118 100644 --- a/src/boot/efi/cpio.c +++ b/src/boot/efi/cpio.c @@ -111,7 +111,7 @@ static EFI_STATUS pack_cpio_one( if (*cpio_buffer_size > UINTN_MAX - l) /* overflow check */ return EFI_OUT_OF_RESOURCES; - a = xreallocate_pool(*cpio_buffer, *cpio_buffer_size, *cpio_buffer_size + l); + a = xrealloc(*cpio_buffer, *cpio_buffer_size, *cpio_buffer_size + l); *cpio_buffer = a; a = (CHAR8*) *cpio_buffer + *cpio_buffer_size; @@ -195,7 +195,7 @@ static EFI_STATUS pack_cpio_dir( if (*cpio_buffer_size > UINTN_MAX - l) /* overflow check */ return EFI_OUT_OF_RESOURCES; - *cpio_buffer = a = xreallocate_pool(*cpio_buffer, *cpio_buffer_size, *cpio_buffer_size + l); + *cpio_buffer = a = xrealloc(*cpio_buffer, *cpio_buffer_size, *cpio_buffer_size + l); a = (CHAR8*) *cpio_buffer + *cpio_buffer_size; memcpy(a, "070701", 6); /* magic ID */ @@ -297,7 +297,7 @@ static EFI_STATUS pack_cpio_trailer( assert(cpio_buffer_size); assert_cc(sizeof(trailer) % 4 == 0); - *cpio_buffer = xreallocate_pool(*cpio_buffer, *cpio_buffer_size, *cpio_buffer_size + sizeof(trailer)); + *cpio_buffer = xrealloc(*cpio_buffer, *cpio_buffer_size, *cpio_buffer_size + sizeof(trailer)); memcpy((UINT8*) *cpio_buffer + *cpio_buffer_size, trailer, sizeof(trailer)); *cpio_buffer_size += sizeof(trailer); @@ -401,7 +401,7 @@ EFI_STATUS pack_cpio( return log_oom(); m = n_items + 16; - items = xreallocate_pool(items, n_allocated * sizeof(UINT16*), m * sizeof(UINT16*)); + items = xrealloc(items, n_allocated * sizeof(UINT16*), m * sizeof(UINT16*)); n_allocated = m; } diff --git a/src/boot/efi/efi-string.c b/src/boot/efi/efi-string.c index 505830e310..80ef0ff076 100644 --- a/src/boot/efi/efi-string.c +++ b/src/boot/efi/efi-string.c @@ -7,7 +7,6 @@ #ifdef SD_BOOT # include "util.h" -# define xmalloc(n) xallocate_pool(n) #else # include # include "macro.h" diff --git a/src/boot/efi/initrd.c b/src/boot/efi/initrd.c index 2d0984990b..1b4e746483 100644 --- a/src/boot/efi/initrd.c +++ b/src/boot/efi/initrd.c @@ -103,7 +103,7 @@ EFI_STATUS initrd_register( &EfiLoadFile2Protocol, loader, NULL); if (EFI_ERROR(err)) - FreePool(loader); + free(loader); return err; } @@ -135,6 +135,6 @@ EFI_STATUS initrd_unregister(EFI_HANDLE initrd_handle) { return err; initrd_handle = NULL; - FreePool(loader); + free(loader); return EFI_SUCCESS; } diff --git a/src/boot/efi/measure.c b/src/boot/efi/measure.c index b388d4bc40..def42fece4 100644 --- a/src/boot/efi/measure.c +++ b/src/boot/efi/measure.c @@ -26,7 +26,8 @@ static EFI_STATUS tpm1_measure_to_pcr_and_event_log( assert(description); desc_len = strsize16(description); - tcg_event = xallocate_zero_pool(offsetof(TCG_PCR_EVENT, Event) + desc_len); + tcg_event = xmalloc(offsetof(TCG_PCR_EVENT, Event) + desc_len); + memset(tcg_event, 0, offsetof(TCG_PCR_EVENT, Event) + desc_len); *tcg_event = (TCG_PCR_EVENT) { .EventSize = desc_len, .PCRIndex = pcrindex, @@ -57,7 +58,8 @@ static EFI_STATUS tpm2_measure_to_pcr_and_event_log( assert(description); desc_len = strsize16(description); - tcg_event = xallocate_zero_pool(offsetof(EFI_TCG2_EVENT, Event) + desc_len); + tcg_event = xmalloc(offsetof(EFI_TCG2_EVENT, Event) + desc_len); + memset(tcg_event, 0, offsetof(EFI_TCG2_EVENT, Event) + desc_len); *tcg_event = (EFI_TCG2_EVENT) { .Size = offsetof(EFI_TCG2_EVENT, Event) + desc_len, .Header.HeaderSize = sizeof(EFI_TCG2_EVENT_HEADER), diff --git a/src/boot/efi/pe.c b/src/boot/efi/pe.c index f9bd57f0ce..c1a4064896 100644 --- a/src/boot/efi/pe.c +++ b/src/boot/efi/pe.c @@ -318,7 +318,7 @@ EFI_STATUS pe_file_locate_sections( return EFI_LOAD_ERROR; section_table_len = pe.FileHeader.NumberOfSections * sizeof(struct PeSectionHeader); - section_table = xallocate_pool(section_table_len); + section_table = xmalloc(section_table_len); if (!section_table) return EFI_OUT_OF_RESOURCES; diff --git a/src/boot/efi/random-seed.c b/src/boot/efi/random-seed.c index f1221ae2a7..116ccfe515 100644 --- a/src/boot/efi/random-seed.c +++ b/src/boot/efi/random-seed.c @@ -32,7 +32,7 @@ static EFI_STATUS acquire_rng(UINTN size, void **ret) { if (!rng) return EFI_UNSUPPORTED; - data = xallocate_pool(size); + data = xmalloc(size); err = rng->GetRNG(rng, NULL, size, data); if (EFI_ERROR(err)) @@ -99,7 +99,7 @@ static EFI_STATUS hash_many( /* Hashes the specified parameters in counter mode, generating n hash values, with the counter in the * range counter_start…counter_start+n-1. */ - output = xallocate_pool(n * HASH_VALUE_SIZE); + output = xmalloc_multiply(HASH_VALUE_SIZE, n); for (UINTN i = 0; i < n; i++) hash_once(old_seed, rng, size, @@ -274,7 +274,7 @@ EFI_STATUS process_random_seed(EFI_FILE *root_dir, RandomSeedMode mode) { if (size > RANDOM_MAX_SIZE_MAX) return log_error_status_stall(EFI_INVALID_PARAMETER, L"Random seed file is too large."); - seed = xallocate_pool(size); + seed = xmalloc(size); rsize = size; err = handle->Read(handle, &rsize, seed); diff --git a/src/boot/efi/stub.c b/src/boot/efi/stub.c index f335d3782d..9fe521efd4 100644 --- a/src/boot/efi/stub.c +++ b/src/boot/efi/stub.c @@ -214,7 +214,7 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { if ((!secure_boot_enabled() || cmdline_len == 0) && loaded_image->LoadOptionsSize > 0 && *(CHAR16 *) loaded_image->LoadOptions > 0x1F) { cmdline_len = (loaded_image->LoadOptionsSize / sizeof(CHAR16)) * sizeof(CHAR8); - cmdline = cmdline_owned = xallocate_pool(cmdline_len); + cmdline = cmdline_owned = xmalloc(cmdline_len); for (UINTN i = 0; i < cmdline_len; i++) cmdline[i] = ((CHAR16 *) loaded_image->LoadOptions)[i]; diff --git a/src/boot/efi/util.c b/src/boot/efi/util.c index 7d607e04c7..69bca420ae 100644 --- a/src/boot/efi/util.c +++ b/src/boot/efi/util.c @@ -115,7 +115,7 @@ EFI_STATUS efivar_get(const EFI_GUID *vendor, const CHAR16 *name, CHAR16 **value } /* Make sure a terminating NUL is available at the end */ - val = xallocate_pool(size + sizeof(CHAR16)); + val = xmalloc(size + sizeof(CHAR16)); memcpy(val, buf, size); val[size / sizeof(CHAR16) - 1] = 0; /* NUL terminate */ @@ -189,7 +189,7 @@ EFI_STATUS efivar_get_raw(const EFI_GUID *vendor, const CHAR16 *name, CHAR8 **bu assert(name); l = sizeof(CHAR16 *) * EFI_MAXIMUM_VARIABLE_SIZE; - buf = xallocate_pool(l); + buf = xmalloc(l); err = RT->GetVariable((CHAR16 *) name, (EFI_GUID *) vendor, NULL, &l, buf); if (!EFI_ERROR(err)) { @@ -391,7 +391,7 @@ EFI_STATUS file_read(EFI_FILE *dir, const CHAR16 *name, UINTN off, UINTN size, C /* Allocate some extra bytes to guarantee the result is NUL-terminated for CHAR8 and CHAR16 strings. */ UINTN extra = size % sizeof(CHAR16) + sizeof(CHAR16); - buf = xallocate_pool(size + extra); + buf = xmalloc(size + extra); err = handle->Read(handle, &size, buf); if (EFI_ERROR(err)) return err; @@ -486,11 +486,11 @@ EFI_STATUS get_file_info_harder( /* A lot like LibFileInfo() but with useful error propagation */ - fi = xallocate_pool(size); + fi = xmalloc(size); err = handle->GetInfo(handle, &GenericFileInfo, &size, fi); if (err == EFI_BUFFER_TOO_SMALL) { - FreePool(fi); - fi = xallocate_pool(size); /* GetInfo tells us the required size, let's use that now */ + free(fi); + fi = xmalloc(size); /* GetInfo tells us the required size, let's use that now */ err = handle->GetInfo(handle, &GenericFileInfo, &size, fi); } @@ -527,15 +527,15 @@ EFI_STATUS readdir_harder( * file name length. * As a side effect, most readdir_harder() calls will now be slightly faster. */ sz = sizeof(EFI_FILE_INFO) + 256 * sizeof(CHAR16); - *buffer = xallocate_pool(sz); + *buffer = xmalloc(sz); *buffer_size = sz; } else sz = *buffer_size; err = handle->Read(handle, &sz, *buffer); if (err == EFI_BUFFER_TOO_SMALL) { - FreePool(*buffer); - *buffer = xallocate_pool(sz); + free(*buffer); + *buffer = xmalloc(sz); *buffer_size = sz; err = handle->Read(handle, &sz, *buffer); } @@ -544,7 +544,7 @@ EFI_STATUS readdir_harder( if (sz == 0) { /* End of directory */ - FreePool(*buffer); + free(*buffer); *buffer = NULL; *buffer_size = 0; } @@ -568,9 +568,9 @@ CHAR16 **strv_free(CHAR16 **v) { return NULL; for (CHAR16 **i = v; *i; i++) - FreePool(*i); + free(*i); - FreePool(v); + free(v); return NULL; } diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h index 2245c21ce7..8e28f5b1be 100644 --- a/src/boot/efi/util.h +++ b/src/boot/efi/util.h @@ -72,19 +72,8 @@ static inline void *xrealloc(void *p, size_t old_size, size_t new_size) { return r; } -#define xnew_alloc(type, n, alloc) \ - ({ \ - UINTN _alloc_size; \ - assert_se(!__builtin_mul_overflow(sizeof(type), (n), &_alloc_size)); \ - (type *) alloc(_alloc_size); \ - }) - -#define xallocate_pool(size) ASSERT_SE_PTR(AllocatePool(size)) -#define xallocate_zero_pool(size) ASSERT_SE_PTR(AllocateZeroPool(size)) -#define xreallocate_pool(p, old_size, new_size) ASSERT_SE_PTR(ReallocatePool((p), (old_size), (new_size))) #define xpool_print(fmt, ...) ((CHAR16 *) ASSERT_SE_PTR(PoolPrint((fmt), ##__VA_ARGS__))) -#define xnew(type, n) xnew_alloc(type, (n), xallocate_pool) -#define xnew0(type, n) xnew_alloc(type, (n), xallocate_zero_pool) +#define xnew(type, n) ((type *) xmalloc_multiply(sizeof(type), (n))) EFI_STATUS parse_boolean(const CHAR8 *v, BOOLEAN *b); diff --git a/src/boot/efi/xbootldr.c b/src/boot/efi/xbootldr.c index 84e443135c..ade2e7f3d3 100644 --- a/src/boot/efi/xbootldr.c +++ b/src/boot/efi/xbootldr.c @@ -17,7 +17,7 @@ static EFI_DEVICE_PATH *path_chop(EFI_DEVICE_PATH *path, EFI_DEVICE_PATH *node) assert(node); UINTN len = (UINT8 *) node - (UINT8 *) path; - EFI_DEVICE_PATH *chopped = xallocate_pool(len + END_DEVICE_PATH_LENGTH); + EFI_DEVICE_PATH *chopped = xmalloc(len + END_DEVICE_PATH_LENGTH); memcpy(chopped, path, len); SetDevicePathEndNode((EFI_DEVICE_PATH *) ((UINT8 *) chopped + len)); @@ -101,7 +101,7 @@ static EFI_STATUS try_gpt( /* Now load the GPT entry table */ size = ALIGN_TO((UINTN) gpt.gpt_header.SizeOfPartitionEntry * (UINTN) gpt.gpt_header.NumberOfPartitionEntries, 512); - entries = xallocate_pool(size); + entries = xmalloc(size); err = block_io->ReadBlocks( block_io, From 79a2b916a0dcfe37617caa1010a9f3778cd2fad5 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Sun, 29 May 2022 10:26:18 +0200 Subject: [PATCH 09/13] boot: Drop use of FileDevicePath --- src/boot/efi/boot.c | 6 +++--- src/boot/efi/drivers.c | 6 +++--- src/boot/efi/util.c | 37 +++++++++++++++++++++++++++++++++++++ src/boot/efi/util.h | 1 + 4 files changed, 44 insertions(+), 6 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index f00f004eaa..5287eabd6c 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -2349,9 +2349,9 @@ static EFI_STATUS image_start( if (err != EFI_SUCCESS) return log_error_status_stall(err, L"Error opening root path: %r", err); - path = FileDevicePath(entry->device, entry->loader); - if (!path) - return log_error_status_stall(EFI_INVALID_PARAMETER, L"Error getting device path."); + err = make_file_device_path(entry->device, entry->loader, &path); + if (err != EFI_SUCCESS) + return log_error_status_stall(err, L"Error making file device path: %r", err); UINTN initrd_size = 0; _cleanup_freepool_ void *initrd = NULL; diff --git a/src/boot/efi/drivers.c b/src/boot/efi/drivers.c index 65c74c461d..615c4ca1f3 100644 --- a/src/boot/efi/drivers.c +++ b/src/boot/efi/drivers.c @@ -21,9 +21,9 @@ static EFI_STATUS load_one_driver( assert(fname); spath = xpool_print(L"\\EFI\\systemd\\drivers\\%s", fname); - path = FileDevicePath(loaded_image->DeviceHandle, spath); - if (!path) - return log_oom(); + err = make_file_device_path(loaded_image->DeviceHandle, spath, &path); + if (err != EFI_SUCCESS) + return log_error_status_stall(err, L"Error making file device path: %r", err); err = BS->LoadImage(FALSE, parent_image, path, NULL, 0, &image); if (EFI_ERROR(err)) diff --git a/src/boot/efi/util.c b/src/boot/efi/util.c index 69bca420ae..c6324035ad 100644 --- a/src/boot/efi/util.c +++ b/src/boot/efi/util.c @@ -701,3 +701,40 @@ EFI_STATUS open_volume(EFI_HANDLE device, EFI_FILE **ret_file) { *ret_file = file; return EFI_SUCCESS; } + +EFI_STATUS make_file_device_path(EFI_HANDLE device, const char16_t *file, EFI_DEVICE_PATH **ret_dp) { + EFI_STATUS err; + EFI_DEVICE_PATH *dp; + + assert(file); + assert(ret_dp); + + err = BS->HandleProtocol(device, &DevicePathProtocol, (void **) &dp); + if (err != EFI_SUCCESS) + return err; + + EFI_DEVICE_PATH *end_node = dp; + while (!IsDevicePathEnd(end_node)) + end_node = NextDevicePathNode(end_node); + + size_t file_size = strsize16(file); + size_t dp_size = ((uint8_t *) end_node - (uint8_t *) dp) + END_DEVICE_PATH_LENGTH; + + /* Make a copy that can also hold a file media device path. */ + *ret_dp = xmalloc(dp_size + file_size + SIZE_OF_FILEPATH_DEVICE_PATH); + memcpy(*ret_dp, dp, dp_size); + + /* Point dp to the end node of the copied device path. */ + dp = (EFI_DEVICE_PATH *) ((uint8_t *) *ret_dp + dp_size - END_DEVICE_PATH_LENGTH); + + /* Replace end node with file media device path. */ + FILEPATH_DEVICE_PATH *file_dp = (FILEPATH_DEVICE_PATH *) dp; + file_dp->Header.Type = MEDIA_DEVICE_PATH; + file_dp->Header.SubType = MEDIA_FILEPATH_DP; + memcpy(&file_dp->PathName, file, file_size); + SetDevicePathNodeLength(&file_dp->Header, SIZE_OF_FILEPATH_DEVICE_PATH + file_size); + + dp = NextDevicePathNode(dp); + SetDevicePathEndNode(dp); + return EFI_SUCCESS; +} diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h index 8e28f5b1be..e191fe5866 100644 --- a/src/boot/efi/util.h +++ b/src/boot/efi/util.h @@ -187,3 +187,4 @@ static inline void beep(UINTN beep_count) {} #endif EFI_STATUS open_volume(EFI_HANDLE device, EFI_FILE **ret_file); +EFI_STATUS make_file_device_path(EFI_HANDLE device, const char16_t *file, EFI_DEVICE_PATH **ret_dp); From b04f818417866d2781bd0be7820b506232ecfc2c Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Sun, 29 May 2022 10:33:42 +0200 Subject: [PATCH 10/13] boot: Drop use of UnpackDevicePath Device paths are a packed data structure and the UEFI spec is clear that members may be misaligned. In this case all accesses are aligned except for the signature. We can simply memcpy it instead of making a whole (aligned) copy of the device path. --- src/boot/efi/disk.c | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/boot/efi/disk.c b/src/boot/efi/disk.c index fcd5812ff1..bf2984ed1f 100644 --- a/src/boot/efi/disk.c +++ b/src/boot/efi/disk.c @@ -8,31 +8,32 @@ EFI_STATUS disk_get_part_uuid(EFI_HANDLE *handle, CHAR16 uuid[static 37]) { EFI_STATUS err; - EFI_DEVICE_PATH *device_path; - _cleanup_freepool_ EFI_DEVICE_PATH *paths = NULL; + EFI_DEVICE_PATH *dp; /* export the device path this image is started from */ if (!handle) return EFI_NOT_FOUND; - err = BS->HandleProtocol(handle, &DevicePathProtocol, (void **) &device_path); + err = BS->HandleProtocol(handle, &DevicePathProtocol, (void **) &dp); if (err != EFI_SUCCESS) return err; - paths = UnpackDevicePath(device_path); - for (EFI_DEVICE_PATH *path = paths; !IsDevicePathEnd(path); path = NextDevicePathNode(path)) { - HARDDRIVE_DEVICE_PATH *drive; - - if (DevicePathType(path) != MEDIA_DEVICE_PATH) + for (; !IsDevicePathEnd(dp); dp = NextDevicePathNode(dp)) { + if (DevicePathType(dp) != MEDIA_DEVICE_PATH) continue; - if (DevicePathSubType(path) != MEDIA_HARDDRIVE_DP) - continue; - drive = (HARDDRIVE_DEVICE_PATH *)path; - if (drive->SignatureType != SIGNATURE_TYPE_GUID) + if (DevicePathSubType(dp) != MEDIA_HARDDRIVE_DP) continue; - GuidToString(uuid, (EFI_GUID *)&drive->Signature); + HARDDRIVE_DEVICE_PATH *hd = (HARDDRIVE_DEVICE_PATH *) dp; + if (hd->SignatureType != SIGNATURE_TYPE_GUID) + continue; + + /* Use memcpy in case the device path node is misaligned. */ + EFI_GUID sig; + memcpy(&sig, hd->Signature, sizeof(hd->Signature)); + + GuidToString(uuid, &sig); return EFI_SUCCESS; } From b05d69ed720590d01e7886b3ac2f6ff0cbfaa921 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Sun, 29 May 2022 10:38:19 +0200 Subject: [PATCH 11/13] boot: Drop use of LibLocateHandle --- src/boot/efi/boot.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index 5287eabd6c..3833ae9461 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -1941,8 +1941,8 @@ static void config_entry_add_osx(Config *config) { if (!config->auto_entries) return; - err = LibLocateHandle(ByProtocol, &FileSystemProtocol, NULL, &n_handles, &handles); - if (EFI_ERROR(err)) + err = BS->LocateHandleBuffer(ByProtocol, &FileSystemProtocol, NULL, &n_handles, &handles); + if (err != EFI_SUCCESS) return; for (UINTN i = 0; i < n_handles; i++) { From e17fd5538fb176a46f7589031da06b902573f64a Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Sun, 29 May 2022 11:03:43 +0200 Subject: [PATCH 12/13] boot: Don't copy device path BS->LocateDevicePath only advances the passed device path pointer. It does not actually modify it, so there is no need to make a copy. --- src/boot/efi/shim.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/boot/efi/shim.c b/src/boot/efi/shim.c index 663eafbae4..20db89817a 100644 --- a/src/boot/efi/shim.c +++ b/src/boot/efi/shim.c @@ -102,7 +102,6 @@ static EFIAPI EFI_STATUS security2_policy_authentication (const EFI_SECURITY2_PR static EFIAPI EFI_STATUS security_policy_authentication (const EFI_SECURITY_PROTOCOL *this, UINT32 authentication_status, const EFI_DEVICE_PATH_PROTOCOL *device_path_const) { EFI_STATUS status; - _cleanup_freepool_ EFI_DEVICE_PATH *dev_path = NULL; _cleanup_freepool_ CHAR16 *dev_path_str = NULL; EFI_HANDLE h; _cleanup_freepool_ CHAR8 *file_buffer = NULL; @@ -113,11 +112,7 @@ static EFIAPI EFI_STATUS security_policy_authentication (const EFI_SECURITY_PROT if (!device_path_const) return EFI_INVALID_PARAMETER; - dev_path = DuplicateDevicePath((EFI_DEVICE_PATH*) device_path_const); - if (!dev_path) - return EFI_OUT_OF_RESOURCES; - - EFI_DEVICE_PATH *dp = dev_path; + EFI_DEVICE_PATH *dp = (EFI_DEVICE_PATH *) device_path_const; status = BS->LocateDevicePath(&FileSystemProtocol, &dp, &h); if (EFI_ERROR(status)) return status; From 55233913c808ecc963eb5edf1062871bdc557b48 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Sun, 29 May 2022 11:08:40 +0200 Subject: [PATCH 13/13] boot: Drop use of DuplicateDevicePath --- src/boot/efi/xbootldr.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/boot/efi/xbootldr.c b/src/boot/efi/xbootldr.c index ade2e7f3d3..6e60e4b45d 100644 --- a/src/boot/efi/xbootldr.c +++ b/src/boot/efi/xbootldr.c @@ -25,6 +25,22 @@ static EFI_DEVICE_PATH *path_chop(EFI_DEVICE_PATH *path, EFI_DEVICE_PATH *node) return chopped; } +static EFI_DEVICE_PATH *path_dup(const EFI_DEVICE_PATH *dp) { + assert(dp); + + const EFI_DEVICE_PATH *node = dp; + size_t size = 0; + while (!IsDevicePathEnd(node)) { + size += DevicePathNodeLength(node); + node = NextDevicePathNode(node); + } + size += DevicePathNodeLength(node); + + EFI_DEVICE_PATH *dup = xmalloc(size); + memcpy(dup, dp, size); + return dup; +} + static BOOLEAN verify_gpt(union GptHeaderBuffer *gpt_header_buffer, EFI_LBA lba_expected) { EFI_PARTITION_TABLE_HEADER *h; UINT32 crc32, crc32_saved; @@ -227,7 +243,7 @@ static EFI_STATUS find_device(EFI_HANDLE *device, EFI_DEVICE_PATH **ret_device_p } /* Patch in the data we found */ - EFI_DEVICE_PATH *xboot_path = ASSERT_SE_PTR(DuplicateDevicePath(partition_path)); + EFI_DEVICE_PATH *xboot_path = path_dup(partition_path); memcpy((UINT8 *) xboot_path + ((UINT8 *) part_node - (UINT8 *) partition_path), &hd, sizeof(hd)); *ret_device_path = xboot_path; return EFI_SUCCESS;