From 38c35970b13c47de731003535efec516b75c473e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 6 Feb 2025 23:08:37 +0100 Subject: [PATCH] core: port mount unit inode creation to make_mount_point_inode_from_mode() too This also ports over things to use chase() to create/pin the underlying to mount, and in particular checks that the path does not contain any symlinks. That's crucial since we cannot allow mounts to be established with that, since it would mean we couldn't recognize the entries in /proc/self/mountinfo anymore. --- src/core/automount.c | 2 +- src/core/mount.c | 31 ++++++++++++++++++++++--------- src/core/unit.c | 35 +++++++++++++++++++++-------------- src/core/unit.h | 3 ++- 4 files changed, 46 insertions(+), 25 deletions(-) diff --git a/src/core/automount.c b/src/core/automount.c index ac81875442..5ac5dec8d9 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -556,7 +556,7 @@ static void automount_enter_waiting(Automount *a) { set_clear(a->tokens); - r = unit_fail_if_noncanonical(UNIT(a), a->where); + r = unit_fail_if_noncanonical_mount_path(UNIT(a), a->where); if (r < 0) goto fail; diff --git a/src/core/mount.c b/src/core/mount.c index d7557bf389..361d8e0318 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -8,11 +8,13 @@ #include "sd-messages.h" #include "alloc-util.h" +#include "chase.h" #include "dbus-mount.h" #include "dbus-unit.h" #include "device.h" #include "exec-credential.h" #include "exit-status.h" +#include "fd-util.h" #include "format-util.h" #include "fs-util.h" #include "fstab-util.h" @@ -1183,22 +1185,32 @@ static int mount_set_mount_command(Mount *m, ExecCommand *c, const MountParamete } static void mount_enter_mounting(Mount *m) { - MountParameters *p; - bool source_is_dir = true; + _cleanup_close_ int fd = -EBADF; + _cleanup_free_ char *fn = NULL; int r; assert(m); - r = unit_fail_if_noncanonical(UNIT(m), m->where); - if (r < 0) + /* Validate that the path we are overmounting does not contain any symlinks, because if it does, we + * couldn't support that reasonably: the mounts in /proc/self/mountinfo would not be recognizable to + * us anymore. */ + fd = chase_and_open_parent(m->where, /* root= */ NULL, CHASE_PROHIBIT_SYMLINKS|CHASE_MKDIR_0755, &fn); + if (fd == -EREMCHG) { + r = unit_log_noncanonical_mount_path(UNIT(m), m->where); goto fail; + } + if (fd < 0) { + log_unit_error_errno(UNIT(m), fd, "Failed to resolve parent of mount point '%s': %m", m->where); + goto fail; + } - p = get_mount_parameters_fragment(m); + MountParameters *p = get_mount_parameters_fragment(m); if (!p) { r = log_unit_warning_errno(UNIT(m), SYNTHETIC_ERRNO(ENOENT), "No mount parameters to operate on."); goto fail; } + bool source_is_dir = true; if (mount_is_bind(p)) { r = is_dir(p->what, /* follow = */ true); if (r < 0 && r != -ENOENT) @@ -1207,10 +1219,11 @@ static void mount_enter_mounting(Mount *m) { source_is_dir = false; } - if (source_is_dir) - r = mkdir_p_label(m->where, m->directory_mode); - else - r = touch_file(m->where, /* parents = */ true, USEC_INFINITY, UID_INVALID, GID_INVALID, MODE_INVALID); + r = make_mount_point_inode_from_mode( + fd, + fn, + source_is_dir ? S_IFDIR : S_IFREG, + m->directory_mode); if (r < 0 && r != -EEXIST) log_unit_warning_errno(UNIT(m), r, "Failed to create mount point '%s', ignoring: %m", m->where); diff --git a/src/core/unit.c b/src/core/unit.c index 3f4432f655..9160876635 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -5125,23 +5125,10 @@ void unit_warn_if_dir_nonempty(Unit *u, const char* where) { "WHERE=%s", where); } -int unit_fail_if_noncanonical(Unit *u, const char* where) { - _cleanup_free_ char *canonical_where = NULL; - int r; - +int unit_log_noncanonical_mount_path(Unit *u, const char *where) { assert(u); assert(where); - r = chase(where, NULL, CHASE_NONEXISTENT, &canonical_where, NULL); - if (r < 0) { - log_unit_debug_errno(u, r, "Failed to check %s for symlinks, ignoring: %m", where); - return 0; - } - - /* We will happily ignore a trailing slash (or any redundant slashes) */ - if (path_equal(where, canonical_where)) - return 0; - /* No need to mention "." or "..", they would already have been rejected by unit_name_from_path() */ log_unit_struct(u, LOG_ERR, "MESSAGE_ID=" SD_MESSAGE_OVERMOUNTING_STR, @@ -5152,6 +5139,26 @@ int unit_fail_if_noncanonical(Unit *u, const char* where) { return -ELOOP; } +int unit_fail_if_noncanonical_mount_path(Unit *u, const char* where) { + int r; + + assert(u); + assert(where); + + _cleanup_free_ char *canonical_where = NULL; + r = chase(where, /* root= */ NULL, CHASE_NONEXISTENT, &canonical_where, /* ret_fd= */ NULL); + if (r < 0) { + log_unit_debug_errno(u, r, "Failed to check %s for symlinks, ignoring: %m", where); + return 0; + } + + /* We will happily ignore a trailing slash (or any redundant slashes) */ + if (path_equal(where, canonical_where)) + return 0; + + return unit_log_noncanonical_mount_path(u, where); +} + bool unit_is_pristine(Unit *u) { assert(u); diff --git a/src/core/unit.h b/src/core/unit.h index 45b7d72b7a..4b19d8f0f7 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -974,7 +974,8 @@ static inline PidRef* unit_main_pid(Unit *u) { } void unit_warn_if_dir_nonempty(Unit *u, const char* where); -int unit_fail_if_noncanonical(Unit *u, const char* where); +int unit_log_noncanonical_mount_path(Unit *u, const char *where); +int unit_fail_if_noncanonical_mount_path(Unit *u, const char* where); int unit_test_start_limit(Unit *u);