From 6eea6e30ab99f8a3eebaa338c02803f33ab5b855 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 25 Jun 2020 15:24:06 +0200 Subject: [PATCH 1/3] tmpfile-util: typo fixes --- src/basic/tmpfile-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/tmpfile-util.c b/src/basic/tmpfile-util.c index 9cbca312fc..a49f7eee70 100644 --- a/src/basic/tmpfile-util.c +++ b/src/basic/tmpfile-util.c @@ -257,7 +257,7 @@ int open_tmpfile_linkable(const char *target, int flags, char **ret_path) { assert((flags & O_EXCL) == 0); /* Creates a temporary file, that shall be renamed to "target" later. If possible, this uses O_TMPFILE – in - * which case "ret_path" will be returned as NULL. If not possible a the tempoary path name used is returned in + * which case "ret_path" will be returned as NULL. If not possible the temporary path name used is returned in * "ret_path". Use link_tmpfile() below to rename the result after writing the file in full. */ fd = open_parent(target, O_TMPFILE|flags, 0640); From e8df4eee65643f9b0806763857a9f265e3d764ee Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 25 Jun 2020 15:24:25 +0200 Subject: [PATCH 2/3] efi-loader: cache LoaderConfigTimeoutOneShot too The data from this EFI variable is exposed as dbus property, and gdbus clients are happy to issue GetAllProperties() as if it was free. Hence make sure it's actually free and cache LoaderConfigTimeoutOneShot, since it's easy. --- src/login/logind-dbus.c | 16 ++-------------- src/shared/efi-loader.c | 38 ++++++++++++++++++++++++++++++++++++++ src/shared/efi-loader.h | 6 ++++++ 3 files changed, 46 insertions(+), 14 deletions(-) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 7fed32c3b4..d8ae0287ec 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -2751,8 +2751,6 @@ static int property_get_reboot_to_boot_loader_menu( r = getenv_bool("SYSTEMD_REBOOT_TO_BOOT_LOADER_MENU"); if (r == -ENXIO) { - _cleanup_free_ char *v = NULL; - /* EFI case: returns the current value of LoaderConfigTimeoutOneShot. Three cases are distuingished: * * 1. Variable not set, boot into boot loader menu is not enabled (we return UINT64_MAX to the user) @@ -2760,20 +2758,10 @@ static int property_get_reboot_to_boot_loader_menu( * 3. Variable set to numeric value formatted in ASCII, boot into boot loader menu with the specified timeout in seconds */ - r = efi_get_variable_string(EFI_VENDOR_LOADER, "LoaderConfigTimeoutOneShot", &v); + r = efi_loader_get_config_timeout_one_shot(&x); if (r < 0) { if (r != -ENOENT) - log_warning_errno(r, "Failed to read LoaderConfigTimeoutOneShot variable: %m"); - } else { - uint64_t sec; - - r = safe_atou64(v, &sec); - if (r < 0) - log_warning_errno(r, "Failed to parse LoaderConfigTimeoutOneShot value '%s': %m", v); - else if (sec > (USEC_INFINITY / USEC_PER_SEC)) - log_warning("LoaderConfigTimeoutOneShot too large, ignoring: %m"); - else - x = sec * USEC_PER_SEC; /* return in µs */ + log_warning_errno(r, "Failed to read LoaderConfigTimeoutOneShot variable, ignoring: %m"); } } else if (r < 0) diff --git a/src/shared/efi-loader.c b/src/shared/efi-loader.c index b6ad43b856..ce9e78b274 100644 --- a/src/shared/efi-loader.c +++ b/src/shared/efi-loader.c @@ -734,3 +734,41 @@ char *efi_tilt_backslashes(char *s) { return s; } + +int efi_loader_get_config_timeout_one_shot(usec_t *ret) { + _cleanup_free_ char *v = NULL, *fn = NULL; + static struct stat cache_stat = {}; + struct stat new_stat; + static usec_t cache; + uint64_t sec; + int r; + + assert(ret); + + fn = efi_variable_path(EFI_VENDOR_LOADER, "LoaderConfigTimeoutOneShot"); + if (!fn) + return -ENOMEM; + + /* stat() the EFI variable, to see if the mtime changed. If it did we need to cache again. */ + if (stat(fn, &new_stat) < 0) + return -errno; + + if (stat_inode_unmodified(&new_stat, &cache_stat)) { + *ret = cache; + return 0; + } + + r = efi_get_variable_string(EFI_VENDOR_LOADER, "LoaderConfigTimeoutOneShot", &v); + if (r < 0) + return r; + + r = safe_atou64(v, &sec); + if (r < 0) + return r; + if (sec > USEC_INFINITY / USEC_PER_SEC) + return -ERANGE; + + cache_stat = new_stat; + *ret = cache = sec * USEC_PER_SEC; /* return in µs */ + return 0; +} diff --git a/src/shared/efi-loader.h b/src/shared/efi-loader.h index 96208d25bf..98bb57ecbd 100644 --- a/src/shared/efi-loader.h +++ b/src/shared/efi-loader.h @@ -23,6 +23,8 @@ int efi_loader_get_entries(char ***ret); int efi_loader_get_features(uint64_t *ret); +int efi_loader_get_config_timeout_one_shot(usec_t *ret); + #else static inline int efi_reboot_to_firmware_supported(void) { @@ -77,6 +79,10 @@ static inline int efi_loader_get_features(uint64_t *ret) { return -EOPNOTSUPP; } +static inline int efi_loader_get_config_timeout_one_shot(usec_t *ret) { + return -EOPNOTSUPP; +} + #endif bool efi_loader_entry_name_valid(const char *s); From af2697e83d0b176371f94223d66dbfd6791babe3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 25 Jun 2020 16:25:41 +0200 Subject: [PATCH 3/3] logind: also cache LoaderEntryOneShot EFI variable With this we are now caching all EFI variables that we expose as property in logind. Thus a client invoking GetAllProperties() should only trgger a single read of each variable, but never repeated ones. Obsoletes: #16190 Fixes: #14828 --- src/login/logind-dbus.c | 25 +++++++++++++------------ src/login/logind.c | 1 + src/login/logind.h | 3 +++ src/shared/efi-loader.c | 32 ++++++++++++++++++++++++++++++++ src/shared/efi-loader.h | 7 +++++++ 5 files changed, 56 insertions(+), 12 deletions(-) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index d8ae0287ec..079338fbb5 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -2920,24 +2920,25 @@ static int property_get_reboot_to_boot_loader_entry( sd_bus_error *error) { _cleanup_free_ char *v = NULL; + Manager *m = userdata; + const char *x = NULL; int r; assert(bus); assert(reply); - assert(userdata); + assert(m); r = getenv_bool("SYSTEMD_REBOOT_TO_BOOT_LOADER_ENTRY"); if (r == -ENXIO) { /* EFI case: let's read the LoaderEntryOneShot variable */ - r = efi_get_variable_string(EFI_VENDOR_LOADER, "LoaderEntryOneShot", &v); + r = efi_loader_update_entry_one_shot_cache(&m->efi_loader_entry_one_shot, &m->efi_loader_entry_one_shot_stat); if (r < 0) { if (r != -ENOENT) - log_warning_errno(r, "Failed to read LoaderEntryOneShot variable: %m"); - } else if (!efi_loader_entry_name_valid(v)) { - log_warning("LoaderEntryOneShot contains invalid entry name '%s', ignoring.", v); - v = mfree(v); - } + log_warning_errno(r, "Failed to read LoaderEntryOneShot variable, ignoring: %m"); + } else + x = m->efi_loader_entry_one_shot; + } else if (r < 0) log_warning_errno(r, "Failed to parse $SYSTEMD_REBOOT_TO_BOOT_LOADER_ENTRY: %m"); else if (r > 0) { @@ -2947,14 +2948,14 @@ static int property_get_reboot_to_boot_loader_entry( r = read_one_line_file("/run/systemd/reboot-to-boot-loader-entry", &v); if (r < 0) { if (r != -ENOENT) - log_warning_errno(r, "Failed to read /run/systemd/reboot-to-boot-loader-entry: %m"); - } else if (!efi_loader_entry_name_valid(v)) { + log_warning_errno(r, "Failed to read /run/systemd/reboot-to-boot-loader-entry, ignoring: %m"); + } else if (!efi_loader_entry_name_valid(v)) log_warning("/run/systemd/reboot-to-boot-loader-entry is not valid, ignoring."); - v = mfree(v); - } + else + x = v; } - return sd_bus_message_append(reply, "s", v); + return sd_bus_message_append(reply, "s", x); } static int boot_loader_entry_exists(Manager *m, const char *id) { diff --git a/src/login/logind.c b/src/login/logind.c index 377fba25cf..18caae3487 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -169,6 +169,7 @@ static Manager* manager_unref(Manager *m) { free(m->action_job); strv_free(m->efi_boot_loader_entries); + free(m->efi_loader_entry_one_shot); return mfree(m); } diff --git a/src/login/logind.h b/src/login/logind.h index 7dbf0c28e1..e64ecce8e2 100644 --- a/src/login/logind.h +++ b/src/login/logind.h @@ -126,6 +126,9 @@ struct Manager { char **efi_boot_loader_entries; bool efi_boot_loader_entries_set; + + char *efi_loader_entry_one_shot; + struct stat efi_loader_entry_one_shot_stat; }; void manager_reset_config(Manager *m); diff --git a/src/shared/efi-loader.c b/src/shared/efi-loader.c index ce9e78b274..00e572c423 100644 --- a/src/shared/efi-loader.c +++ b/src/shared/efi-loader.c @@ -772,3 +772,35 @@ int efi_loader_get_config_timeout_one_shot(usec_t *ret) { *ret = cache = sec * USEC_PER_SEC; /* return in µs */ return 0; } + +int efi_loader_update_entry_one_shot_cache(char **cache, struct stat *cache_stat) { + _cleanup_free_ char *fn = NULL, *v = NULL; + struct stat new_stat; + int r; + + assert(cache); + assert(cache_stat); + + fn = efi_variable_path(EFI_VENDOR_LOADER, "LoaderEntryOneShot"); + if (!fn) + return -ENOMEM; + + /* stat() the EFI variable, to see if the mtime changed. If it did we need to cache again. */ + if (stat(fn, &new_stat) < 0) + return -errno; + + if (stat_inode_unmodified(&new_stat, cache_stat)) + return 0; + + r = efi_get_variable_string(EFI_VENDOR_LOADER, "LoaderEntryOneShot", &v); + if (r < 0) + return r; + + if (!efi_loader_entry_name_valid(v)) + return -EINVAL; + + *cache_stat = new_stat; + free_and_replace(*cache, v); + + return 0; +} diff --git a/src/shared/efi-loader.h b/src/shared/efi-loader.h index 98bb57ecbd..171274a0e3 100644 --- a/src/shared/efi-loader.h +++ b/src/shared/efi-loader.h @@ -3,6 +3,8 @@ #include "efivars.h" +#include + #if ENABLE_EFI int efi_reboot_to_firmware_supported(void); @@ -24,6 +26,7 @@ int efi_loader_get_entries(char ***ret); int efi_loader_get_features(uint64_t *ret); int efi_loader_get_config_timeout_one_shot(usec_t *ret); +int efi_loader_update_entry_one_shot_cache(char **cache, struct stat *cache_stat); #else @@ -83,6 +86,10 @@ static inline int efi_loader_get_config_timeout_one_shot(usec_t *ret) { return -EOPNOTSUPP; } +static inline int efi_loader_update_entry_one_shot_cache(char **cache, struct stat *cache_stat) { + return -EOPNOTSUPP; +} + #endif bool efi_loader_entry_name_valid(const char *s);