From fbc88824b6379a902e611c6d5820d3aa0bd85136 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 31 Mar 2024 20:17:14 +0800 Subject: [PATCH 1/7] hibernate-util: make clear_efi_hibernate_location_and_warn return 1 if actually cleared the variable --- src/hibernate-resume/hibernate-resume.c | 2 +- src/shared/hibernate-util.c | 10 +++++++--- src/shared/hibernate-util.h | 2 +- src/sleep/sleep.c | 2 +- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/hibernate-resume/hibernate-resume.c b/src/hibernate-resume/hibernate-resume.c index 3f45836d59..519aefe29f 100644 --- a/src/hibernate-resume/hibernate-resume.c +++ b/src/hibernate-resume/hibernate-resume.c @@ -60,7 +60,7 @@ static int run(int argc, char *argv[]) { return r; if (arg_info.efi) - clear_efi_hibernate_location_and_warn(); + (void) clear_efi_hibernate_location_and_warn(); } if (stat(arg_info.device, &st) < 0) diff --git a/src/shared/hibernate-util.c b/src/shared/hibernate-util.c index 3547476c85..2e3f7cfb84 100644 --- a/src/shared/hibernate-util.c +++ b/src/shared/hibernate-util.c @@ -513,13 +513,17 @@ int write_resume_config(dev_t devno, uint64_t offset, const char *device) { return 0; } -void clear_efi_hibernate_location_and_warn(void) { +int clear_efi_hibernate_location_and_warn(void) { int r; if (!is_efi_boot()) - return; + return 0; r = efi_set_variable(EFI_SYSTEMD_VARIABLE(HibernateLocation), NULL, 0); + if (r == -ENOENT) + return 0; if (r < 0) - log_warning_errno(r, "Failed to clear EFI variable HibernateLocation, ignoring: %m"); + return log_warning_errno(r, "Failed to clear EFI variable HibernateLocation: %m"); + + return 1; } diff --git a/src/shared/hibernate-util.h b/src/shared/hibernate-util.h index 1064d2b4dd..394d0b4176 100644 --- a/src/shared/hibernate-util.h +++ b/src/shared/hibernate-util.h @@ -22,7 +22,7 @@ int hibernation_is_safe(void); int write_resume_config(dev_t devno, uint64_t offset, const char *device); -void clear_efi_hibernate_location_and_warn(void); +int clear_efi_hibernate_location_and_warn(void); /* Only for test-fiemap */ int read_fiemap(int fd, struct fiemap **ret); diff --git a/src/sleep/sleep.c b/src/sleep/sleep.c index 693484e886..c96207428d 100644 --- a/src/sleep/sleep.c +++ b/src/sleep/sleep.c @@ -308,7 +308,7 @@ static int execute( fail: if (SLEEP_OPERATION_IS_HIBERNATION(operation)) - clear_efi_hibernate_location_and_warn(); + (void) clear_efi_hibernate_location_and_warn(); return r; } From 667b0c48f2f4ca3ae7b5fcb93fe00bd3aba21d20 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 3 Apr 2024 00:51:37 +0800 Subject: [PATCH 2/7] hibernate-resume: EINVAL -> ENOTBLK where appropriate --- src/hibernate-resume/hibernate-resume.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hibernate-resume/hibernate-resume.c b/src/hibernate-resume/hibernate-resume.c index 519aefe29f..27411faa7f 100644 --- a/src/hibernate-resume/hibernate-resume.c +++ b/src/hibernate-resume/hibernate-resume.c @@ -67,7 +67,7 @@ static int run(int argc, char *argv[]) { return log_error_errno(errno, "Failed to stat resume device '%s': %m", arg_info.device); if (!S_ISBLK(st.st_mode)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + return log_error_errno(SYNTHETIC_ERRNO(ENOTBLK), "Resume device '%s' is not a block device.", arg_info.device); /* The write shall not return if a resume takes place. */ From 9fa0f584ec8590b22c3b8529b65e7368983fcd71 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 3 Apr 2024 09:42:59 +0800 Subject: [PATCH 3/7] hibernate-resume-config: expose get_efi_hibernate_location --- .../hibernate-resume-config.c | 35 ++++++------------- .../hibernate-resume-config.h | 31 ++++++++++------ 2 files changed, 30 insertions(+), 36 deletions(-) diff --git a/src/hibernate-resume/hibernate-resume-config.c b/src/hibernate-resume/hibernate-resume-config.c index c6db83a9d3..504d27a396 100644 --- a/src/hibernate-resume/hibernate-resume-config.c +++ b/src/hibernate-resume/hibernate-resume-config.c @@ -28,20 +28,7 @@ static KernelHibernateLocation* kernel_hibernate_location_free(KernelHibernateLo DEFINE_TRIVIAL_CLEANUP_FUNC(KernelHibernateLocation*, kernel_hibernate_location_free); -typedef struct EFIHibernateLocation { - char *device; - - sd_id128_t uuid; - uint64_t offset; - - char *kernel_version; - char *id; - char *image_id; - char *version_id; - char *image_version; -} EFIHibernateLocation; - -static EFIHibernateLocation* efi_hibernate_location_free(EFIHibernateLocation *e) { +EFIHibernateLocation* efi_hibernate_location_free(EFIHibernateLocation *e) { if (!e) return NULL; @@ -55,8 +42,6 @@ static EFIHibernateLocation* efi_hibernate_location_free(EFIHibernateLocation *e return mfree(e); } -DEFINE_TRIVIAL_CLEANUP_FUNC(EFIHibernateLocation*, efi_hibernate_location_free); - void hibernate_info_done(HibernateInfo *info) { assert(info); @@ -140,7 +125,7 @@ static bool validate_efi_hibernate_location(EFIHibernateLocation *e) { if (!streq_ptr(id, e->id) || !streq_ptr(image_id, e->image_id)) { - log_notice("HibernateLocation system identifier doesn't match currently running system, not resuming from it."); + log_notice("HibernateLocation system identifier doesn't match currently running system, would not resume from it."); return false; } @@ -152,8 +137,9 @@ static bool validate_efi_hibernate_location(EFIHibernateLocation *e) { return true; } +#endif -static int get_efi_hibernate_location(EFIHibernateLocation **ret) { +int get_efi_hibernate_location(EFIHibernateLocation **ret) { static const JsonDispatch dispatch_table[] = { { "uuid", JSON_VARIANT_STRING, json_dispatch_id128, offsetof(EFIHibernateLocation, uuid), JSON_MANDATORY }, @@ -171,8 +157,6 @@ static int get_efi_hibernate_location(EFIHibernateLocation **ret) { _cleanup_free_ char *location_str = NULL; int r; - assert(ret); - if (!is_efi_boot()) goto skip; @@ -211,15 +195,18 @@ static int get_efi_hibernate_location(EFIHibernateLocation **ret) { if (asprintf(&e->device, "/dev/disk/by-uuid/" SD_ID128_UUID_FORMAT_STR, SD_ID128_FORMAT_VAL(e->uuid)) < 0) return log_oom(); - *ret = TAKE_PTR(e); + if (ret) + *ret = TAKE_PTR(e); return 1; skip: - *ret = NULL; + if (ret) + *ret = NULL; return 0; } void compare_hibernate_location_and_warn(const HibernateInfo *info) { +#if ENABLE_EFI int r; assert(info); @@ -243,8 +230,8 @@ void compare_hibernate_location_and_warn(const HibernateInfo *info) { if (info->cmdline->offset != info->efi->offset) log_warning("resume_offset=%" PRIu64 " doesn't match with EFI HibernateLocation offset %" PRIu64 ", proceeding anyway with resume_offset=.", info->cmdline->offset, info->efi->offset); -} #endif +} int acquire_hibernate_info(HibernateInfo *ret) { _cleanup_(hibernate_info_done) HibernateInfo i = {}; @@ -254,11 +241,9 @@ int acquire_hibernate_info(HibernateInfo *ret) { if (r < 0) return r; -#if ENABLE_EFI r = get_efi_hibernate_location(&i.efi); if (r < 0) return r; -#endif if (i.cmdline) { i.device = i.cmdline->device; diff --git a/src/hibernate-resume/hibernate-resume-config.h b/src/hibernate-resume/hibernate-resume-config.h index c0aa9e00ad..68ef075355 100644 --- a/src/hibernate-resume/hibernate-resume-config.h +++ b/src/hibernate-resume/hibernate-resume-config.h @@ -5,8 +5,27 @@ #include "sd-id128.h" +#include "macro.h" + typedef struct KernelHibernateLocation KernelHibernateLocation; -typedef struct EFIHibernateLocation EFIHibernateLocation; + +typedef struct EFIHibernateLocation { + char *device; + + sd_id128_t uuid; + uint64_t offset; + + char *kernel_version; + char *id; + char *image_id; + char *version_id; + char *image_version; +} EFIHibernateLocation; + +EFIHibernateLocation* efi_hibernate_location_free(EFIHibernateLocation *e); +DEFINE_TRIVIAL_CLEANUP_FUNC(EFIHibernateLocation*, efi_hibernate_location_free); + +int get_efi_hibernate_location(EFIHibernateLocation **ret); typedef struct HibernateInfo { const char *device; @@ -20,14 +39,4 @@ void hibernate_info_done(HibernateInfo *info); int acquire_hibernate_info(HibernateInfo *ret); -#if ENABLE_EFI - void compare_hibernate_location_and_warn(const HibernateInfo *info); - -#else - -static inline void compare_hibernate_location_and_warn(const HibernateInfo *info) { - return; -} - -#endif From 89c727e0f49b2d87cdef9d6ac1fa868a75ab57ba Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 31 Mar 2024 19:47:34 +0800 Subject: [PATCH 4/7] hibernate-resume: add options handling --- src/hibernate-resume/hibernate-resume.c | 86 ++++++++++++++++++++++--- 1 file changed, 76 insertions(+), 10 deletions(-) diff --git a/src/hibernate-resume/hibernate-resume.c b/src/hibernate-resume/hibernate-resume.c index 27411faa7f..cd92d8bdf1 100644 --- a/src/hibernate-resume/hibernate-resume.c +++ b/src/hibernate-resume/hibernate-resume.c @@ -1,8 +1,10 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #include +#include #include +#include "build.h" #include "devnum-util.h" #include "hibernate-resume-config.h" #include "hibernate-util.h" @@ -10,12 +12,72 @@ #include "log.h" #include "main-func.h" #include "parse-util.h" +#include "pretty-print.h" #include "static-destruct.h" +#include "terminal-util.h" static HibernateInfo arg_info = {}; STATIC_DESTRUCTOR_REGISTER(arg_info, hibernate_info_done); +static int help(void) { + _cleanup_free_ char *link = NULL; + int r; + + r = terminal_urlify_man("systemd-hibernate-resume", "8", &link); + if (r < 0) + return log_oom(); + + printf("%s [OPTIONS...] [DEVICE [OFFSET]]\n" + "\n%sInitiate resume from hibernation.%s\n\n" + " -h --help Show this help\n" + " --version Show package version\n" + "\nSee the %s for details.\n", + program_invocation_short_name, + ansi_highlight(), + ansi_normal(), + link); + + return 0; +} + +static int parse_argv(int argc, char *argv[]) { + + enum { + ARG_VERSION = 0x100, + }; + + static const struct option options[] = { + { "help", no_argument, NULL, 'h' }, + { "version", no_argument, NULL, ARG_VERSION }, + {} + }; + + int c; + + assert(argc >= 0); + assert(argv); + + while ((c = getopt_long(argc, argv, "h", options, NULL)) >= 0) + + switch (c) { + + case 'h': + return help(); + + case ARG_VERSION: + return version(); + + case '?': + return -EINVAL; + + default: + assert_not_reached(); + } + + return 1; +} + static int setup_hibernate_info_and_warn(void) { int r; @@ -38,7 +100,11 @@ static int run(int argc, char *argv[]) { log_setup(); - if (argc < 1 || argc > 3) + r = parse_argv(argc, argv); + if (r <= 0) + return r; + + if (argc - optind > 2) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "This program expects zero, one, or two arguments."); umask(0022); @@ -46,21 +112,21 @@ static int run(int argc, char *argv[]) { if (!in_initrd()) return 0; - if (argc > 1) { - arg_info.device = argv[1]; - - if (argc == 3) { - r = safe_atou64(argv[2], &arg_info.offset); - if (r < 0) - return log_error_errno(r, "Failed to parse resume offset %s: %m", argv[2]); - } - } else { + if (argc <= optind) { r = setup_hibernate_info_and_warn(); if (r <= 0) return r; if (arg_info.efi) (void) clear_efi_hibernate_location_and_warn(); + } else { + arg_info.device = ASSERT_PTR(argv[optind]); + + if (argc - optind == 2) { + r = safe_atou64(argv[optind + 1], &arg_info.offset); + if (r < 0) + return log_error_errno(r, "Failed to parse resume offset %s: %m", argv[optind + 1]); + } } if (stat(arg_info.device, &st) < 0) From 8f8e481f03c49cd2464ed0f54d5ba4da69b8796c Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 31 Mar 2024 20:30:50 +0800 Subject: [PATCH 5/7] hibernate-resume: introduce --clear for clearing hibernate storage var --- src/hibernate-resume/hibernate-resume.c | 36 ++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/src/hibernate-resume/hibernate-resume.c b/src/hibernate-resume/hibernate-resume.c index cd92d8bdf1..c6494b99a9 100644 --- a/src/hibernate-resume/hibernate-resume.c +++ b/src/hibernate-resume/hibernate-resume.c @@ -17,6 +17,7 @@ #include "terminal-util.h" static HibernateInfo arg_info = {}; +static bool arg_clear = false; STATIC_DESTRUCTOR_REGISTER(arg_info, hibernate_info_done); @@ -32,6 +33,7 @@ static int help(void) { "\n%sInitiate resume from hibernation.%s\n\n" " -h --help Show this help\n" " --version Show package version\n" + " --clear Clear hibernation storage information from EFI and exit\n" "\nSee the %s for details.\n", program_invocation_short_name, ansi_highlight(), @@ -45,11 +47,13 @@ static int parse_argv(int argc, char *argv[]) { enum { ARG_VERSION = 0x100, + ARG_CLEAR, }; static const struct option options[] = { { "help", no_argument, NULL, 'h' }, { "version", no_argument, NULL, ARG_VERSION }, + { "clear", no_argument, NULL, ARG_CLEAR }, {} }; @@ -68,6 +72,10 @@ static int parse_argv(int argc, char *argv[]) { case ARG_VERSION: return version(); + case ARG_CLEAR: + arg_clear = true; + break; + case '?': return -EINVAL; @@ -75,6 +83,10 @@ static int parse_argv(int argc, char *argv[]) { assert_not_reached(); } + if (argc > optind && arg_clear) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Extraneous arguments specified with --clear, refusing."); + return 1; } @@ -94,6 +106,24 @@ static int setup_hibernate_info_and_warn(void) { return 1; } +static int action_clear(void) { + int r; + + assert(arg_clear); + + /* Let's insist that the system identifier is verified still. After all if things don't match, + * the resume wouldn't get triggered in the first place. We should not erase the var if booted + * from LiveCD/portable systems/... */ + r = get_efi_hibernate_location(/* ret = */ NULL); + if (r <= 0) + return r; + + r = clear_efi_hibernate_location_and_warn(); + if (r > 0) + log_notice("Successfully cleared HibernateLocation EFI variable."); + return r; +} + static int run(int argc, char *argv[]) { struct stat st; int r; @@ -109,8 +139,12 @@ static int run(int argc, char *argv[]) { umask(0022); + if (arg_clear) + return action_clear(); + if (!in_initrd()) - return 0; + return log_error_errno(SYNTHETIC_ERRNO(ENOTRECOVERABLE), + "Not running in initrd, refusing to initiate resume from hibernation."); if (argc <= optind) { r = setup_hibernate_info_and_warn(); From dfad86b83807fa8696ca94982a5ba591b6d86a80 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sun, 31 Mar 2024 20:52:39 +0800 Subject: [PATCH 6/7] units: introduce systemd-hibernate-clear.service that clears stale HibernateLocation EFI variable Currently, if the HibernateLocation EFI variable exists, but we failed to resume from it, the boot carries on without clearing the stale variable. Therefore, the subsequent boots would still be waiting for the device timeout, unless the variable is purged manually. There's no point to keep trying to resume after a successful switch-root, because the hibernation image state would have been invalidated by then. OTOH, we don't want to clear the variable prematurely either, i.e. in initrd, since if the resume device is the same as root one, the boot won't succeed and the user might be able to try resuming again. So, let's introduce a unit that only runs after switch-root and clears the var. Fixes #32021 --- man/rules/meson.build | 2 +- man/systemd-hibernate-resume.service.xml | 8 ++++++++ units/meson.build | 5 +++++ units/systemd-hibernate-clear.service.in | 24 ++++++++++++++++++++++++ 4 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 units/systemd-hibernate-clear.service.in diff --git a/man/rules/meson.build b/man/rules/meson.build index e483196c5c..b67daa85fd 100644 --- a/man/rules/meson.build +++ b/man/rules/meson.build @@ -944,7 +944,7 @@ manpages = [ ['systemd-hibernate-resume-generator', '8', [], 'ENABLE_HIBERNATE'], ['systemd-hibernate-resume.service', '8', - ['systemd-hibernate-resume'], + ['systemd-hibernate-resume', 'systemd-hibernate-clear.service'], 'ENABLE_HIBERNATE'], ['systemd-homed.service', '8', ['systemd-homed'], 'ENABLE_HOMED'], ['systemd-hostnamed.service', '8', ['systemd-hostnamed'], 'ENABLE_HOSTNAMED'], diff --git a/man/systemd-hibernate-resume.service.xml b/man/systemd-hibernate-resume.service.xml index f6cdefdd3a..c0c545cc9c 100644 --- a/man/systemd-hibernate-resume.service.xml +++ b/man/systemd-hibernate-resume.service.xml @@ -17,12 +17,14 @@ systemd-hibernate-resume.service + systemd-hibernate-clear.service systemd-hibernate-resume Resume from hibernation systemd-hibernate-resume.service + systemd-hibernate-clear.service /usr/lib/systemd/systemd-hibernate-resume @@ -37,6 +39,12 @@ /sys/power/resume, along with the offset in memory pages (/sys/power/resume_offset) if supported. + The resume device node is either passed directly through arguments, or automatically acquired + from kernel command line options and/or HibernateLocation EFI variable. The latter + will normally be cleared by systemd-hibernate-resume.service on resumption. + If a stale variable is detected, it would be cleared by + systemd-hibernate-clear.service. + Failing to initiate a resume is not an error condition. It may mean that there was no resume image (e. g. if the system has been simply powered off and not hibernated). In such cases, the boot is ordinarily continued. diff --git a/units/meson.build b/units/meson.build index 6fecbea0a1..16a5564086 100644 --- a/units/meson.build +++ b/units/meson.build @@ -312,6 +312,11 @@ units = [ { 'file' : 'systemd-growfs-root.service.in' }, { 'file' : 'systemd-growfs@.service.in' }, { 'file' : 'systemd-halt.service' }, + { + 'file' : 'systemd-hibernate-clear.service.in', + 'conditions' : ['ENABLE_HIBERNATE', 'ENABLE_EFI'], + 'symlinks' : ['sysinit.target.wants/'], + }, { 'file' : 'systemd-hibernate-resume.service.in', 'conditions' : ['ENABLE_HIBERNATE'], diff --git a/units/systemd-hibernate-clear.service.in b/units/systemd-hibernate-clear.service.in new file mode 100644 index 0000000000..2e8587e692 --- /dev/null +++ b/units/systemd-hibernate-clear.service.in @@ -0,0 +1,24 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Unit] +Description=Clear Stale Hibernate Storage Info +Documentation=man:systemd-hibernate-clear.service(8) + +ConditionPathExists=/sys/firmware/efi/efivars/HibernateLocation-8cf2644b-4b0b-428f-9387-6d876050dc67 +ConditionPathExists=!/etc/initrd-release + +DefaultDependencies=no +Before=sysinit.target shutdown.target +Conflicts=shutdown.target + +[Service] +Type=oneshot +RemainAfterExit=yes +ExecStart={{LIBEXECDIR}}/systemd-hibernate-resume --clear From 5f0cd5717fd374ad5f40363e1134b86ae0e3b84a Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 3 Apr 2024 05:38:51 +0800 Subject: [PATCH 7/7] man/kernel-command-line: be clear that resumeflags= is about timeout opts only --- man/kernel-command-line.xml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/man/kernel-command-line.xml b/man/kernel-command-line.xml index 72003d86f2..2226749148 100644 --- a/man/kernel-command-line.xml +++ b/man/kernel-command-line.xml @@ -590,10 +590,9 @@ resumeflags= - Enables resume from hibernation using the specified - device and mount options. All - fstab5-like - paths are supported. For details, see + Enable resume from hibernation using the specified device and timeout options. All + fstab5-style + device identifiers are supported. For details, see systemd-hibernate-resume-generator8.