From 8ad7deffa95d33b5849ad6589dd52ab12e645edc Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Tue, 22 Nov 2022 15:55:07 +0100 Subject: [PATCH 1/5] boot: Add xstrn8_to_16 --- src/boot/efi/efi-string.c | 78 +++++++++++++++++++++++++++++++++- src/boot/efi/efi-string.h | 5 +++ src/boot/efi/test-efi-string.c | 27 ++++++++++++ 3 files changed, 109 insertions(+), 1 deletion(-) diff --git a/src/boot/efi/efi-string.c b/src/boot/efi/efi-string.c index b877c6f224..2ba15673c9 100644 --- a/src/boot/efi/efi-string.c +++ b/src/boot/efi/efi-string.c @@ -9,7 +9,8 @@ # include "util.h" #else # include -# include "macro.h" +# include "alloc-util.h" +# define xnew(t, n) ASSERT_SE_PTR(new(t, n)) # define xmalloc(n) ASSERT_SE_PTR(malloc(n)) #endif @@ -138,6 +139,81 @@ DEFINE_STRCHR(char16_t, strchr16); DEFINE_STRNDUP(char, xstrndup8, strnlen8); DEFINE_STRNDUP(char16_t, xstrndup16, strnlen16); +static unsigned utf8_to_unichar(const char *utf8, size_t n, char32_t *c) { + char32_t unichar; + unsigned len; + + assert(utf8); + assert(c); + + if (!(utf8[0] & 0x80)) { + *c = utf8[0]; + return 1; + } else if ((utf8[0] & 0xe0) == 0xc0) { + len = 2; + unichar = utf8[0] & 0x1f; + } else if ((utf8[0] & 0xf0) == 0xe0) { + len = 3; + unichar = utf8[0] & 0x0f; + } else if ((utf8[0] & 0xf8) == 0xf0) { + len = 4; + unichar = utf8[0] & 0x07; + } else if ((utf8[0] & 0xfc) == 0xf8) { + len = 5; + unichar = utf8[0] & 0x03; + } else if ((utf8[0] & 0xfe) == 0xfc) { + len = 6; + unichar = utf8[0] & 0x01; + } else { + *c = UINT32_MAX; + return 1; + } + + if (len > n) { + *c = UINT32_MAX; + return len; + } + + for (unsigned i = 1; i < len; i++) { + if ((utf8[i] & 0xc0) != 0x80) { + *c = UINT32_MAX; + return len; + } + unichar <<= 6; + unichar |= utf8[i] & 0x3f; + } + + *c = unichar; + return len; +} + +/* Convert UTF-8 to UCS-2, skipping any invalid or short byte sequences. */ +char16_t *xstrn8_to_16(const char *str8, size_t n) { + if (!str8 || n == 0) + return NULL; + + size_t i = 0; + char16_t *str16 = xnew(char16_t, n + 1); + + while (n > 0 && *str8 != '\0') { + char32_t unichar; + + size_t utf8len = utf8_to_unichar(str8, n, &unichar); + str8 += utf8len; + n = LESS_BY(n, utf8len); + + switch (unichar) { + case 0 ... 0xd7ffU: + case 0xe000U ... 0xffffU: + str16[i++] = unichar; + break; + } + } + + str16[i] = '\0'; + return str16; +} + static bool efi_fnmatch_prefix(const char16_t *p, const char16_t *h, const char16_t **ret_p, const char16_t **ret_h) { assert(p); assert(h); diff --git a/src/boot/efi/efi-string.h b/src/boot/efi/efi-string.h index d4d76a7c18..e12add0b19 100644 --- a/src/boot/efi/efi-string.h +++ b/src/boot/efi/efi-string.h @@ -99,6 +99,11 @@ static inline char16_t *xstrdup16(const char16_t *s) { return xstrndup16(s, SIZE_MAX); } +char16_t *xstrn8_to_16(const char *str8, size_t n); +static inline char16_t *xstr8_to_16(const char *str8) { + return xstrn8_to_16(str8, strlen8(str8)); +} + bool efi_fnmatch(const char16_t *pattern, const char16_t *haystack); bool parse_number8(const char *s, uint64_t *ret_u, const char **ret_tail); diff --git a/src/boot/efi/test-efi-string.c b/src/boot/efi/test-efi-string.c index 2b2359fe5c..7b43e1d629 100644 --- a/src/boot/efi/test-efi-string.c +++ b/src/boot/efi/test-efi-string.c @@ -324,6 +324,33 @@ TEST(xstrdup16) { free(s); } +TEST(xstrn8_to_16) { + char16_t *s = NULL; + + assert_se(xstrn8_to_16(NULL, 1) == NULL); + assert_se(xstrn8_to_16("a", 0) == NULL); + + assert_se(s = xstrn8_to_16("", 1)); + assert_se(streq16(s, u"")); + free(s); + + assert_se(s = xstrn8_to_16("1", 1)); + assert_se(streq16(s, u"1")); + free(s); + + assert_se(s = xstr8_to_16("abcxyzABCXYZ09 .,-_#*!\"§$%&/()=?`~")); + assert_se(streq16(s, u"abcxyzABCXYZ09 .,-_#*!\"§$%&/()=?`~")); + free(s); + + assert_se(s = xstr8_to_16("ÿⱿ𝇉 😺")); + assert_se(streq16(s, u"ÿⱿ ")); + free(s); + + assert_se(s = xstrn8_to_16("¶¶", 3)); + assert_se(streq16(s, u"¶")); + free(s); +} + #define TEST_FNMATCH_ONE(pattern, haystack, expect) \ ({ \ assert_se(fnmatch(pattern, haystack, 0) == (expect ? 0 : FNM_NOMATCH)); \ From aee515bbb58496272a6d975858aa26a355c4fb90 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Tue, 22 Nov 2022 16:03:03 +0100 Subject: [PATCH 2/5] boot: Use xstr8_to_16 --- src/boot/efi/boot.c | 30 +++++++++++++++--------------- src/boot/efi/linux.c | 2 +- src/boot/efi/measure.c | 2 +- src/boot/efi/util.c | 30 ------------------------------ src/boot/efi/util.h | 1 - 5 files changed, 17 insertions(+), 48 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index caa2a69a6f..a13c7edac1 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -1204,7 +1204,7 @@ static void config_defaults_load_from_file(Config *config, char *content) { continue; } free(config->entry_default_config); - config->entry_default_config = xstra_to_str(value); + config->entry_default_config = xstr8_to_16(value); continue; } @@ -1418,25 +1418,25 @@ static void config_entry_add_type1( while ((line = line_get_key_value(content, " \t", &pos, &key, &value))) { if (streq8(key, "title")) { free(entry->title); - entry->title = xstra_to_str(value); + entry->title = xstr8_to_16(value); continue; } if (streq8(key, "sort-key")) { free(entry->sort_key); - entry->sort_key = xstra_to_str(value); + entry->sort_key = xstr8_to_16(value); continue; } if (streq8(key, "version")) { free(entry->version); - entry->version = xstra_to_str(value); + entry->version = xstr8_to_16(value); continue; } if (streq8(key, "machine-id")) { free(entry->machine_id); - entry->machine_id = xstra_to_str(value); + entry->machine_id = xstr8_to_16(value); continue; } @@ -1489,7 +1489,7 @@ static void config_entry_add_type1( if (streq8(key, "options")) { _cleanup_free_ char16_t *new = NULL; - new = xstra_to_str(value); + new = xstr8_to_16(value); if (entry->options) { char16_t *s = xpool_print(L"%s %s", entry->options, new); free(entry->options); @@ -2134,49 +2134,49 @@ static void config_entry_add_unified( while ((line = line_get_key_value(content, "=", &pos, &key, &value))) { if (streq8(key, "PRETTY_NAME")) { free(os_pretty_name); - os_pretty_name = xstra_to_str(value); + os_pretty_name = xstr8_to_16(value); continue; } if (streq8(key, "IMAGE_ID")) { free(os_image_id); - os_image_id = xstra_to_str(value); + os_image_id = xstr8_to_16(value); continue; } if (streq8(key, "NAME")) { free(os_name); - os_name = xstra_to_str(value); + os_name = xstr8_to_16(value); continue; } if (streq8(key, "ID")) { free(os_id); - os_id = xstra_to_str(value); + os_id = xstr8_to_16(value); continue; } if (streq8(key, "IMAGE_VERSION")) { free(os_image_version); - os_image_version = xstra_to_str(value); + os_image_version = xstr8_to_16(value); continue; } if (streq8(key, "VERSION")) { free(os_version); - os_version = xstra_to_str(value); + os_version = xstr8_to_16(value); continue; } if (streq8(key, "VERSION_ID")) { free(os_version_id); - os_version_id = xstra_to_str(value); + os_version_id = xstr8_to_16(value); continue; } if (streq8(key, "BUILD_ID")) { free(os_build_id); - os_build_id = xstra_to_str(value); + os_build_id = xstr8_to_16(value); continue; } } @@ -2225,7 +2225,7 @@ static void config_entry_add_unified( if (content[szs[SECTION_CMDLINE] - 1] == '\n') content[szs[SECTION_CMDLINE] - 1] = '\0'; - entry->options = xstra_to_str(content); + entry->options = xstr8_to_16(content); } } } diff --git a/src/boot/efi/linux.c b/src/boot/efi/linux.c index dd7eb48c8c..668510fca3 100644 --- a/src/boot/efi/linux.c +++ b/src/boot/efi/linux.c @@ -133,7 +133,7 @@ EFI_STATUS linux_exec( return log_error_status_stall(err, u"Error getting kernel loaded image protocol: %r", err); if (cmdline) { - loaded_image->LoadOptions = xstra_to_str(cmdline); + loaded_image->LoadOptions = xstrn8_to_16(cmdline, cmdline_len); loaded_image->LoadOptionsSize = strsize16(loaded_image->LoadOptions); } diff --git a/src/boot/efi/measure.c b/src/boot/efi/measure.c index 9a16920787..6da07d917e 100644 --- a/src/boot/efi/measure.c +++ b/src/boot/efi/measure.c @@ -187,7 +187,7 @@ EFI_STATUS tpm_log_event_ascii(uint32_t pcrindex, EFI_PHYSICAL_ADDRESS buffer, U _cleanup_free_ char16_t *c = NULL; if (description) - c = xstra_to_str(description); + c = xstr8_to_16(description); return tpm_log_event(pcrindex, buffer, buffer_size, c, ret_measured); } diff --git a/src/boot/efi/util.c b/src/boot/efi/util.c index 57436dbf0c..676204ce01 100644 --- a/src/boot/efi/util.c +++ b/src/boot/efi/util.c @@ -298,36 +298,6 @@ static int utf8_to_16(const char *stra, char16_t *c) { return len; } -char16_t *xstra_to_str(const char *stra) { - UINTN strlen; - UINTN len; - UINTN i; - char16_t *str; - - assert(stra); - - len = strlen8(stra); - str = xnew(char16_t, len + 1); - - strlen = 0; - i = 0; - while (i < len) { - int utf8len; - - utf8len = utf8_to_16(stra + i, str + strlen); - if (utf8len <= 0) { - /* invalid utf8 sequence, skip the garbage */ - i++; - continue; - } - - strlen++; - i += utf8len; - } - str[strlen] = '\0'; - return str; -} - char16_t *xstra_to_path(const char *stra) { char16_t *str; UINTN strlen; diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h index 4c5b6cab13..b91d713272 100644 --- a/src/boot/efi/util.h +++ b/src/boot/efi/util.h @@ -138,7 +138,6 @@ EFI_STATUS efivar_get_uint64_le(const EFI_GUID *vendor, const char16_t *name, ui EFI_STATUS efivar_get_boolean_u8(const EFI_GUID *vendor, const char16_t *name, bool *ret); char16_t *xstra_to_path(const char *stra); -char16_t *xstra_to_str(const char *stra); EFI_STATUS file_read(EFI_FILE *dir, const char16_t *name, UINTN off, UINTN size, char **content, UINTN *content_size); From 7444e10611671abac35be3ab9fe9697cd4c90d62 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Tue, 22 Nov 2022 16:30:44 +0100 Subject: [PATCH 3/5] boot: Use xstr8_to_16 for path conversion --- src/boot/efi/boot.c | 8 ++-- src/boot/efi/cpio.c | 19 +-------- src/boot/efi/util.c | 102 ++++++++------------------------------------ src/boot/efi/util.h | 3 +- 4 files changed, 24 insertions(+), 108 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index a13c7edac1..a72e6e62fd 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -1443,7 +1443,7 @@ static void config_entry_add_type1( if (streq8(key, "linux")) { free(entry->loader); entry->type = LOADER_LINUX; - entry->loader = xstra_to_path(value); + entry->loader = xstr8_to_path(value); entry->key = 'l'; continue; } @@ -1451,7 +1451,7 @@ static void config_entry_add_type1( if (streq8(key, "efi")) { entry->type = LOADER_EFI; free(entry->loader); - entry->loader = xstra_to_path(value); + entry->loader = xstr8_to_path(value); /* do not add an entry for ourselves */ if (strcaseeq16(entry->loader, loaded_image_path)) { @@ -1472,7 +1472,7 @@ static void config_entry_add_type1( if (streq8(key, "devicetree")) { free(entry->devicetree); - entry->devicetree = xstra_to_path(value); + entry->devicetree = xstr8_to_path(value); continue; } @@ -1481,7 +1481,7 @@ static void config_entry_add_type1( entry->initrd, n_initrd == 0 ? 0 : (n_initrd + 1) * sizeof(uint16_t *), (n_initrd + 2) * sizeof(uint16_t *)); - entry->initrd[n_initrd++] = xstra_to_path(value); + entry->initrd[n_initrd++] = xstr8_to_path(value); entry->initrd[n_initrd] = NULL; continue; } diff --git a/src/boot/efi/cpio.c b/src/boot/efi/cpio.c index 648f9f000f..76e2cd7f4e 100644 --- a/src/boot/efi/cpio.c +++ b/src/boot/efi/cpio.c @@ -359,24 +359,7 @@ static char16_t *get_dropin_dir(const EFI_DEVICE_PATH *file_path) { if (device_path_to_str(file_path, &file_path_str) != EFI_SUCCESS) return NULL; - for (char16_t *i = file_path_str, *fixed = i;; i++) { - if (*i == '\0') { - *fixed = '\0'; - break; - } - - /* Fix device path node separator. */ - if (*i == '/') - *i = '\\'; - - /* Double '\' is not allowed in EFI file paths. */ - if (fixed != file_path_str && fixed[-1] == '\\' && *i == '\\') - continue; - - *fixed = *i; - fixed++; - } - + convert_efi_path(file_path_str); return xpool_print(u"%s.extra.d", file_path_str); } diff --git a/src/boot/efi/util.c b/src/boot/efi/util.c index 676204ce01..9c22032a0c 100644 --- a/src/boot/efi/util.c +++ b/src/boot/efi/util.c @@ -244,97 +244,29 @@ void efivar_set_time_usec(const EFI_GUID *vendor, const char16_t *name, uint64_t efivar_set(vendor, name, str, 0); } -static int utf8_to_16(const char *stra, char16_t *c) { - char16_t unichar; - UINTN len; +void convert_efi_path(char16_t *path) { + assert(path); - assert(stra); - assert(c); + for (size_t i = 0, fixed = 0;; i++) { + /* Fix device path node separator. */ + path[fixed] = (path[i] == '/') ? '\\' : path[i]; - if (!(stra[0] & 0x80)) - len = 1; - else if ((stra[0] & 0xe0) == 0xc0) - len = 2; - else if ((stra[0] & 0xf0) == 0xe0) - len = 3; - else if ((stra[0] & 0xf8) == 0xf0) - len = 4; - else if ((stra[0] & 0xfc) == 0xf8) - len = 5; - else if ((stra[0] & 0xfe) == 0xfc) - len = 6; - else - return -1; + /* Double '\' is not allowed in EFI file paths. */ + if (fixed > 0 && path[fixed - 1] == '\\' && path[fixed] == '\\') + continue; - switch (len) { - case 1: - unichar = stra[0]; - break; - case 2: - unichar = stra[0] & 0x1f; - break; - case 3: - unichar = stra[0] & 0x0f; - break; - case 4: - unichar = stra[0] & 0x07; - break; - case 5: - unichar = stra[0] & 0x03; - break; - case 6: - unichar = stra[0] & 0x01; - break; + if (path[i] == '\0') + break; + + fixed++; } - - for (UINTN i = 1; i < len; i++) { - if ((stra[i] & 0xc0) != 0x80) - return -1; - unichar <<= 6; - unichar |= stra[i] & 0x3f; - } - - *c = unichar; - return len; } -char16_t *xstra_to_path(const char *stra) { - char16_t *str; - UINTN strlen; - UINTN len; - UINTN i; - - assert(stra); - - len = strlen8(stra); - str = xnew(char16_t, len + 2); - - str[0] = '\\'; - strlen = 1; - i = 0; - while (i < len) { - int utf8len; - - utf8len = utf8_to_16(stra + i, str + strlen); - if (utf8len <= 0) { - /* invalid utf8 sequence, skip the garbage */ - i++; - continue; - } - - if (str[strlen] == '/') - str[strlen] = '\\'; - if (str[strlen] == '\\' && str[strlen-1] == '\\') { - /* skip double slashes */ - i += utf8len; - continue; - } - - strlen++; - i += utf8len; - } - str[strlen] = '\0'; - return str; +char16_t *xstr8_to_path(const char *str8) { + assert(str8); + char16_t *path = xstr8_to_16(str8); + convert_efi_path(path); + return path; } EFI_STATUS file_read(EFI_FILE *dir, const char16_t *name, UINTN off, UINTN size, char **ret, UINTN *ret_size) { diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h index b91d713272..f688321109 100644 --- a/src/boot/efi/util.h +++ b/src/boot/efi/util.h @@ -137,7 +137,8 @@ EFI_STATUS efivar_get_uint32_le(const EFI_GUID *vendor, const char16_t *name, ui EFI_STATUS efivar_get_uint64_le(const EFI_GUID *vendor, const char16_t *name, uint64_t *ret); EFI_STATUS efivar_get_boolean_u8(const EFI_GUID *vendor, const char16_t *name, bool *ret); -char16_t *xstra_to_path(const char *stra); +void convert_efi_path(char16_t *path); +char16_t *xstr8_to_path(const char *stra); EFI_STATUS file_read(EFI_FILE *dir, const char16_t *name, UINTN off, UINTN size, char **content, UINTN *content_size); From 927ebebe588970fa2dd082a0daaef246229f009b Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Tue, 22 Nov 2022 17:42:38 +0100 Subject: [PATCH 4/5] stub: Fix cmdline handling This fixes some bugs that could lead to garbage getting appended to the command line passed to the kernel: 1. The .cmdline section is not guaranteed to be NUL-terminated, but it was used as if it was. 2. The conversion of the command line to ASCII that was passed to the stub ate the NUL at the end. 3. LoadOptions is not guaranteed to be a NUL-terminated EFI string (it really should be and generally always is, though). This also fixes the inconsistent mangling of the command line. If the .cmdline section was used ASCII controls chars (new lines in particular) would not be converted to spaces. As part of this commit, we optimize conversion for the generic code instead of the (deprecated) EFI handover protocol. Previously we would convert to ASCII/UTF-8 and then back to EFI string for the (now) default generic code path. Instead we now convert to EFI string and mangle that back to ASCII in the EFI handover protocol path. --- src/boot/efi/boot.c | 10 ++++------ src/boot/efi/linux.c | 12 ++++++------ src/boot/efi/linux.h | 17 +++++++++++------ src/boot/efi/linux_x86.c | 21 ++++++++++++++------- src/boot/efi/stub.c | 38 +++++++++++++++++--------------------- src/boot/efi/util.c | 7 +++++++ src/boot/efi/util.h | 1 + 7 files changed, 60 insertions(+), 46 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index a72e6e62fd..95104a2e40 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -2219,13 +2219,11 @@ static void config_entry_add_unified( content = mfree(content); /* read the embedded cmdline file */ - err = file_read(linux_dir, f->FileName, offs[SECTION_CMDLINE], szs[SECTION_CMDLINE], &content, NULL); + size_t cmdline_len; + err = file_read(linux_dir, f->FileName, offs[SECTION_CMDLINE], szs[SECTION_CMDLINE], &content, &cmdline_len); if (err == EFI_SUCCESS) { - /* chomp the newline */ - if (content[szs[SECTION_CMDLINE] - 1] == '\n') - content[szs[SECTION_CMDLINE] - 1] = '\0'; - - entry->options = xstr8_to_16(content); + entry->options = xstrn8_to_16(content, cmdline_len); + mangle_stub_cmdline(entry->options); } } } diff --git a/src/boot/efi/linux.c b/src/boot/efi/linux.c index 668510fca3..48801f9dd8 100644 --- a/src/boot/efi/linux.c +++ b/src/boot/efi/linux.c @@ -93,15 +93,16 @@ static EFI_STATUS load_image(EFI_HANDLE parent, const void *source, size_t len, EFI_STATUS linux_exec( EFI_HANDLE parent, - const char *cmdline, UINTN cmdline_len, - const void *linux_buffer, UINTN linux_length, - const void *initrd_buffer, UINTN initrd_length) { + const char16_t *cmdline, + const void *linux_buffer, + size_t linux_length, + const void *initrd_buffer, + size_t initrd_length) { uint32_t compat_address; EFI_STATUS err; assert(parent); - assert(cmdline || cmdline_len == 0); assert(linux_buffer && linux_length > 0); assert(initrd_buffer || initrd_length == 0); @@ -113,7 +114,6 @@ EFI_STATUS linux_exec( return linux_exec_efi_handover( parent, cmdline, - cmdline_len, linux_buffer, linux_length, initrd_buffer, @@ -133,7 +133,7 @@ EFI_STATUS linux_exec( return log_error_status_stall(err, u"Error getting kernel loaded image protocol: %r", err); if (cmdline) { - loaded_image->LoadOptions = xstrn8_to_16(cmdline, cmdline_len); + loaded_image->LoadOptions = (void *) cmdline; loaded_image->LoadOptionsSize = strsize16(loaded_image->LoadOptions); } diff --git a/src/boot/efi/linux.h b/src/boot/efi/linux.h index 19e5f5c4a8..f0a6a37ed1 100644 --- a/src/boot/efi/linux.h +++ b/src/boot/efi/linux.h @@ -2,14 +2,19 @@ #pragma once #include +#include EFI_STATUS linux_exec( EFI_HANDLE parent, - const char *cmdline, UINTN cmdline_len, - const void *linux_buffer, UINTN linux_length, - const void *initrd_buffer, UINTN initrd_length); + const char16_t *cmdline, + const void *linux_buffer, + size_t linux_length, + const void *initrd_buffer, + size_t initrd_length); EFI_STATUS linux_exec_efi_handover( EFI_HANDLE parent, - const char *cmdline, UINTN cmdline_len, - const void *linux_buffer, UINTN linux_length, - const void *initrd_buffer, UINTN initrd_length); + const char16_t *cmdline, + const void *linux_buffer, + size_t linux_length, + const void *initrd_buffer, + size_t initrd_length); diff --git a/src/boot/efi/linux_x86.c b/src/boot/efi/linux_x86.c index 64336ce348..6a5e431107 100644 --- a/src/boot/efi/linux_x86.c +++ b/src/boot/efi/linux_x86.c @@ -126,12 +126,13 @@ static void linux_efi_handover(EFI_HANDLE parent, uintptr_t kernel, BootParams * EFI_STATUS linux_exec_efi_handover( EFI_HANDLE parent, - const char *cmdline, UINTN cmdline_len, - const void *linux_buffer, UINTN linux_length, - const void *initrd_buffer, UINTN initrd_length) { + const char16_t *cmdline, + const void *linux_buffer, + size_t linux_length, + const void *initrd_buffer, + size_t initrd_length) { assert(parent); - assert(cmdline || cmdline_len == 0); assert(linux_buffer); assert(initrd_buffer || initrd_length == 0); @@ -185,14 +186,20 @@ EFI_STATUS linux_exec_efi_handover( _cleanup_pages_ Pages cmdline_pages = {}; if (cmdline) { + size_t len = MIN(strlen16(cmdline), image_params->hdr.cmdline_size); + cmdline_pages = xmalloc_pages( can_4g ? AllocateAnyPages : AllocateMaxAddress, EfiLoaderData, - EFI_SIZE_TO_PAGES(cmdline_len + 1), + EFI_SIZE_TO_PAGES(len + 1), CMDLINE_PTR_MAX); - memcpy(PHYSICAL_ADDRESS_TO_POINTER(cmdline_pages.addr), cmdline, cmdline_len); - ((char *) PHYSICAL_ADDRESS_TO_POINTER(cmdline_pages.addr))[cmdline_len] = 0; + /* Convert cmdline to ASCII. */ + char *cmdline8 = PHYSICAL_ADDRESS_TO_POINTER(cmdline_pages.addr); + for (size_t i = 0; i < len; i++) + cmdline8[i] = cmdline[i] <= 0x7E ? cmdline[i] : ' '; + cmdline8[len] = '\0'; + boot_params->hdr.cmd_line_ptr = (uint32_t) cmdline_pages.addr; boot_params->ext_cmd_line_ptr = cmdline_pages.addr >> 32; assert(can_4g || cmdline_pages.addr <= CMDLINE_PTR_MAX); diff --git a/src/boot/efi/stub.c b/src/boot/efi/stub.c index a842c5c679..841a0e41bd 100644 --- a/src/boot/efi/stub.c +++ b/src/boot/efi/stub.c @@ -132,14 +132,13 @@ static void export_variables(EFI_LOADED_IMAGE_PROTOCOL *loaded_image) { EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { _cleanup_free_ void *credential_initrd = NULL, *global_credential_initrd = NULL, *sysext_initrd = NULL, *pcrsig_initrd = NULL, *pcrpkey_initrd = NULL; - UINTN credential_initrd_size = 0, global_credential_initrd_size = 0, sysext_initrd_size = 0, pcrsig_initrd_size = 0, pcrpkey_initrd_size = 0; - UINTN cmdline_len = 0, linux_size, initrd_size, dt_size; + size_t credential_initrd_size = 0, global_credential_initrd_size = 0, sysext_initrd_size = 0, pcrsig_initrd_size = 0, pcrpkey_initrd_size = 0; + size_t linux_size, initrd_size, dt_size; EFI_PHYSICAL_ADDRESS linux_base, initrd_base, dt_base; _cleanup_(devicetree_cleanup) struct devicetree_state dt_state = {}; EFI_LOADED_IMAGE_PROTOCOL *loaded_image; - UINTN addrs[_UNIFIED_SECTION_MAX] = {}, szs[_UNIFIED_SECTION_MAX] = {}; - char *cmdline = NULL; - _cleanup_free_ char *cmdline_owned = NULL; + size_t addrs[_UNIFIED_SECTION_MAX] = {}, szs[_UNIFIED_SECTION_MAX] = {}; + _cleanup_free_ char16_t *cmdline = NULL; int sections_measured = -1, parameters_measured = -1; bool sysext_measured = false, m; EFI_STATUS err; @@ -208,32 +207,29 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { /* Show splash screen as early as possible */ graphics_splash((const uint8_t*) loaded_image->ImageBase + addrs[UNIFIED_SECTION_SPLASH], szs[UNIFIED_SECTION_SPLASH]); - if (szs[UNIFIED_SECTION_CMDLINE] > 0) { - cmdline = (char *) loaded_image->ImageBase + addrs[UNIFIED_SECTION_CMDLINE]; - cmdline_len = szs[UNIFIED_SECTION_CMDLINE]; - } - /* if we are not in secure boot mode, or none was provided, accept a custom command line and replace * the built-in one. We also do a superficial check whether first character of passed command line * is printable character (for compat with some Dell systems which fill in garbage?). */ - if ((!secure_boot_enabled() || cmdline_len == 0) && - loaded_image->LoadOptionsSize > 0 && + if ((!secure_boot_enabled() || szs[UNIFIED_SECTION_CMDLINE] == 0) && + loaded_image->LoadOptionsSize > sizeof(char16_t) && ((char16_t *) loaded_image->LoadOptions)[0] > 0x1F) { - cmdline_len = (loaded_image->LoadOptionsSize / sizeof(char16_t)) * sizeof(char); - cmdline = cmdline_owned = xnew(char, cmdline_len); - - for (UINTN i = 0; i < cmdline_len; i++) { - char16_t c = ((char16_t *) loaded_image->LoadOptions)[i]; - cmdline[i] = c > 0x1F && c < 0x7F ? c : ' '; /* convert non-printable and non_ASCII characters to spaces. */ - } + /* Note that LoadOptions is a void*, so it could be anything! */ + cmdline = xstrndup16( + loaded_image->LoadOptions, loaded_image->LoadOptionsSize / sizeof(char16_t)); + mangle_stub_cmdline(cmdline); /* Let's measure the passed kernel command line into the TPM. Note that this possibly * duplicates what we already did in the boot menu, if that was already used. However, since * we want the boot menu to support an EFI binary, and want to this stub to be usable from * any boot menu, let's measure things anyway. */ m = false; - (void) tpm_log_load_options(loaded_image->LoadOptions, &m); + (void) tpm_log_load_options(cmdline, &m); parameters_measured = m; + } else if (szs[UNIFIED_SECTION_CMDLINE] > 0) { + cmdline = xstrn8_to_16( + (char *) loaded_image->ImageBase + addrs[UNIFIED_SECTION_CMDLINE], + szs[UNIFIED_SECTION_CMDLINE]); + mangle_stub_cmdline(cmdline); } export_variables(loaded_image); @@ -374,7 +370,7 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { log_error_stall(L"Error loading embedded devicetree: %r", err); } - err = linux_exec(image, cmdline, cmdline_len, + err = linux_exec(image, cmdline, PHYSICAL_ADDRESS_TO_POINTER(linux_base), linux_size, PHYSICAL_ADDRESS_TO_POINTER(initrd_base), initrd_size); graphics_mode(false); diff --git a/src/boot/efi/util.c b/src/boot/efi/util.c index 9c22032a0c..f9aeeb4833 100644 --- a/src/boot/efi/util.c +++ b/src/boot/efi/util.c @@ -269,6 +269,13 @@ char16_t *xstr8_to_path(const char *str8) { return path; } +void mangle_stub_cmdline(char16_t *cmdline) { + for (; *cmdline != '\0'; cmdline++) + /* Convert ASCII control characters to spaces. */ + if (*cmdline <= 0x1F) + *cmdline = ' '; +} + EFI_STATUS file_read(EFI_FILE *dir, const char16_t *name, UINTN off, UINTN size, char **ret, UINTN *ret_size) { _cleanup_(file_closep) EFI_FILE *handle = NULL; _cleanup_free_ char *buf = NULL; diff --git a/src/boot/efi/util.h b/src/boot/efi/util.h index f688321109..08f732f484 100644 --- a/src/boot/efi/util.h +++ b/src/boot/efi/util.h @@ -139,6 +139,7 @@ EFI_STATUS efivar_get_boolean_u8(const EFI_GUID *vendor, const char16_t *name, b void convert_efi_path(char16_t *path); char16_t *xstr8_to_path(const char *stra); +void mangle_stub_cmdline(char16_t *cmdline); EFI_STATUS file_read(EFI_FILE *dir, const char16_t *name, UINTN off, UINTN size, char **content, UINTN *content_size); From b17f3b3d8077ab6827549a123ac636d655fe8d4d Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Wed, 2 Nov 2022 10:25:32 +0100 Subject: [PATCH 5/5] stub: Detect empty LoadOptions when run from EFI shell The EFI shell will pass the entire command line to the application it starts, which includes the file path of the stub binary. This prevents us from using the built-in cmdline if the command line is otherwise empty. Fortunately, the EFI shell registers a protocol on any images it starts this way. The protocol even lets us access the args individually, making it easy to strip the stub path off. Fixes: #25201 --- src/boot/efi/missing_efi.h | 13 +++++++++ src/boot/efi/stub.c | 59 +++++++++++++++++++++++++++++++------- 2 files changed, 61 insertions(+), 11 deletions(-) diff --git a/src/boot/efi/missing_efi.h b/src/boot/efi/missing_efi.h index f9169248ec..250c84c248 100644 --- a/src/boot/efi/missing_efi.h +++ b/src/boot/efi/missing_efi.h @@ -385,3 +385,16 @@ typedef struct _EFI_CONSOLE_CONTROL_PROTOCOL { { 0xd719b2cb, 0x3d3a, 0x4596, {0xa3, 0xbc, 0xda, 0xd0, 0xe, 0x67, 0x65, 0x6f }} #endif + +#ifndef EFI_SHELL_PARAMETERS_PROTOCOL_GUID +# define EFI_SHELL_PARAMETERS_PROTOCOL_GUID \ + { 0x752f3136, 0x4e16, 0x4fdc, { 0xa2, 0x2a, 0xe5, 0xf4, 0x68, 0x12, 0xf4, 0xca } } + +typedef struct { + CHAR16 **Argv; + UINTN Argc; + void *StdIn; + void *StdOut; + void *StdErr; +} EFI_SHELL_PARAMETERS_PROTOCOL; +#endif diff --git a/src/boot/efi/stub.c b/src/boot/efi/stub.c index 841a0e41bd..7c42a16c70 100644 --- a/src/boot/efi/stub.c +++ b/src/boot/efi/stub.c @@ -130,6 +130,53 @@ static void export_variables(EFI_LOADED_IMAGE_PROTOCOL *loaded_image) { (void) efivar_set_uint64_le(LOADER_GUID, L"StubFeatures", stub_features, 0); } +static bool use_load_options( + EFI_HANDLE stub_image, + EFI_LOADED_IMAGE_PROTOCOL *loaded_image, + bool have_cmdline, + char16_t **ret) { + + assert(stub_image); + assert(loaded_image); + assert(ret); + + /* We only allow custom command lines if we aren't in secure boot or if no cmdline was baked into + * the stub image. */ + if (secure_boot_enabled() && have_cmdline) + return false; + + /* We also do a superficial check whether first character of passed command line + * is printable character (for compat with some Dell systems which fill in garbage?). */ + if (loaded_image->LoadOptionsSize < sizeof(char16_t) || ((char16_t *) loaded_image->LoadOptions)[0] <= 0x1F) + return false; + + /* The UEFI shell registers EFI_SHELL_PARAMETERS_PROTOCOL onto images it runs. This lets us know that + * LoadOptions starts with the stub binary path which we want to strip off. */ + EFI_SHELL_PARAMETERS_PROTOCOL *shell; + if (BS->HandleProtocol(stub_image, &(EFI_GUID) EFI_SHELL_PARAMETERS_PROTOCOL_GUID, (void **) &shell) + != EFI_SUCCESS) { + /* Not running from EFI shell, use entire LoadOptions. Note that LoadOptions is a void*, so + * it could be anything! */ + *ret = xstrndup16(loaded_image->LoadOptions, loaded_image->LoadOptionsSize / sizeof(char16_t)); + mangle_stub_cmdline(*ret); + return true; + } + + if (shell->Argc < 2) + /* No arguments were provided? Then we fall back to built-in cmdline. */ + return false; + + /* Assemble the command line ourselves without our stub path. */ + *ret = xstrdup16(shell->Argv[1]); + for (size_t i = 2; i < shell->Argc; i++) { + _cleanup_free_ char16_t *old = *ret; + *ret = xpool_print(u"%s %s", old, shell->Argv[i]); + } + + mangle_stub_cmdline(*ret); + return true; +} + EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { _cleanup_free_ void *credential_initrd = NULL, *global_credential_initrd = NULL, *sysext_initrd = NULL, *pcrsig_initrd = NULL, *pcrpkey_initrd = NULL; size_t credential_initrd_size = 0, global_credential_initrd_size = 0, sysext_initrd_size = 0, pcrsig_initrd_size = 0, pcrpkey_initrd_size = 0; @@ -207,17 +254,7 @@ EFI_STATUS efi_main(EFI_HANDLE image, EFI_SYSTEM_TABLE *sys_table) { /* Show splash screen as early as possible */ graphics_splash((const uint8_t*) loaded_image->ImageBase + addrs[UNIFIED_SECTION_SPLASH], szs[UNIFIED_SECTION_SPLASH]); - /* if we are not in secure boot mode, or none was provided, accept a custom command line and replace - * the built-in one. We also do a superficial check whether first character of passed command line - * is printable character (for compat with some Dell systems which fill in garbage?). */ - if ((!secure_boot_enabled() || szs[UNIFIED_SECTION_CMDLINE] == 0) && - loaded_image->LoadOptionsSize > sizeof(char16_t) && - ((char16_t *) loaded_image->LoadOptions)[0] > 0x1F) { - /* Note that LoadOptions is a void*, so it could be anything! */ - cmdline = xstrndup16( - loaded_image->LoadOptions, loaded_image->LoadOptionsSize / sizeof(char16_t)); - mangle_stub_cmdline(cmdline); - + if (use_load_options(image, loaded_image, szs[UNIFIED_SECTION_CMDLINE] > 0, &cmdline)) { /* Let's measure the passed kernel command line into the TPM. Note that this possibly * duplicates what we already did in the boot menu, if that was already used. However, since * we want the boot menu to support an EFI binary, and want to this stub to be usable from