From 6db980988fd31559828822180518613f0bdc29bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 26 Sep 2022 14:22:49 +0200 Subject: [PATCH 1/2] tmpfiles: fix detection of unitialized-/etc In 6ec4c852c910b1aca649e87ba3143841334f01fa I changed specifier_machine_id() to return EUNATCH, but the code catching in tmpfiles wasn't adjusted. Instead of translating errors at various levels, let the "original" errno value (whatever was returned by the low-level reading function) propagate all the way to the logging function, and only check which errnos to suppress there. This makes it easier to follow the flow of data through all the layers. Also, we don't need to provide wrapper functions for each place where we want to do the supression. The common set of errnos that have similar meaning are are caught using ERRNO_IS_NOINFO(). It is more managable to use a wider net than to figure out which errors could be returned in specific cases. --- src/tmpfiles/tmpfiles.c | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 705721ffbd..6cd19d929b 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -225,17 +225,12 @@ static const char *const creation_mode_verb_table[_CREATION_MODE_MAX] = { DEFINE_PRIVATE_STRING_TABLE_LOOKUP_TO_STRING(creation_mode_verb, CreationMode); -static int specifier_machine_id_safe(char specifier, const void *data, const char *root, const void *userdata, char **ret) { - int r; - - /* If /etc/machine_id is missing or empty (e.g. in a chroot environment) return a recognizable error - * so that the caller can skip the rule gracefully. */ - - r = specifier_machine_id(specifier, data, root, userdata, ret); - if (IN_SET(r, -ENOENT, -ENOMEDIUM)) - return -ENXIO; - - return r; +/* Different kinds of errors that mean that information is not available in the environment. */ +static inline bool ERRNO_IS_NOINFO(int r) { + return IN_SET(abs(r), + EUNATCH, /* os-release or machine-id missing */ + ENOMEDIUM, /* machine-id or another file empty */ + ENXIO); /* env var is unset */ } static int specifier_directory(char specifier, const void *data, const char *root, const void *userdata, char **ret) { @@ -320,15 +315,15 @@ static int user_config_paths(char*** ret) { return r; r = xdg_user_config_dir(&persistent_config, "/user-tmpfiles.d"); - if (r < 0 && r != -ENXIO) + if (r < 0 && !ERRNO_IS_NOINFO(r)) return r; r = xdg_user_runtime_dir(&runtime_config, "/user-tmpfiles.d"); - if (r < 0 && r != -ENXIO) + if (r < 0 && !ERRNO_IS_NOINFO(r)) return r; r = xdg_user_data_dir(&data_home, "/user-tmpfiles.d"); - if (r < 0 && r != -ENXIO) + if (r < 0 && !ERRNO_IS_NOINFO(r)) return r; r = strv_extend_strv_concat(&res, config_dirs, "/user-tmpfiles.d"); @@ -3151,7 +3146,7 @@ static int parse_line( { 'B', specifier_os_build_id, NULL }, { 'H', specifier_hostname, NULL }, { 'l', specifier_short_hostname, NULL }, - { 'm', specifier_machine_id_safe, NULL }, + { 'm', specifier_machine_id, NULL }, { 'o', specifier_os_id, NULL }, { 'v', specifier_kernel_release, NULL }, { 'w', specifier_os_version_id, NULL }, @@ -3231,7 +3226,7 @@ static int parse_line( i.try_replace = try_replace; r = specifier_printf(path, PATH_MAX-1, specifier_table, arg_root, NULL, &i.path); - if (r == -ENXIO) + if (ERRNO_IS_NOINFO(r)) return log_unresolvable_specifier(fname, line); if (r < 0) { if (IN_SET(r, -EINVAL, -EBADSLT)) @@ -3419,7 +3414,7 @@ static int parse_line( if (!unbase64) { /* Do specifier expansion except if base64 mode is enabled */ r = specifier_expansion_from_arg(specifier_table, &i); - if (r == -ENXIO) + if (ERRNO_IS_NOINFO(r)) return log_unresolvable_specifier(fname, line); if (r < 0) { if (IN_SET(r, -EINVAL, -EBADSLT)) @@ -3437,7 +3432,8 @@ static int parse_line( r = read_credential(i.argument, &i.binary_argument, &i.binary_argument_size); if (IN_SET(r, -ENXIO, -ENOENT)) { /* Silently skip over lines that have no credentials passed */ - log_syntax(NULL, LOG_INFO, fname, line, 0, "Credential '%s' not specified, skipping line.", i.argument); + log_syntax(NULL, LOG_INFO, fname, line, 0, + "Credential '%s' not specified, skipping line.", i.argument); return 0; } if (r < 0) From 1401882fdd0ac5c024b2c50ef584c1e06c911c0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 26 Sep 2022 14:46:08 +0200 Subject: [PATCH 2/2] tmpfiles: downgrade message about unitialized-/etc If we're running with --root, or in a chroot (*), it's expected that machine-id and other specifiers will be unresolvable, so downgrade the warning to debug. Fixes #24655. (*) sd_booted() in principle means more than that, but nowadays systemd dominates and those others things are infrequently seen. --- src/tmpfiles/tmpfiles.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 6cd19d929b..020219b176 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -69,6 +69,7 @@ #include "terminal-util.h" #include "umask-util.h" #include "user-util.h" +#include "virt.h" /* This reads all files listed in /etc/tmpfiles.d/?*.conf and creates * them in the file system. This is intended to be used to create @@ -285,20 +286,28 @@ static int specifier_directory(char specifier, const void *data, const char *roo static int log_unresolvable_specifier(const char *filename, unsigned line) { static bool notified = false; - /* In system mode, this is called when /etc is not fully initialized (e.g. - * in a chroot environment) where some specifiers are unresolvable. In user - * mode, this is called when some variables are not defined. These cases are - * not considered as an error so log at LOG_NOTICE only for the first time - * and then downgrade this to LOG_DEBUG for the rest. */ + /* In system mode, this is called when /etc is not fully initialized and some specifiers are + * unresolvable. In user mode, this is called when some variables are not defined. These cases are + * not considered a fatal error, so log at LOG_NOTICE only for the first time and then downgrade this + * to LOG_DEBUG for the rest. + * + * If we're running in a chroot (--root was used or sd_booted() reports that systemd is not running), + * always use LOG_DEBUG. We may be called to initialize a chroot before booting and there is no + * expectation that machine-id and other files will be populated. + */ + + int log_level = notified || arg_root || running_in_chroot() > 0 ? + LOG_DEBUG : LOG_NOTICE; log_syntax(NULL, - notified ? LOG_DEBUG : LOG_NOTICE, + log_level, filename, line, 0, - "Failed to resolve specifier: %s, skipping", + "Failed to resolve specifier: %s, skipping.", arg_user ? "Required $XDG_... variable not defined" : "uninitialized /etc/ detected"); if (!notified) - log_notice("All rules containing unresolvable specifiers will be skipped."); + log_full(log_level, + "All rules containing unresolvable specifiers will be skipped."); notified = true; return 0;