From 2230e8f29ddb018697b9c3d13c1f522c62976e70 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 23 Apr 2020 14:49:14 +0200 Subject: [PATCH 1/5] sysusers: add accidentally forgotten 'return' --- src/sysusers/sysusers.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index c3e5457fc1..a06eafae22 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -1443,7 +1443,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { if (name) { r = specifier_printf(name, specifier_table, NULL, &resolved_name); if (r < 0) - log_error_errno(r, "[%s:%u] Failed to replace specifiers: %s", fname, line, name); + return log_error_errno(r, "[%s:%u] Failed to replace specifiers: %s", fname, line, name); if (!valid_user_group_name(resolved_name, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), From 0f7e4b2888840e7e6a702aa8ce5d3e3b2bc077ef Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 23 Apr 2020 14:50:07 +0200 Subject: [PATCH 2/5] sysusers,tmpfiles: always mention error when failing to replace specifiers --- src/sysusers/sysusers.c | 10 +++++----- src/tmpfiles/tmpfiles.c | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/sysusers/sysusers.c b/src/sysusers/sysusers.c index a06eafae22..f0359e3019 100644 --- a/src/sysusers/sysusers.c +++ b/src/sysusers/sysusers.c @@ -1443,7 +1443,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { if (name) { r = specifier_printf(name, specifier_table, NULL, &resolved_name); if (r < 0) - return log_error_errno(r, "[%s:%u] Failed to replace specifiers: %s", fname, line, name); + return log_error_errno(r, "[%s:%u] Failed to replace specifiers in '%s': %m", fname, line, name); if (!valid_user_group_name(resolved_name, 0)) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), @@ -1458,7 +1458,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { if (id) { r = specifier_printf(id, specifier_table, NULL, &resolved_id); if (r < 0) - return log_error_errno(r, "[%s:%u] Failed to replace specifiers: %s", + return log_error_errno(r, "[%s:%u] Failed to replace specifiers in '%s': %m", fname, line, name); } @@ -1469,7 +1469,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { if (description) { r = specifier_printf(description, specifier_table, NULL, &resolved_description); if (r < 0) - return log_error_errno(r, "[%s:%u] Failed to replace specifiers: %s", + return log_error_errno(r, "[%s:%u] Failed to replace specifiers in '%s': %m", fname, line, description); if (!valid_gecos(resolved_description)) @@ -1485,7 +1485,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { if (home) { r = specifier_printf(home, specifier_table, NULL, &resolved_home); if (r < 0) - return log_error_errno(r, "[%s:%u] Failed to replace specifiers: %s", + return log_error_errno(r, "[%s:%u] Failed to replace specifiers in '%s': %m", fname, line, home); if (!valid_home(resolved_home)) @@ -1501,7 +1501,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer) { if (shell) { r = specifier_printf(shell, specifier_table, NULL, &resolved_shell); if (r < 0) - return log_error_errno(r, "[%s:%u] Failed to replace specifiers: %s", + return log_error_errno(r, "[%s:%u] Failed to replace specifiers in '%s': %m", fname, line, shell); if (!valid_shell(resolved_shell)) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 6ab30cdecf..6f0dc2426e 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -2556,7 +2556,7 @@ static int parse_line(const char *fname, unsigned line, const char *buffer, bool if (r < 0) { if (IN_SET(r, -EINVAL, -EBADSLT)) *invalid_config = true; - return log_error_errno(r, "[%s:%u] Failed to replace specifiers: %s", fname, line, path); + return log_error_errno(r, "[%s:%u] Failed to replace specifiers in '%s': %m", fname, line, path); } r = patch_var_run(fname, line, &i.path); From 6d965610bde404f60c43accb12f570ab75d4195e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 23 Apr 2020 14:50:53 +0200 Subject: [PATCH 3/5] stat-util: no need to open a file to check fs type --- src/basic/stat-util.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/basic/stat-util.c b/src/basic/stat-util.c index 5412ccbf7d..6a48af23ae 100644 --- a/src/basic/stat-util.c +++ b/src/basic/stat-util.c @@ -178,13 +178,12 @@ int fd_is_fs_type(int fd, statfs_f_type_t magic_value) { } int path_is_fs_type(const char *path, statfs_f_type_t magic_value) { - _cleanup_close_ int fd = -1; + struct statfs s; - fd = open(path, O_RDONLY|O_CLOEXEC|O_NOCTTY|O_PATH); - if (fd < 0) + if (statfs(path, &s) < 0) return -errno; - return fd_is_fs_type(fd, magic_value); + return is_fs_type(&s, magic_value); } bool is_temporary_fs(const struct statfs *s) { From 883fff25f4841bcaec88ef49078b3512ce6eb07c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 23 Apr 2020 14:51:08 +0200 Subject: [PATCH 4/5] stat-util: add simpler helper for checking if /proc/ is mounted --- src/basic/stat-util.c | 12 ++++++++++++ src/basic/stat-util.h | 2 ++ 2 files changed, 14 insertions(+) diff --git a/src/basic/stat-util.c b/src/basic/stat-util.c index 6a48af23ae..1f3de56cf9 100644 --- a/src/basic/stat-util.c +++ b/src/basic/stat-util.c @@ -376,3 +376,15 @@ int device_path_parse_major_minor(const char *path, mode_t *ret_mode, dev_t *ret return 0; } + +int proc_mounted(void) { + int r; + + /* A quick check of procfs is properly mounted */ + + r = path_is_fs_type("/proc/", PROC_SUPER_MAGIC); + if (r == -ENOENT) /* not mounted at all */ + return false; + + return r; +} diff --git a/src/basic/stat-util.h b/src/basic/stat-util.h index 7824af3500..8160748312 100644 --- a/src/basic/stat-util.h +++ b/src/basic/stat-util.h @@ -87,3 +87,5 @@ int fd_verify_directory(int fd); int device_path_make_major_minor(mode_t mode, dev_t devno, char **ret); int device_path_make_canonical(mode_t mode, dev_t devno, char **ret); int device_path_parse_major_minor(const char *path, mode_t *ret_mode, dev_t *ret_devno); + +int proc_mounted(void); From f8606626ed3c2582e06543550d58fe9886cdca5f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 23 Apr 2020 14:52:10 +0200 Subject: [PATCH 5/5] tmpfiles: if we get ENOENT when opening /proc/self/fd/, check if /proc is mounted let's return ENOSYS in that case, to make things a bit less confusng. Previously we'd just propagate ENOENT, which people might mistake as applying to the object being modified rather than /proc/ just not being there. Let's return ENOSYS instead, i.e. an error clearly indicating that some kernel API is not available. This hopefully should put people on a better track. Note that we only do the procfs check in the error path, which hopefully means it's the less likely path. We probably can add similar bits to more suitable codepaths dealing with /proc/self/fd, but for now, let's pick to the ones noticed in #14745. Fixes: #14745 --- src/basic/fd-util.c | 14 +++++++++++--- src/basic/fs-util.c | 11 +++++++++-- src/tmpfiles/tmpfiles.c | 5 +++++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index a3dced441a..e2bee51147 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -21,9 +21,10 @@ #include "path-util.h" #include "process-util.h" #include "socket-util.h" +#include "stat-util.h" #include "stdio-util.h" -#include "util.h" #include "tmpfile-util.h" +#include "util.h" /* The maximum number of iterations in the loop to close descriptors in the fallback case * when /proc/self/fd/ is inaccessible. */ @@ -939,8 +940,15 @@ int fd_reopen(int fd, int flags) { xsprintf(procfs_path, "/proc/self/fd/%i", fd); new_fd = open(procfs_path, flags); - if (new_fd < 0) - return -errno; + if (new_fd < 0) { + if (errno != ENOENT) + return -errno; + + if (proc_mounted() == 0) + return -ENOSYS; /* if we have no /proc/, the concept is not implementable */ + + return -ENOENT; + } return new_fd; } diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index ef3b5a5184..e16bfef3c3 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -337,8 +337,15 @@ int fchmod_opath(int fd, mode_t m) { * fchownat() does. */ xsprintf(procfs_path, "/proc/self/fd/%i", fd); - if (chmod(procfs_path, m) < 0) - return -errno; + if (chmod(procfs_path, m) < 0) { + if (errno != ENOENT) + return -errno; + + if (proc_mounted() == 0) + return -ENOSYS; /* if we have no /proc/, the concept is not implementable */ + + return -ENOENT; + } return 0; } diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 6f0dc2426e..ff1dff13da 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -1078,6 +1078,11 @@ static int fd_set_acls(Item *item, int fd, const char *path, const struct stat * if (r > 0) return -r; /* already warned */ + + /* The above procfs paths don't work if /proc is not mounted. */ + if (r == -ENOENT && proc_mounted() == 0) + r = -ENOSYS; + if (r == -EOPNOTSUPP) { log_debug_errno(r, "ACLs not supported by file system at %s", path); return 0;