From 0d02d0bc51f2fd431f81daa39a970d5dea279f29 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 22 Apr 2026 17:19:21 +0100 Subject: [PATCH 1/5] sysupdate: Prevent a possible invalid partial+pending state on an instance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the resource code is recursing, it’s possible for one iteration to set a partial flag, and then a recursive iteration to set a pending flag (or vice-versa). It doesn’t make sense to have both set at the same time for a specific instance, so make sure to clear the other flag when setting one of them. Add some assertions to make this invariant clearer and easier to debug if it fails. Signed-off-by: Philip Withnall --- src/sysupdate/sysupdate-resource.c | 8 ++++++++ src/sysupdate/sysupdate.c | 5 ++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/sysupdate/sysupdate-resource.c b/src/sysupdate/sysupdate-resource.c index acd9604f29..bd1560371d 100644 --- a/src/sysupdate/sysupdate-resource.c +++ b/src/sysupdate/sysupdate-resource.c @@ -139,9 +139,11 @@ static int resource_load_from_directory_recursive( if ((stripped = startswith(de->d_name, ".sysupdate.partial."))) { de_d_name_stripped = stripped; is_partial = true; + is_pending = false; } else if ((stripped = startswith(de->d_name, ".sysupdate.pending."))) { de_d_name_stripped = stripped; is_pending = true; + is_partial = false; } else de_d_name_stripped = de->d_name; @@ -192,6 +194,9 @@ static int resource_load_from_directory_recursive( if (instance->metadata.mode == MODE_INVALID) instance->metadata.mode = st.st_mode & 0775; /* mask out world-writability and suid and stuff, for safety */ + /* Can’t be both partial and pending. */ + assert(!(is_partial && is_pending)); + instance->is_partial = is_partial; instance->is_pending = is_pending; } @@ -313,6 +318,9 @@ static int resource_load_from_blockdev(Resource *rr) { if (instance->metadata.read_only < 0) instance->metadata.read_only = instance->partition_info.read_only; + /* Can’t be both partial and pending. */ + assert(!(is_partial && is_pending)); + instance->is_partial = is_partial; instance->is_pending = is_pending; } diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index 76b6507f9a..2dd1bfdaac 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -410,7 +410,10 @@ static int context_discover_update_sets_by_flag(Context *c, UpdateSetFlags flags extra_flags |= UPDATE_PROTECTED; /* Partial or pending updates by definition are not incomplete, they’re - * partial/pending instead */ + * partial/pending instead. While an individual Instance cannot be both partial and + * pending, an UpdateSet as a whole can contain both partial and pending instances. */ + assert(!match || !(match->is_partial && match->is_pending)); + if (match && match->is_partial) extra_flags = (extra_flags | UPDATE_PARTIAL) & ~UPDATE_INCOMPLETE; From b2d19b4651fb87b8f6dc8a427a96d7a9d3a16961 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 22 Apr 2026 17:23:49 +0100 Subject: [PATCH 2/5] sysupdate: Allow partial+pending flags in a few more places for UpdateSets While a resource Instance can either be partial or pending, but not both; an UpdateSet (which potentially comprises several Instances) can be both partial *and* pending if it contains Instances in both those states. Amend a few bits of internal code to allow that in situations which were previously overlooked. Signed-off-by: Philip Withnall --- src/sysupdate/sysupdate-update-set-flags.c | 5 +++++ src/sysupdate/sysupdate.c | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/sysupdate/sysupdate-update-set-flags.c b/src/sysupdate/sysupdate-update-set-flags.c index 36801938f6..7b684576b0 100644 --- a/src/sysupdate/sysupdate-update-set-flags.c +++ b/src/sysupdate/sysupdate-update-set-flags.c @@ -70,6 +70,11 @@ const char* update_set_flags_to_string(UpdateSetFlags flags) { case UPDATE_INSTALLED|UPDATE_PARTIAL|UPDATE_NEWEST|UPDATE_PROTECTED: case UPDATE_INSTALLED|UPDATE_AVAILABLE|UPDATE_PARTIAL|UPDATE_NEWEST: case UPDATE_INSTALLED|UPDATE_AVAILABLE|UPDATE_PARTIAL|UPDATE_NEWEST|UPDATE_PROTECTED: + /* can also contain pending instances: */ + case UPDATE_INSTALLED|UPDATE_PARTIAL|UPDATE_PENDING|UPDATE_NEWEST: + case UPDATE_INSTALLED|UPDATE_PARTIAL|UPDATE_PENDING|UPDATE_NEWEST|UPDATE_PROTECTED: + case UPDATE_INSTALLED|UPDATE_AVAILABLE|UPDATE_PARTIAL|UPDATE_PENDING|UPDATE_NEWEST: + case UPDATE_INSTALLED|UPDATE_AVAILABLE|UPDATE_PARTIAL|UPDATE_PENDING|UPDATE_NEWEST|UPDATE_PROTECTED: return "current+partial"; case UPDATE_AVAILABLE|UPDATE_NEWEST: diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index 2dd1bfdaac..89efe36c85 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -669,7 +669,7 @@ static int context_show_version(Context *c, const char *version) { Instance *i = *inst; if (!i) { - assert(FLAGS_SET(us->flags, UPDATE_INCOMPLETE)); + assert(us->flags & (UPDATE_INCOMPLETE|UPDATE_PARTIAL|UPDATE_PENDING)); continue; } From 2babac90137ea4b6a70958133c783131d2619051 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 22 Apr 2026 17:25:44 +0100 Subject: [PATCH 3/5] sysupdate: Allow a partial version to be the candidate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously we allowed a pending version to be the candidate — but if there are no better choices, then we might as well allow a partial version to be candidate as well. The alternative is having no update candidate when a new version is partially installed (i.e. downloaded but not moved into place). This would mean that an update which is interrupted then needs to be re-run with an explicit version number to progress, rather than being able to be re-run without a version number (as it was in the first place). Signed-off-by: Philip Withnall Helps: https://github.com/systemd/systemd/issues/41502 --- src/sysupdate/sysupdate.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index 89efe36c85..cba7960f0b 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -505,8 +505,9 @@ static int context_discover_update_sets_by_flag(Context *c, UpdateSetFlags flags c->candidate && strverscmp_improved(c->newest_installed->version, c->candidate->version) >= 0) c->candidate = NULL; - /* Newest installed is still pending and no candidate is set? Then it becomes the candidate. */ - if (c->newest_installed && FLAGS_SET(c->newest_installed->flags, UPDATE_PENDING) && + /* Newest installed is still pending or partial and no candidate is set? Then it becomes the candidate. */ + if (c->newest_installed && + (c->newest_installed->flags & (UPDATE_PENDING|UPDATE_PARTIAL)) && !c->candidate) c->candidate = c->newest_installed; From e1146fe710d0d1bbe0fdca974f66edd7f6c573cc Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 22 Apr 2026 17:28:31 +0100 Subject: [PATCH 4/5] sysupdate: Allow a partial version to be vacuumed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously we prevented partial and pending versions from being vacuumed. But until we support resuming downloads, there’s nothing else which can be done with a partial version except to vacuum it and try again. Accordingly, allow partial versions (but not pending versions) to be vacuumed. This behaviour can be changed again once resuming downloads is supported — at that point I expect we’ll want to try resuming the partial download rather than throwing it all away and trying again. Signed-off-by: Philip Withnall Helps: https://github.com/systemd/systemd/issues/41502 --- src/sysupdate/sysupdate-transfer.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/sysupdate/sysupdate-transfer.c b/src/sysupdate/sysupdate-transfer.c index 3ed7a2ae3a..8db1c81962 100644 --- a/src/sysupdate/sysupdate-transfer.c +++ b/src/sysupdate/sysupdate-transfer.c @@ -816,10 +816,13 @@ int transfer_vacuum( continue; } - /* If this is listed among the protected versions, then let's not remove it */ - if (strv_contains(t->protected_versions, instance->metadata.version) || - (extra_protected_version && streq(extra_protected_version, instance->metadata.version))) { - log_debug("Version '%s' is pending/partial but protected, not removing.", instance->metadata.version); + /* If this is pending and listed among the protected versions, then let's not remove it. + * In future, we will also want to keep partial protected versions, but that’s only useful + * once we support resuming downloads. */ + if (instance->is_pending && + (strv_contains(t->protected_versions, instance->metadata.version) || + (extra_protected_version && streq(extra_protected_version, instance->metadata.version)))) { + log_debug("Version '%s' is pending but protected, not removing.", instance->metadata.version); i++; continue; } From 66b950cd3f47f49087ddd4a2f4812e82b209f2b7 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 22 Apr 2026 17:31:27 +0100 Subject: [PATCH 5/5] updatectl: Show a helpful error if an update is partially downloaded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If an update is partially downloaded and the user tries to update again, `updatectl` can’t currently do anything (it doesn’t yet support resuming downloads). At the moment, though, it’ll return success as if the system was up to date, even though it isn’t up to date. Instead, print a more helpful error message telling the user to try vacuuming the partial version and trying again. I decided not to make it automatically vacuum the partial version, as that seems like a way to get into a nasty retry loop if, for example, the checksum provided by the server doesn’t match that of the downloaded file (which is one way to trigger this code path). Add an integration test which simulates this failure by corrupting the `SHA256SUMS` file, trying to download an update, and then working through the recovery steps. Signed-off-by: Philip Withnall Fixes: https://github.com/systemd/systemd/issues/41502 --- src/sysupdate/sysupdate.c | 4 +-- src/sysupdate/updatectl.c | 4 +++ test/units/TEST-72-SYSUPDATE.sh | 50 +++++++++++++++++++++++++++++++-- 3 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/sysupdate/sysupdate.c b/src/sysupdate/sysupdate.c index cba7960f0b..4d083db220 100644 --- a/src/sysupdate/sysupdate.c +++ b/src/sysupdate/sysupdate.c @@ -1029,9 +1029,7 @@ static int context_acquire( if (FLAGS_SET(us->flags, UPDATE_INCOMPLETE)) log_info("Selected update '%s' is already installed, but incomplete. Repairing.", us->version); else if (FLAGS_SET(us->flags, UPDATE_PARTIAL)) { - log_info("Selected update '%s' is already acquired and partially installed. Vacuum it to try installing again.", us->version); - - return 0; + return log_error_errno(SYNTHETIC_ERRNO(EUCLEAN), "Selected update '%s' is already acquired and partially installed. Vacuum it to try installing again.", us->version); } else if (FLAGS_SET(us->flags, UPDATE_PENDING)) { log_info("Selected update '%s' is already acquired and pending installation.", us->version); diff --git a/src/sysupdate/updatectl.c b/src/sysupdate/updatectl.c index c6e5c33fdf..16e9d21ae9 100644 --- a/src/sysupdate/updatectl.c +++ b/src/sysupdate/updatectl.c @@ -867,6 +867,10 @@ static int update_render_progress(sd_event_source *source, void *userdata) { clear_progress_bar_unbuffered(target); fprintf(stderr, "%s: %s Already up-to-date\n", target, GREEN_CHECK_MARK()); n--; /* Don't consider this target in the total */ + } else if (progress == -EUCLEAN) { + clear_progress_bar_unbuffered(target); + fprintf(stderr, "%s: %s Update is already acquired and partially installed. Vacuum it to try installing again.\n", target, RED_CROSS_MARK()); + total += 100; } else if (progress < 0) { clear_progress_bar_unbuffered(target); fprintf(stderr, "%s: %s %s\n", target, RED_CROSS_MARK(), STRERROR(progress)); diff --git a/test/units/TEST-72-SYSUPDATE.sh b/test/units/TEST-72-SYSUPDATE.sh index 27268c250b..6709cd543f 100755 --- a/test/units/TEST-72-SYSUPDATE.sh +++ b/test/units/TEST-72-SYSUPDATE.sh @@ -66,6 +66,7 @@ update_checksums_with_best_before() { new_version() { local sector_size="${1:?}" local version="${2:?}" + local corrupt="${3:-}" # Create a pair of random partition payloads, and compress one. # To make not the initial bytes of part1-xxx.raw accidentally match one of the compression header, @@ -90,11 +91,26 @@ new_version() { echo $RANDOM >"$WORKDIR/source/dir-$version/bar.txt" tar --numeric-owner -C "$WORKDIR/source/dir-$version/" -czf "$WORKDIR/source/dir-$version.tar.gz" . - update_checksums + if [[ "$corrupt" == "corrupt-checksum" ]]; then + # As requested, add a deliberately corrupt checksum for this file. This + # will get overwritten next time update_checksums() is called, but the + # integration test will probably have moved on to other things by then. + { + echo "abad1deaabad1deaabad1deaabad1deaabad1deaabad1deaabad1deaabad1dea part1-$version.raw" + echo "abad1deaabad1deaabad1deaabad1deaabad1deaabad1deaabad1deaabad1dea part2-$version.raw" + echo "abad1deaabad1deaabad1deaabad1deaabad1deaabad1deaabad1deaabad1dea part2-$version.raw.gz" + echo "abad1deaabad1deaabad1deaabad1deaabad1deaabad1deaabad1deaabad1dea uki-$version.efi" + echo "abad1deaabad1deaabad1deaabad1deaabad1deaabad1deaabad1deaabad1dea uki-extra-$version.efi" + echo "abad1deaabad1deaabad1deaabad1deaabad1deaabad1deaabad1deaabad1dea dir-$version.tar.gz" + } >> "$WORKDIR/source/SHA256SUMS" + else + update_checksums + fi } update_now() { local update_type="${1:?}" + local checks="${2:-}" # Update to newest version. First there should be an update ready, then we # do the update, and then there should not be any ready anymore @@ -105,7 +121,10 @@ update_now() { # modes. Some updates in the test suite need to be monolithic (e.g. when # repairing an installation), so that can be overridden via the local. - "$SYSUPDATE" --verify=no check-new + if [[ "$checks" != "no-checks" ]]; then + "$SYSUPDATE" --verify=no check-new + fi + if [[ "$update_type" == "monolithic" ]]; then "$SYSUPDATE" --verify=no update elif [[ "$update_type" == "split-offline" ]]; then @@ -125,7 +144,10 @@ update_now() { else exit 1 fi - (! "$SYSUPDATE" --verify=no check-new) + + if [[ "$checks" != "no-checks" ]]; then + (! "$SYSUPDATE" --verify=no check-new) + fi } verify_version() { @@ -462,6 +484,28 @@ EOF verify_version_current "$blockdev" "$sector_size" v8 1 verify_version "$blockdev" "$sector_size" v7 2 + # Create a 9th version but corrupt the checksum in SHA256SUMS so pulling it + # fails when verifying the checksum, in order to create a current+partial + # state. Try to update again and verify that this results in an error. + # Vacuum the partial version, regenerate it on the server, try updating + # again and it should succeed. + new_version "$sector_size" v9 "corrupt-checksum" + (! update_now "$update_type") + "$SYSUPDATE" --offline list v9 | grep "partial" >/dev/null + verify_version_current "$blockdev" "$sector_size" v8 1 + # don’t verify the other part of the block device as it’s in an indeterminate state + (! update_now "$update_type" "no-checks") |& tee "$WORKDIR"/update_now-9 + cat "$WORKDIR"/update_now-9 + grep "is already acquired and partially installed. Vacuum it to try installing again." "$WORKDIR"/update_now-9 + "$SYSUPDATE" --offline vacuum |& grep "Removing old partial" >/dev/null + verify_version_current "$blockdev" "$sector_size" v8 1 + # don’t verify the other part of the block device as it’s in an indeterminate state + "$SYSUPDATE" --verify=no list v9 | grep "candidate" >/dev/null + new_version "$sector_size" v9 + update_now "$update_type" + verify_version "$blockdev" "$sector_size" v8 1 + verify_version_current "$blockdev" "$sector_size" v9 2 + # Cleanup [[ -b "$blockdev" ]] && losetup --detach "$blockdev" rm "$BACKING_FILE"