diff --git a/src/bootctl/bootctl-install.c b/src/bootctl/bootctl-install.c index b609d8f227c..5fd65e02f15 100644 --- a/src/bootctl/bootctl-install.c +++ b/src/bootctl/bootctl-install.c @@ -462,13 +462,13 @@ static int version_check(int fd_from, const char *from, int fd_to, const char *t if (r == -ESRCH) return log_notice_errno(r, "Source file \"%s\" does not carry version information!", from); if (r < 0) - return r; + return log_error_errno(r, "Failed to get file version of '%s': %m", from); r = get_file_version(fd_to, &b); if (r == -ESRCH) return log_info_errno(r, "Skipping \"%s\", it's owned by another boot loader (no version info found).", to); if (r < 0) - return r; + return log_error_errno(r, "Failed to get file version of '%s': %m", to); if (compare_product(a, b) != 0) return log_info_errno(SYNTHETIC_ERRNO(ESRCH), "Skipping \"%s\", it's owned by another boot loader.", to); @@ -1806,6 +1806,8 @@ static int remove_boot_efi(InstallContext *c) { return log_oom(); fd = xopenat_full(dirfd(d), de->d_name, O_RDONLY|O_CLOEXEC|O_NONBLOCK|O_NOCTTY|O_NOFOLLOW, XO_REGULAR, /* mode= */ MODE_INVALID); + if (fd == -ENOENT) + continue; if (fd < 0) return log_error_errno(fd, "Failed to open '%s' for reading: %m", z); @@ -1821,8 +1823,10 @@ static int remove_boot_efi(InstallContext *c) { r = get_file_version(fd, &v); if (r == -ESRCH) continue; /* No version information */ - if (r < 0) - return r; + if (r < 0) { + log_warning_errno(r, "Failed to get file version of '%s', skipping: %m", de->d_name); + continue; + } if (!startswith(v, "systemd-boot ")) continue; diff --git a/src/bootctl/bootctl-status.c b/src/bootctl/bootctl-status.c index f1c75c4a65a..8ac24447442 100644 --- a/src/bootctl/bootctl-status.c +++ b/src/bootctl/bootctl-status.c @@ -217,13 +217,15 @@ static int enumerate_binaries( return log_oom(); LOG_SET_PREFIX(filename); - fd = openat(dirfd(d), de->d_name, O_RDONLY|O_CLOEXEC); + fd = RET_NERRNO(openat(dirfd(d), de->d_name, O_RDONLY|O_CLOEXEC)); + if (fd == -ENOENT) + continue; if (fd < 0) - return log_error_errno(errno, "Failed to open file for reading: %m"); + return log_error_errno(fd, "Failed to open file '%s' for reading: %m", de->d_name); r = get_file_version(fd, &v); if (r < 0 && r != -ESRCH) - return r; + return log_error_errno(r, "Failed to get file version of '%s': %m", de->d_name); if (*previous) { /* Let's output the previous entry now, since now we know that there will be * one more, and can draw the tree glyph properly. */ @@ -237,7 +239,7 @@ static int enumerate_binaries( /* Do not output this entry immediately, but store what should be printed in a state * variable, because we only will know the tree glyph to print (branch or final edge) once we * read one more entry */ - if (r == -ESRCH) /* No systemd-owned file but still interesting to print */ + if (r == -ESRCH) /* Not systemd-owned file but still interesting to print */ r = asprintf(previous, "%s%s/%s/%s/%s", ansi_grey(), esp_path, ansi_normal(), path, de->d_name); else /* if (r >= 0) */ diff --git a/src/bootctl/bootctl-util.c b/src/bootctl/bootctl-util.c index 9ed31372bf4..9bd85d64614 100644 --- a/src/bootctl/bootctl-util.c +++ b/src/bootctl/bootctl-util.c @@ -1,7 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #include -#include #include #include "alloc-util.h" @@ -12,7 +11,7 @@ #include "errno-util.h" #include "fileio.h" #include "log.h" -#include "stat-util.h" +#include "pe-binary.h" #include "string-util.h" #include "sync-util.h" #include "virt.h" @@ -133,61 +132,60 @@ const char* get_efi_arch(void) { return EFI_MACHINE_TYPE_NAME; } -/* search for "#### LoaderInfo: systemd-boot 218 ####" string inside the binary */ int get_file_version(int fd, char **ret) { - struct stat st; - char *buf; - const char *s, *e; - char *marker = NULL; int r; assert(fd >= 0); assert(ret); - /* Does not reposition file offset (as it uses mmap()) */ + /* Reads the version marker that systemd-boot/systemd-stub and friends store in their ".sdmagic" PE + * section, i.e. a string such as "#### LoaderInfo: systemd-boot 218 ####", and returns the inner + * part, e.g. "systemd-boot 218". Does not reposition the file offset (as it uses pread()). */ - if (fstat(fd, &st) < 0) - return log_error_errno(errno, "Failed to stat EFI binary: %m"); + _cleanup_free_ IMAGE_DOS_HEADER *dos_header = NULL; + _cleanup_free_ PeHeader *pe_header = NULL; + r = pe_load_headers(fd, &dos_header, &pe_header); + if (r == -EBADMSG) + return log_debug_errno(SYNTHETIC_ERRNO(ESRCH), "EFI binary is not a valid PE file, assuming no version information."); + if (r < 0) + return r; - r = stat_verify_regular(&st); - if (r < 0) { - log_debug_errno(r, "EFI binary is not a regular file, assuming no version information: %m"); - return -ESRCH; - } + _cleanup_free_ IMAGE_SECTION_HEADER *sections = NULL; + r = pe_load_sections(fd, dos_header, pe_header, §ions); + if (r == -EBADMSG) + return log_debug_errno(SYNTHETIC_ERRNO(ESRCH), "Failed to load PE section table, assuming no version information."); + if (r < 0) + return r; - if (st.st_size < 27 || file_offset_beyond_memory_size(st.st_size)) - return log_debug_errno(SYNTHETIC_ERRNO(ESRCH), - "EFI binary size too %s: %"PRIi64, - st.st_size < 27 ? "small" : "large", st.st_size); + _cleanup_free_ char *sdmagic = NULL; + r = pe_read_section_data_by_name( + fd, + pe_header, + sections, + ".sdmagic", + /* max_size= */ 4U*1024U, + (void**) &sdmagic, + /* ret_size= */ NULL); + if (IN_SET(r, -ENXIO, -EBADMSG)) + return log_debug_errno(SYNTHETIC_ERRNO(ESRCH), "EFI binary has no .sdmagic section, assuming no version information."); + if (r < 0) + return log_debug_errno(r, "Failed to read .sdmagic section of EFI binary: %m"); - buf = mmap(NULL, st.st_size, PROT_READ, MAP_PRIVATE, fd, 0); - if (buf == MAP_FAILED) - return log_error_errno(errno, "Failed to mmap EFI binary: %m"); + const char *p = startswith(sdmagic, "#### LoaderInfo: "); + if (!p) + return log_debug_errno(SYNTHETIC_ERRNO(ESRCH), "EFI binary .sdmagic section lacks LoaderInfo marker."); - s = mempmem_safe(buf, st.st_size - 8, "#### LoaderInfo: ", 17); - if (!s) { - r = log_debug_errno(SYNTHETIC_ERRNO(ESRCH), "EFI binary has no LoaderInfo marker."); - goto finish; - } + const char *e = endswith(p, " ####"); + if (!e || e <= p) + return log_debug_errno(SYNTHETIC_ERRNO(ESRCH), "EFI binary has malformed LoaderInfo marker."); - e = memmem_safe(s, st.st_size - (s - buf), " ####", 5); - if (!e || e - s < 3) { - r = log_error_errno(SYNTHETIC_ERRNO(EINVAL), "EFI binary has malformed LoaderInfo marker."); - goto finish; - } - - marker = strndup(s, e - s); - if (!marker) { - r = log_oom(); - goto finish; - } + char *marker = strndup(p, e - p); + if (!marker) + return log_oom_debug(); log_debug("EFI binary LoaderInfo marker: \"%s\"", marker); - r = 0; - *ret = marker; -finish: - (void) munmap(buf, st.st_size); - return r; + *ret = TAKE_PTR(marker); + return 0; } int settle_entry_token(void) { diff --git a/test/units/TEST-87-AUX-UTILS-VM.bootctl.sh b/test/units/TEST-87-AUX-UTILS-VM.bootctl.sh index 90daaf52ead..2b095c1fbcb 100755 --- a/test/units/TEST-87-AUX-UTILS-VM.bootctl.sh +++ b/test/units/TEST-87-AUX-UTILS-VM.bootctl.sh @@ -113,6 +113,48 @@ testcase_bootctl_basic() { basic_tests } +cleanup_file_version() { + if [[ -n "${FILE_VERSION_GARBAGE:-}" ]]; then + rm -f "$FILE_VERSION_GARBAGE" + unset FILE_VERSION_GARBAGE + fi + restore_esp +} + +testcase_bootctl_file_version() { + # Exercise get_file_version() (src/bootctl/bootctl-util.c), which reads the + # version marker that systemd-boot/-stub store in their ".sdmagic" PE + # section, e.g. "#### LoaderInfo: systemd-boot 257 ####". 'bootctl status' + # surfaces the inner part next to each .efi file it finds in the ESP, e.g. + # ".../systemd-bootx64.efi (systemd-boot 257)". + + backup_esp + trap cleanup_file_version RETURN ERR + + bootctl install + + local ESP + ESP="$(bootctl --print-esp-path)" + + # The freshly installed systemd-boot binary must be listed with the version + # extracted from its ".sdmagic" section. + bootctl status | grep -E "systemd-boot[a-z0-9]*\.efi \(systemd-boot [^)]+\)" >/dev/null + + # A non-PE/non-systemd file must be handled gracefully (get_file_version() + # returns -ESRCH): it is still listed, but without a version annotation, and + # 'bootctl status' must not fail. + FILE_VERSION_GARBAGE="$ESP/EFI/systemd/not-a-loader.efi" + echo "this is not a PE binary" >"$FILE_VERSION_GARBAGE" + + local status + status="$(bootctl status)" + grep -F "not-a-loader.efi" <<<"$status" >/dev/null + (! grep -E "not-a-loader\.efi .*\(" <<<"$status" >/dev/null) + + rm -f "$FILE_VERSION_GARBAGE" + unset FILE_VERSION_GARBAGE +} + cleanup_image() ( set +e