File fix-bless-boot.patch of Package systemd
From 7a8372a9f1380d5e178accc3d5379dd2857da33a Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Wed, 7 May 2025 15:23:00 +0200
Subject: [PATCH 1/3] bless-boot: switch from last_path_component() to
path_find_last_component()
Using path_find_last_component() means special cases such as the root dir
and paths referencing dirs are detected and refused.
---
src/bless-boot/bless-boot.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/src/bless-boot/bless-boot.c b/src/bless-boot/bless-boot.c
index b3205dec3631a..c41a948549b38 100644
--- a/src/bless-boot/bless-boot.c
+++ b/src/bless-boot/bless-boot.c
@@ -215,11 +215,9 @@ static int acquire_boot_count_path(
uint64_t *ret_done,
char **ret_suffix) {
- _cleanup_free_ char *path = NULL, *prefix = NULL, *suffix = NULL;
- const char *last, *e;
- uint64_t left, done;
int r;
+ _cleanup_free_ char *path = NULL;
r = efi_get_variable_path(EFI_LOADER_VARIABLE_STR("LoaderBootCountPath"), &path);
if (r == -ENOENT)
return -EUNATCH; /* in this case, let the caller print a message */
@@ -236,23 +234,34 @@ static int acquire_boot_count_path(
"Path read from LoaderBootCountPath is not absolute, refusing: %s",
path);
- last = last_path_component(path);
- e = strrchr(last, '+');
+ const char *last = NULL;
+ r = path_find_last_component(path, /* accept_dot_dot= */ false, /* next= */ NULL, &last);
+ if (r < 0)
+ return log_error_errno(r, "Failed to extract filename from LoaderBootCountPath '%s': %m", path);
+ if (r == 0)
+ return log_error_errno(SYNTHETIC_ERRNO(EADDRNOTAVAIL), "LoaderBootCountPath '%s' refers to the root directory: %m", path);
+ if (strlen(last) > (size_t) r)
+ return log_error_errno(SYNTHETIC_ERRNO(EISDIR), "LoaderBootCountPath '%s' refers to directory path, refusing.", path);
+
+ const char *e = strrchr(last, '+');
if (!e)
return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
"Path read from LoaderBootCountPath does not contain a counter, refusing: %s",
path);
+ _cleanup_free_ char *prefix = NULL;
if (ret_prefix) {
prefix = strndup(path, e - path);
if (!prefix)
return log_oom();
}
+ uint64_t left, done;
r = parse_counter(path, &e, &left, &done);
if (r < 0)
return r;
+ _cleanup_free_ char *suffix = NULL;
if (ret_suffix) {
suffix = strdup(e);
if (!suffix)
From 9420a0e6cb832d6035c8cf634f11c4b2da0097bd Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Wed, 7 May 2025 15:24:06 +0200
Subject: [PATCH 2/3] bless-boot: in "status" output report bad state from prev
boot as "dirty"
The bless-boot logic currently assumes that if the name of the boot
entry reported via the EFI var matches the name on disk that the state
is "indeterminate", as we haven't counted down the counter (to mark it
bad) or drop the counter (to mark it good) yet. But there's one corner
case we so far didn't care about: what if the entry already reached 0
left tries in a previous boot, i.e. if the user invoked an entry already
known to be completely bad. In that case we'd still return
"indeterminate", but that's kinda misleading, because we *know* the
currently booted entry is bad, however we inherited that fact from a
previous boot, we didn't determine it on the current.
hence, let's introduce a new status we report in this case, that is both
distinct from "bad" (which indicates whether the *current* boot is bad)
and "indirect" (which indicates the current boot has not been decided on
yet): "dirty".
Why "dirty"? To mirror "clean" which we already have, which indicates a
boot already marked good in a previous boot, which is a relatively
symmetric state.
This is a really weak api break of sorts, because it introduces a new
state we never reported before, but I think it's fine, because the old
reporting was just wrong, and in a way this is bugfix, that we now
report correctly something where previously returned kind of rubbish
(though systematic rubbish).
Replaces: #37350
---
man/systemd-bless-boot.service.xml | 28 ++++++++++++++++++----------
src/bless-boot/bless-boot.c | 17 ++++++++++++++---
2 files changed, 32 insertions(+), 13 deletions(-)
diff --git a/man/systemd-bless-boot.service.xml b/man/systemd-bless-boot.service.xml
index 86a3fac799f5d..eb1d345fb82c0 100644
--- a/man/systemd-bless-boot.service.xml
+++ b/man/systemd-bless-boot.service.xml
@@ -57,13 +57,17 @@
<varlistentry>
<term><option>status</option></term>
- <listitem><para>The current status of the boot loader entry file or unified kernel image file is shown. This
- outputs one of <literal>good</literal>, <literal>bad</literal>, <literal>indeterminate</literal>,
- <literal>clean</literal>, depending on the state and previous invocations of the command. The string
- <literal>indeterminate</literal> is shown initially after boot, before it has been marked as "good" or
- "bad". The string <literal>good</literal> is shown after the boot was marked as "good" with the
- <option>good</option> command below, and "bad" conversely after the <option>bad</option> command was
- invoked. The string <literal>clean</literal> is returned when boot counting is currently not in effect.</para>
+ <listitem><para>The current status of the boot loader entry file or unified kernel image file is
+ shown. This outputs one of <literal>good</literal>, <literal>bad</literal>,
+ <literal>indeterminate</literal>, <literal>clean</literal>, <literal>dirty</literal>, depending on
+ the state and previous invocations of the command. The string <literal>indeterminate</literal> is
+ shown initially after boot, before it has been marked as "good" or "bad". The string
+ <literal>good</literal> is shown after the boot was marked as "good" with the <option>good</option>
+ command below, and "bad" conversely after the <option>bad</option> command was invoked. The string
+ <literal>clean</literal> is returned when boot counting is currently not in effect (which includes
+ the case where the current entry was already marked as persistently good). The string
+ <literal>dirty</literal> is returned when the system is booted up with a known-bad kernel (i.e. one
+ where the tries left counter has previously reached zero already).</para>
<para>This command is implied if no command argument is specified.</para>
@@ -96,9 +100,13 @@
<varlistentry>
<term><option>indeterminate</option></term>
- <listitem><para>This command undoes any marking of the current boot loader entry file or unified kernel image
- file as good or bad. This is implemented by renaming the boot loader entry file or unified kernel image file
- back to the path encoded in the <varname>LoaderBootCountPath</varname> EFI variable.</para>
+ <listitem><para>This command undoes any marking of the current boot loader entry file or unified
+ kernel image file as good or bad. This is implemented by renaming the boot loader entry file or
+ unified kernel image file back to the path encoded in the <varname>LoaderBootCountPath</varname> EFI
+ variable. Note that operation will fail if the current kernel is not booted with boot counting
+ enabled (i.e. if the EFI variable is not set). If the boot counter already reached zero tries left on
+ a previous boot this operation will fail too: once an entry is marked <option>bad</option> it can
+ only be reset to <option>good</option> again, but not to <option>indeterminate</option>.</para>
<xi:include href="version-info.xml" xpointer="v240"/></listitem>
</varlistentry>
diff --git a/src/bless-boot/bless-boot.c b/src/bless-boot/bless-boot.c
index c41a948549b38..e0143d973470b 100644
--- a/src/bless-boot/bless-boot.c
+++ b/src/bless-boot/bless-boot.c
@@ -372,7 +372,14 @@ static int verb_status(int argc, char *argv[], void *userdata) {
}
if (faccessat(fd, skip_leading_slash(path), F_OK, 0) >= 0) {
- puts("indeterminate");
+ /* If the item we booted with still exists under its name, it means we have not
+ * change the current boot's marking so far. This may have two reasons: because we
+ * simply didn't do that yet but still plan to, or because the left tries counter is
+ * already at zero, hence we cannot further decrease it to mark it even
+ * "worse"... Here we check the current counter to detect the latter case and return
+ * "dirty", since the item is already marked bad from a previous boot, but otherwise
+ * report "indeterminate" since we just didn't make a decision yet. */
+ puts(left == 0 ? "dirty" : "indeterminate");
return 0;
}
if (errno != ENOENT)
@@ -402,10 +409,10 @@ static int verb_status(int argc, char *argv[], void *userdata) {
static int verb_set(int argc, char *argv[], void *userdata) {
_cleanup_free_ char *path = NULL, *prefix = NULL, *suffix = NULL, *good = NULL, *bad = NULL;
const char *target, *source1, *source2;
- uint64_t done;
+ uint64_t left, done;
int r;
- r = acquire_boot_count_path(&path, &prefix, NULL, &done, &suffix);
+ r = acquire_boot_count_path(&path, &prefix, &left, &done, &suffix);
if (r == -EUNATCH) /* acquire_boot_count_path() won't log on its own for this specific error */
return log_error_errno(r, "Not booted with boot counting in effect.");
if (r < 0)
@@ -434,6 +441,10 @@ static int verb_set(int argc, char *argv[], void *userdata) {
source2 = good; /* Maybe this boot was previously marked as 'good'? */
} else {
assert(streq(argv[0], "indeterminate"));
+
+ if (left == 0)
+ return log_error_errno(r, "Current boot entry was already marked bad in a previous boot, cannot reset to indeterminate.");
+
target = path;
source1 = good;
source2 = bad;
From 30284e3a59f221f5711f1ce63a881dcbfc4c4ead Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Wed, 7 May 2025 15:34:51 +0200
Subject: [PATCH 3/3] bless-boot: never try to rename an entry file onto itself
If we are booting a known bad entry, and we are asked to mark it as bad,
we so far would end up renaming the entry onto itself, which resulted in
EEXIST and is really borked operation. Let's catch that case and handle
it explicitly.
---
src/bless-boot/bless-boot.c | 37 +++++++++++++++++++++++++++++++++----
1 file changed, 33 insertions(+), 4 deletions(-)
diff --git a/src/bless-boot/bless-boot.c b/src/bless-boot/bless-boot.c
index e0143d973470b..3fbd09b7e1eb8 100644
--- a/src/bless-boot/bless-boot.c
+++ b/src/bless-boot/bless-boot.c
@@ -406,6 +406,30 @@ static int verb_status(int argc, char *argv[], void *userdata) {
return log_error_errno(SYNTHETIC_ERRNO(EBUSY), "Couldn't determine boot state.");
}
+static int rename_in_dir_idempotent(int fd, const char *from, const char *to) {
+ int r;
+
+ assert(fd >= 0);
+ assert(from);
+ assert(to);
+
+ /* A wrapper around rename_noreplace() which executes no operation if the source and target are the
+ * same. */
+
+ if (streq(from, to)) {
+ if (faccessat(fd, from, F_OK, AT_SYMLINK_NOFOLLOW) < 0)
+ return -errno;
+
+ return 0;
+ }
+
+ r = rename_noreplace(fd, from, fd, to);
+ if (r < 0)
+ return r;
+
+ return 1;
+}
+
static int verb_set(int argc, char *argv[], void *userdata) {
_cleanup_free_ char *path = NULL, *prefix = NULL, *suffix = NULL, *good = NULL, *bad = NULL;
const char *target, *source1, *source2;
@@ -457,12 +481,12 @@ static int verb_set(int argc, char *argv[], void *userdata) {
if (fd < 0)
return log_error_errno(errno, "Failed to open $BOOT partition '%s': %m", *p);
- r = rename_noreplace(fd, skip_leading_slash(source1), fd, skip_leading_slash(target));
+ r = rename_in_dir_idempotent(fd, skip_leading_slash(source1), skip_leading_slash(target));
if (r == -EEXIST)
goto exists;
if (r == -ENOENT) {
- r = rename_noreplace(fd, skip_leading_slash(source2), fd, skip_leading_slash(target));
+ r = rename_in_dir_idempotent(fd, skip_leading_slash(source2), skip_leading_slash(target));
if (r == -EEXIST)
goto exists;
if (r == -ENOENT) {
@@ -479,11 +503,16 @@ static int verb_set(int argc, char *argv[], void *userdata) {
if (r < 0)
return log_error_errno(r, "Failed to rename '%s' to '%s': %m", source2, target);
- log_debug("Successfully renamed '%s' to '%s'.", source2, target);
+ if (r > 0)
+ log_debug("Successfully renamed '%s' to '%s'.", source2, target);
+ else
+ log_debug("Not renaming, as '%s' already matches target name.", source2);
} else if (r < 0)
return log_error_errno(r, "Failed to rename '%s' to '%s': %m", source1, target);
- else
+ else if (r > 0)
log_debug("Successfully renamed '%s' to '%s'.", source1, target);
+ else
+ log_debug("Not renaming, as '%s' already matches target name.", source1);
/* First, fsync() the directory these files are located in */
r = fsync_parent_at(fd, skip_leading_slash(target));
