From: Lennart Poettering Date: Thu, 6 Feb 2025 22:08:37 +0000 (+0100) Subject: core: port mount unit inode creation to make_mount_point_inode_from_mode() too X-Git-Tag: v258-rc1~1299^2~4 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=38c35970b13c47de731003535efec516b75c473e;p=thirdparty%2Fsystemd.git 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. --- diff --git a/src/core/automount.c b/src/core/automount.c index ac81875442e..5ac5dec8d9d 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 d7557bf3897..361d8e03189 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 3f4432f6556..91608766351 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -5125,14 +5125,28 @@ 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 unit_log_noncanonical_mount_path(Unit *u, const char *where) { + assert(u); + assert(where); + + /* 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, + LOG_UNIT_INVOCATION_ID(u), + LOG_UNIT_MESSAGE(u, "Mount path %s is not canonical (contains a symlink).", where), + "WHERE=%s", where); + + return -ELOOP; +} + +int unit_fail_if_noncanonical_mount_path(Unit *u, const char* where) { int r; assert(u); assert(where); - r = chase(where, NULL, CHASE_NONEXISTENT, &canonical_where, NULL); + _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; @@ -5142,14 +5156,7 @@ int unit_fail_if_noncanonical(Unit *u, const char* where) { 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, - LOG_UNIT_INVOCATION_ID(u), - LOG_UNIT_MESSAGE(u, "Mount path %s is not canonical (contains a symlink).", where), - "WHERE=%s", where); - - return -ELOOP; + return unit_log_noncanonical_mount_path(u, where); } bool unit_is_pristine(Unit *u) { diff --git a/src/core/unit.h b/src/core/unit.h index 45b7d72b7af..4b19d8f0f75 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);