From 651b44bddaeb7d48a970b4b16f2877c37d93f1a2 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 5 Mar 2025 15:54:26 +0100 Subject: [PATCH 1/7] units: refuse manual operations on factory-reset-now.target and friends It is strictly mandatory that this is done during initial transaction, and not later when the system is already running. Hence let's refuse manual start for all of the involved units. Additionally, refuse manual stop for systemd-factory-reset-complete.service, as it flags the factory reset completion through /run/systemd/factory-reset-complete, which never gets removed for the whole boot. --- units/factory-reset-now.target | 1 + units/systemd-factory-reset-complete.service.in | 2 ++ 2 files changed, 3 insertions(+) diff --git a/units/factory-reset-now.target b/units/factory-reset-now.target index 6415cc1232e19..008d565b17c9d 100644 --- a/units/factory-reset-now.target +++ b/units/factory-reset-now.target @@ -11,3 +11,4 @@ Description=Factory Reset Execution Documentation=man:systemd.special(7) Wants=systemd-factory-reset-complete.service +RefuseManualStart=yes diff --git a/units/systemd-factory-reset-complete.service.in b/units/systemd-factory-reset-complete.service.in index 337b99d3d4286..aa0913dbe699a 100644 --- a/units/systemd-factory-reset-complete.service.in +++ b/units/systemd-factory-reset-complete.service.in @@ -15,6 +15,8 @@ Requires=factory-reset-now.target After=factory-reset-now.target Conflicts=shutdown.target Before=shutdown.target +RefuseManualStart=yes +RefuseManualStop=yes [Service] Type=oneshot From 28ac3309d75ae4eb69adbf04b23c77dfd4920e66 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 5 Mar 2025 15:59:14 +0100 Subject: [PATCH 2/7] units/meson: remove unneeded linebreak --- units/meson.build | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/units/meson.build b/units/meson.build index ae13f85adebc2..84c3757b10aea 100644 --- a/units/meson.build +++ b/units/meson.build @@ -323,9 +323,7 @@ units = [ }, { 'file' : 'systemd-creds@.service' }, { 'file' : 'systemd-exit.service' }, - { - 'file' : 'systemd-factory-reset@.service.in', - }, + { 'file' : 'systemd-factory-reset@.service.in' }, { 'file' : 'systemd-factory-reset.socket', 'symlinks' : ['sockets.target.wants/'], From ab4c84b0e9da96059bda9c9ef63fada5b5ae37a6 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 5 Mar 2025 16:06:00 +0100 Subject: [PATCH 3/7] tpm2-clear: use plain DEFINE_MAIN_FUNCTION We don't return any positive exit status. --- src/tpm2-setup/tpm2-clear.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tpm2-setup/tpm2-clear.c b/src/tpm2-setup/tpm2-clear.c index 330d5cbd59db9..3f76798a6fbc8 100644 --- a/src/tpm2-setup/tpm2-clear.c +++ b/src/tpm2-setup/tpm2-clear.c @@ -103,7 +103,7 @@ static int request_tpm2_clear(void) { if (clear == 0) { log_info("Clearing TPM2 disabled, exiting early."); - return EXIT_SUCCESS; + return 0; } /* Now issue PPI request */ @@ -131,10 +131,10 @@ static int run(int argc, char *argv[]) { * to rebuild it. */ if (arg_graceful && !tpm2_is_fully_supported()) { log_notice("No complete TPM2 support detected, exiting gracefully."); - return EXIT_SUCCESS; + return 0; } return request_tpm2_clear(); } -DEFINE_MAIN_FUNCTION_WITH_POSITIVE_FAILURE(run); +DEFINE_MAIN_FUNCTION(run); From 5c7b3335dbd87f5bef335efa553a1173a584df1a Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 5 Mar 2025 16:07:50 +0100 Subject: [PATCH 4/7] tpm2-clear: make getenv() failure fatal, correct one log level This operation is destructive, and we bail if the proc_cmdline_get_bool() call below fails already. Better be safe than sorry. --- src/tpm2-setup/tpm2-clear.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tpm2-setup/tpm2-clear.c b/src/tpm2-setup/tpm2-clear.c index 3f76798a6fbc8..0dcee96a07ad2 100644 --- a/src/tpm2-setup/tpm2-clear.c +++ b/src/tpm2-setup/tpm2-clear.c @@ -88,7 +88,7 @@ static int request_tpm2_clear(void) { r = secure_getenv_bool("SYSTEMD_TPM2_ALLOW_CLEAR"); if (r < 0 && r != -ENXIO) - log_warning_errno(r, "Failed to parse $SYSTEMD_TPM2_ALLOW_CLEAR, ignoring: %m"); + return log_error_errno(r, "Failed to parse $SYSTEMD_TPM2_ALLOW_CLEAR: %m"); if (r >= 0) clear = r; @@ -96,7 +96,7 @@ static int request_tpm2_clear(void) { bool b; r = proc_cmdline_get_bool("systemd.tpm2_allow_clear", /* flags= */ 0, &b); if (r < 0) - return log_debug_errno(r, "Failed to parse systemd.tpm2_allow_clear kernel command line argument: %m"); + return log_error_errno(r, "Failed to parse systemd.tpm2_allow_clear kernel command line argument: %m"); if (r > 0) clear = b; } From f1d790a18bef5ceaa9dc151d7a5f80a128b3ab0c Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 5 Mar 2025 16:13:55 +0100 Subject: [PATCH 5/7] tpm2-clear: make it clear that we default to true for systemd.tpm2_allow_clear --- src/tpm2-setup/tpm2-clear.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/tpm2-setup/tpm2-clear.c b/src/tpm2-setup/tpm2-clear.c index 0dcee96a07ad2..5c4fc2a5dee3b 100644 --- a/src/tpm2-setup/tpm2-clear.c +++ b/src/tpm2-setup/tpm2-clear.c @@ -94,14 +94,15 @@ static int request_tpm2_clear(void) { if (clear < 0) { bool b; - r = proc_cmdline_get_bool("systemd.tpm2_allow_clear", /* flags= */ 0, &b); + r = proc_cmdline_get_bool("systemd.tpm2_allow_clear", PROC_CMDLINE_TRUE_WHEN_MISSING, &b); if (r < 0) return log_error_errno(r, "Failed to parse systemd.tpm2_allow_clear kernel command line argument: %m"); - if (r > 0) - clear = b; + clear = b; } - if (clear == 0) { + assert(clear >= 0); + + if (!clear) { log_info("Clearing TPM2 disabled, exiting early."); return 0; } From 97be702ffcbe310c0660052ee53b9ed45d5d61ca Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 5 Mar 2025 17:01:04 +0100 Subject: [PATCH 6/7] factory-reset-tool: error out if we can't cancel pending reset First of all, it seems very unlikely that we'd be in the pending state if not booted via EFI in the first place. Moreover, the operation didn't work out, hence let's not spurious report success. --- src/factory-reset/factory-reset-tool.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/factory-reset/factory-reset-tool.c b/src/factory-reset/factory-reset-tool.c index 8bf573460114d..bfb1a8d26fefc 100644 --- a/src/factory-reset/factory-reset-tool.c +++ b/src/factory-reset/factory-reset-tool.c @@ -209,12 +209,9 @@ static int verb_cancel(int argc, char *argv[], void *userdata) { return 0; } - if (!is_efi_boot()) { - if (!arg_quiet) - log_info("Not an EFI boot, cannot remove FactoryResetMode EFI variable, not cancelling."); - - return 0; - } + if (!is_efi_boot()) + return log_error_errno(SYNTHETIC_ERRNO(ENOTRECOVERABLE), + "Not an EFI boot, cannot remove FactoryResetMode EFI variable, not cancelling."); r = efi_set_variable(EFI_SYSTEMD_VARIABLE_STR("FactoryResetRequest"), /* value= */ NULL, /* size= */ 0); if (r < 0) From 911de19c7262cf28428fb8a31318ff92bbcd5adb Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Wed, 5 Mar 2025 16:25:48 +0100 Subject: [PATCH 7/7] hibernate-resume-config: log louder on invalid kernel version/os-release id Prompted by 45623d4ad63243f96614013600167cf080efc353 We do make use of the os-release ids to determine whether to initial resume if they're present, hence log at warning level if invalid. While at it, raise the level for the kernel version too, which is generally interesting to the user if something goes wrong. --- src/hibernate-resume/hibernate-resume-config.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/hibernate-resume/hibernate-resume-config.c b/src/hibernate-resume/hibernate-resume-config.c index d93919956b48d..a11c68ae4a93a 100644 --- a/src/hibernate-resume/hibernate-resume-config.c +++ b/src/hibernate-resume/hibernate-resume-config.c @@ -143,11 +143,11 @@ static bool validate_efi_hibernate_location(EFIHibernateLocation *e) { int get_efi_hibernate_location(EFIHibernateLocation **ret) { #if ENABLE_EFI static const sd_json_dispatch_field dispatch_table[] = { - { "uuid", SD_JSON_VARIANT_STRING, sd_json_dispatch_id128, offsetof(EFIHibernateLocation, uuid), SD_JSON_MANDATORY }, - { "offset", _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_uint64, offsetof(EFIHibernateLocation, offset), SD_JSON_MANDATORY }, - { "kernelVersion", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(EFIHibernateLocation, kernel_version), SD_JSON_PERMISSIVE|SD_JSON_DEBUG }, - { "osReleaseId", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(EFIHibernateLocation, id), SD_JSON_PERMISSIVE|SD_JSON_DEBUG }, - { "osReleaseImageId", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(EFIHibernateLocation, image_id), SD_JSON_PERMISSIVE|SD_JSON_DEBUG }, + { "uuid", SD_JSON_VARIANT_STRING, sd_json_dispatch_id128, offsetof(EFIHibernateLocation, uuid), SD_JSON_MANDATORY }, + { "offset", _SD_JSON_VARIANT_TYPE_INVALID, sd_json_dispatch_uint64, offsetof(EFIHibernateLocation, offset), SD_JSON_MANDATORY }, + { "kernelVersion", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(EFIHibernateLocation, kernel_version), SD_JSON_PERMISSIVE }, + { "osReleaseId", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(EFIHibernateLocation, id), SD_JSON_PERMISSIVE }, + { "osReleaseImageId", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(EFIHibernateLocation, image_id), SD_JSON_PERMISSIVE }, { "osReleaseVersionId", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(EFIHibernateLocation, version_id), SD_JSON_PERMISSIVE|SD_JSON_DEBUG }, { "osReleaseImageVersion", SD_JSON_VARIANT_STRING, sd_json_dispatch_string, offsetof(EFIHibernateLocation, image_version), SD_JSON_PERMISSIVE|SD_JSON_DEBUG }, {},