From 8f30a066ff48325c9197ae3b103cd446852b9f3d Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Wed, 2 Aug 2023 15:16:33 +0200 Subject: [PATCH 1/3] bless-boot: Actually return successfully $ journalctl -u systemd-bless-boot.service systemd[1]: Starting Mark the Current Boot Loader Entry as Good... systemd-bless-boot[536]: Marked boot as 'good'. (Boot attempt counter is at 2.) systemd-bless-boot[536]: Can't find boot counter source file for '/loader/entries/arch.conf': Device or resource busy systemd[1]: Finished Mark the Current Boot Loader Entry as Good. --- src/boot/bless-boot.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/boot/bless-boot.c b/src/boot/bless-boot.c index 59f02b761ac..0c0b4f23c77 100644 --- a/src/boot/bless-boot.c +++ b/src/boot/bless-boot.c @@ -483,6 +483,7 @@ static int verb_set(int argc, char *argv[], void *userdata) { log_debug_errno(errno, "Failed to synchronize $BOOT partition, ignoring: %m"); log_info("Marked boot as '%s'. (Boot attempt counter is at %" PRIu64".)", argv[0], done); + return 0; } log_error_errno(SYNTHETIC_ERRNO(EBUSY), "Can't find boot counter source file for '%s': %m", target); From 66fd078ba89e90e8aeba6edac52d20456fc2cd5d Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Wed, 2 Aug 2023 16:00:07 +0200 Subject: [PATCH 2/3] boot: Fix boot counting for XBOOTLDR entries We were passing the dir handle for the ESP to config_entry_bump_counters(), which will obviously fail if the entry actually resides on the XBOOTLDR partition. Fixes: #28637 --- src/boot/efi/boot.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c index 1321658a01d..ccba953b412 100644 --- a/src/boot/efi/boot.c +++ b/src/boot/efi/boot.c @@ -1346,7 +1346,7 @@ static void config_entry_parse_tries( suffix); } -static void config_entry_bump_counters(ConfigEntry *entry, EFI_FILE *root_dir) { +static EFI_STATUS config_entry_bump_counters(ConfigEntry *entry) { _cleanup_free_ char16_t* old_path = NULL, *new_path = NULL; _cleanup_(file_closep) EFI_FILE *handle = NULL; _cleanup_free_ EFI_FILE_INFO *file_info = NULL; @@ -1354,34 +1354,39 @@ static void config_entry_bump_counters(ConfigEntry *entry, EFI_FILE *root_dir) { EFI_STATUS err; assert(entry); - assert(root_dir); if (entry->tries_left < 0) - return; + return EFI_SUCCESS; if (!entry->path || !entry->current_name || !entry->next_name) - return; + return EFI_SUCCESS; + + _cleanup_(file_closep) EFI_FILE *root = NULL; + err = open_volume(entry->device, &root); + if (err != EFI_SUCCESS) + return log_error_status(err, "Error opening entry root path: %m"); old_path = xasprintf("%ls\\%ls", entry->path, entry->current_name); - err = root_dir->Open(root_dir, &handle, old_path, EFI_FILE_MODE_READ|EFI_FILE_MODE_WRITE, 0ULL); + err = root->Open(root, &handle, old_path, EFI_FILE_MODE_READ|EFI_FILE_MODE_WRITE, 0ULL); if (err != EFI_SUCCESS) - return; + return log_error_status(err, "Error opening boot entry: %m"); err = get_file_info(handle, &file_info, &file_info_size); if (err != EFI_SUCCESS) - return; + return log_error_status(err, "Error getting boot entry file info: %m"); /* And rename the file */ strcpy16(file_info->FileName, entry->next_name); err = handle->SetInfo(handle, MAKE_GUID_PTR(EFI_FILE_INFO), file_info_size, file_info); - if (err != EFI_SUCCESS) { - log_error_status(err, "Failed to rename '%ls' to '%ls', ignoring: %m", old_path, entry->next_name); - return; - } + if (err != EFI_SUCCESS) + return log_error_status( + err, "Failed to rename '%ls' to '%ls', ignoring: %m", old_path, entry->next_name); /* Flush everything to disk, just in case… */ - (void) handle->Flush(handle); + err = handle->Flush(handle); + if (err != EFI_SUCCESS) + return log_error_status(err, "Error flushing boot entry file info: %m"); /* Let's tell the OS that we renamed this file, so that it knows what to rename to the counter-less name on * success */ @@ -1393,6 +1398,8 @@ static void config_entry_bump_counters(ConfigEntry *entry, EFI_FILE *root_dir) { free(entry->loader); entry->loader = TAKE_PTR(new_path); } + + return EFI_SUCCESS; } static void config_entry_add_type1( @@ -2714,7 +2721,7 @@ static EFI_STATUS run(EFI_HANDLE image) { continue; } - config_entry_bump_counters(entry, root_dir); + (void) config_entry_bump_counters(entry); save_selected_entry(&config, entry); /* Optionally, read a random seed off the ESP and pass it to the OS */ From e80037b10fff052357ab0e1fd2169b1cae00f875 Mon Sep 17 00:00:00 2001 From: Jan Janssen Date: Wed, 2 Aug 2023 16:21:51 +0200 Subject: [PATCH 3/3] boot: Make file info size a constant --- src/boot/efi/proto/file-io.h | 6 ++++++ src/boot/efi/util.c | 9 ++------- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/boot/efi/proto/file-io.h b/src/boot/efi/proto/file-io.h index 106c267fdfb..57df9e111c1 100644 --- a/src/boot/efi/proto/file-io.h +++ b/src/boot/efi/proto/file-io.h @@ -31,6 +31,12 @@ typedef struct { char16_t FileName[]; } EFI_FILE_INFO; +/* Some broken firmware violates the EFI spec by still advancing the readdir + * position when returning EFI_BUFFER_TOO_SMALL, effectively skipping over any files when + * the buffer was too small. Therefore, we always start with a buffer that should handle FAT32 + * max file name length. */ +#define EFI_FILE_INFO_MIN_SIZE (sizeof(EFI_FILE_INFO) + 256 * sizeof(char16_t)) + typedef struct EFI_SIMPLE_FILE_SYSTEM_PROTOCOL EFI_SIMPLE_FILE_SYSTEM_PROTOCOL; struct EFI_SIMPLE_FILE_SYSTEM_PROTOCOL { uint64_t Revision; diff --git a/src/boot/efi/util.c b/src/boot/efi/util.c index d2b8f882500..3beab238d4b 100644 --- a/src/boot/efi/util.c +++ b/src/boot/efi/util.c @@ -412,7 +412,7 @@ void sort_pointer_array( } EFI_STATUS get_file_info(EFI_FILE *handle, EFI_FILE_INFO **ret, size_t *ret_size) { - size_t size = offsetof(EFI_FILE_INFO, FileName) + 256; + size_t size = EFI_FILE_INFO_MIN_SIZE; _cleanup_free_ EFI_FILE_INFO *fi = NULL; EFI_STATUS err; @@ -454,12 +454,7 @@ EFI_STATUS readdir( * the specified buffer needs to be freed by caller, after final use. */ if (!*buffer) { - /* Some broken firmware violates the EFI spec by still advancing the readdir - * position when returning EFI_BUFFER_TOO_SMALL, effectively skipping over any files when - * the buffer was too small. Therefore, start with a buffer that should handle FAT32 max - * file name length. - * As a side effect, most readdir() calls will now be slightly faster. */ - sz = sizeof(EFI_FILE_INFO) + 256 * sizeof(char16_t); + sz = EFI_FILE_INFO_MIN_SIZE; *buffer = xmalloc(sz); *buffer_size = sz; } else