From abd17aa3f492b211da37419c90b02c22456a8b74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 21 Nov 2025 12:32:18 +0100 Subject: [PATCH 1/2] bootctl: rework setting of menu timeout variables menu-force and menu-hidden were added in 97f077df052c75224dcc73375bfaaa69af6a1c26, menu-disable was added in 6efdd7fec5106205240332bd3b7fd2f93d4d9d4c, a year later. So we can assume that if the feature flag is set, the other string values are supported too. The comment that there's no way check that was added later in 5b45fad4fcfa2dd81f25b13fe8d7717f62fa5843, but it was incorrect even at that time. Fixes https://github.com/systemd/systemd/issues/39167. As described in the issue, we documented various string values in the BLI, but bootctl didn't use the string values. At the time menu-force and menu-hidden were added, using numerical values for compatibility made sense. But that stopped being needed when a string value that didn't have a strictly equivalent numerical value and a feature flag were added. When converting a large number to menu-force, message is downgraded to debug, since the severity of the issue is very minor. Debug messages are added in other places when the requested setting is modified too. --- src/bootctl/bootctl-set-efivar.c | 66 ++++++++++++++++++++------------ 1 file changed, 42 insertions(+), 24 deletions(-) diff --git a/src/bootctl/bootctl-set-efivar.c b/src/bootctl/bootctl-set-efivar.c index b102d6600e..360664b879 100644 --- a/src/bootctl/bootctl-set-efivar.c +++ b/src/bootctl/bootctl-set-efivar.c @@ -15,51 +15,69 @@ #include "virt.h" static int parse_timeout(const char *arg1, char16_t **ret_timeout, size_t *ret_timeout_size) { - char utf8[DECIMAL_STR_MAX(usec_t)]; - char16_t *encoded; + char buf[DECIMAL_STR_MAX(usec_t)]; usec_t timeout; - bool menu_disabled = false; + uint64_t loader_features = 0; int r; assert(arg1); assert(ret_timeout); assert(ret_timeout_size); - assert_cc(STRLEN("menu-disabled") < ELEMENTSOF(utf8)); + assert_cc(STRLEN("menu-force") < ELEMENTSOF(buf)); + assert_cc(STRLEN("menu-hidden") < ELEMENTSOF(buf)); + assert_cc(STRLEN("menu-disabled") < ELEMENTSOF(buf)); - /* Note: Since there is no way to query if the bootloader supports the string tokens, we explicitly - * set their numerical value(s) instead. This means that some of the sd-boot internal ABI has leaked - * although the ship has sailed and the side-effects are self-contained. + /* Use feature EFI_LOADER_FEATURE_MENU_DISABLE as a mark that the boot loader supports the other + * string values too. When unsupported, convert to the timeout with the closest meaning. */ - if (streq(arg1, "menu-force")) - timeout = USEC_INFINITY; - else if (streq(arg1, "menu-hidden")) - timeout = 0; - else if (streq(arg1, "menu-disabled")) { - uint64_t loader_features = 0; + if (streq(arg1, "menu-force")) { (void) efi_loader_get_features(&loader_features); + + if (!(loader_features & EFI_LOADER_FEATURE_MENU_DISABLE)) { + log_debug("Using maximum timeout instead of '%s'.", arg1); + timeout = USEC_INFINITY; + arg1 = NULL; + } + + } else if (streq(arg1, "menu-hidden")) { + (void) efi_loader_get_features(&loader_features); + + if (!(loader_features & EFI_LOADER_FEATURE_MENU_DISABLE)) { + log_debug("Using zero timeout instead of '%s'.", arg1); + timeout = 0; + arg1 = NULL; /* replace the arg by printed timeout value later */ + } + + } else if (streq(arg1, "menu-disabled")) { + (void) efi_loader_get_features(&loader_features); + if (!(loader_features & EFI_LOADER_FEATURE_MENU_DISABLE)) { if (arg_graceful() == ARG_GRACEFUL_NO) - return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "Loader does not support 'menu-disabled'."); - - log_warning("Loader does not support 'menu-disabled', setting anyway."); + return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), + "Loader does not support '%s'.", arg1); + log_warning("Using zero timeout instead of '%s'.", arg1); + timeout = 0; + arg1 = NULL; } - menu_disabled = true; + } else { r = parse_time(arg1, &timeout, USEC_PER_SEC); if (r < 0) return log_error_errno(r, "Failed to parse timeout '%s': %m", arg1); - if (timeout != USEC_INFINITY && timeout > UINT32_MAX * USEC_PER_SEC) - log_warning("Timeout is too long and will be treated as 'menu-force' instead."); + + assert_cc(USEC_INFINITY > UINT32_MAX * USEC_PER_SEC); + if (timeout > UINT32_MAX * USEC_PER_SEC && timeout != USEC_INFINITY) + log_debug("Timeout is too long and will be clamped to maximum timeout."); + + arg1 = NULL; } - if (menu_disabled) - xsprintf(utf8, "menu-disabled"); - else - xsprintf(utf8, USEC_FMT, MIN(timeout / USEC_PER_SEC, UINT32_MAX)); + if (!arg1) + xsprintf(buf, USEC_FMT, MIN(timeout / USEC_PER_SEC, UINT32_MAX)); - encoded = utf8_to_utf16(utf8, SIZE_MAX); + char16_t *encoded = utf8_to_utf16(arg1 ?: buf, SIZE_MAX); if (!encoded) return log_oom(); From 1d26e7c1d58e5e30742d95a1adea3dd9f4983f67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 21 Nov 2025 12:53:20 +0100 Subject: [PATCH 2/2] bootctl: round the timeout up I think this is better: if I specify 0.5s, I'd be suprised if the menu didn't show up at all. --- src/bootctl/bootctl-set-efivar.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/bootctl/bootctl-set-efivar.c b/src/bootctl/bootctl-set-efivar.c index 360664b879..82770dd6c6 100644 --- a/src/bootctl/bootctl-set-efivar.c +++ b/src/bootctl/bootctl-set-efivar.c @@ -74,8 +74,10 @@ static int parse_timeout(const char *arg1, char16_t **ret_timeout, size_t *ret_t arg1 = NULL; } - if (!arg1) - xsprintf(buf, USEC_FMT, MIN(timeout / USEC_PER_SEC, UINT32_MAX)); + if (!arg1) { + timeout = DIV_ROUND_UP(timeout, USEC_PER_SEC); + xsprintf(buf, USEC_FMT, MIN(timeout, UINT32_MAX)); + } char16_t *encoded = utf8_to_utf16(arg1 ?: buf, SIZE_MAX); if (!encoded)