diff --git a/src/core/automount.c b/src/core/automount.c index 28d5cc3917..c191336c0e 100644 --- a/src/core/automount.c +++ b/src/core/automount.c @@ -578,7 +578,7 @@ static void automount_enter_waiting(Automount *a) { set_clear(a->tokens); - r = unit_fail_if_symlink(UNIT(a), a->where); + r = unit_fail_if_noncanonical(UNIT(a), a->where); if (r < 0) goto fail; diff --git a/src/core/mount.c b/src/core/mount.c index d424e5a3a8..453a83f93d 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -55,7 +55,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(struct libmnt_iter*, mnt_free_iter); static const UnitActiveState state_translation_table[_MOUNT_STATE_MAX] = { [MOUNT_DEAD] = UNIT_INACTIVE, [MOUNT_MOUNTING] = UNIT_ACTIVATING, - [MOUNT_MOUNTING_DONE] = UNIT_ACTIVE, + [MOUNT_MOUNTING_DONE] = UNIT_ACTIVATING, [MOUNT_MOUNTED] = UNIT_ACTIVE, [MOUNT_REMOUNTING] = UNIT_RELOADING, [MOUNT_UNMOUNTING] = UNIT_DEACTIVATING, @@ -942,7 +942,7 @@ static void mount_enter_mounting(Mount *m) { assert(m); - r = unit_fail_if_symlink(UNIT(m), m->where); + r = unit_fail_if_noncanonical(UNIT(m), m->where); if (r < 0) goto fail; @@ -1135,10 +1135,6 @@ static int mount_reload(Unit *u) { Mount *m = MOUNT(u); assert(m); - - if (m->state == MOUNT_MOUNTING_DONE) /* not yet ready to reload, try again */ - return -EAGAIN; - assert(m->state == MOUNT_MOUNTED); mount_enter_remounting(m); @@ -1280,23 +1276,25 @@ static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) { log_unit_full(u, f == MOUNT_SUCCESS ? LOG_DEBUG : LOG_NOTICE, 0, "Mount process exited, code=%s status=%i", sigchld_code_to_string(code), status); - /* Note that mount(8) returning and the kernel sending us a mount table change event might happen - * out-of-order. If an operation succeed we assume the kernel will follow soon too and already change into the - * resulting state. If it fails we check if the kernel still knows about the mount. and change state - * accordingly. */ + /* Note that due to the io event priority logic, we can be sure the new mountinfo is loaded + * before we process the SIGCHLD for the mount command. */ switch (m->state) { case MOUNT_MOUNTING: - case MOUNT_MOUNTING_DONE: + /* Our mount point has not appeared in mountinfo. Something went wrong. */ - if (f == MOUNT_SUCCESS || m->from_proc_self_mountinfo) - /* If /bin/mount returned success, or if we see the mount point in /proc/self/mountinfo we are - * happy. If we see the first condition first, we should see the second condition - * immediately after – or /bin/mount lies to us and is broken. */ - mount_enter_mounted(m, f); - else - mount_enter_dead(m, f); + if (f == MOUNT_SUCCESS) { + /* Either /bin/mount has an unexpected definition of success, + * or someone raced us and we lost. */ + log_unit_warning(UNIT(m), "Mount process finished, but there is no mount."); + f = MOUNT_FAILURE_PROTOCOL; + } + mount_enter_dead(m, f); + break; + + case MOUNT_MOUNTING_DONE: + mount_enter_mounted(m, f); break; case MOUNT_REMOUNTING: @@ -1306,21 +1304,19 @@ static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) { break; case MOUNT_UNMOUNTING: - case MOUNT_UNMOUNTING_SIGKILL: - case MOUNT_UNMOUNTING_SIGTERM: if (f == MOUNT_SUCCESS && m->from_proc_self_mountinfo) { /* Still a mount point? If so, let's try again. Most likely there were multiple mount points - * stacked on top of each other. Note that due to the io event priority logic we can be sure - * the new mountinfo is loaded before we process the SIGCHLD for the mount command. */ + * stacked on top of each other. We might exceed the timeout specified by the user overall, + * but we will stop as soon as any one umount times out. */ if (m->n_retry_umount < RETRY_UMOUNT_MAX) { log_unit_debug(u, "Mount still present, trying again."); m->n_retry_umount++; mount_enter_unmounting(m); } else { - log_unit_debug(u, "Mount still present after %u attempts to unmount, giving up.", m->n_retry_umount); + log_unit_warning(u, "Mount still present after %u attempts to unmount, giving up.", m->n_retry_umount); mount_enter_mounted(m, f); } } else @@ -1328,6 +1324,11 @@ static void mount_sigchld_event(Unit *u, pid_t pid, int code, int status) { break; + case MOUNT_UNMOUNTING_SIGKILL: + case MOUNT_UNMOUNTING_SIGTERM: + mount_enter_dead_or_mounted(m, f); + break; + default: assert_not_reached("Uh, control process died at wrong time."); } @@ -1951,6 +1952,7 @@ static const char* const mount_result_table[_MOUNT_RESULT_MAX] = { [MOUNT_FAILURE_SIGNAL] = "signal", [MOUNT_FAILURE_CORE_DUMP] = "core-dump", [MOUNT_FAILURE_START_LIMIT_HIT] = "start-limit-hit", + [MOUNT_FAILURE_PROTOCOL] = "protocol", }; DEFINE_STRING_TABLE_LOOKUP(mount_result, MountResult); diff --git a/src/core/mount.h b/src/core/mount.h index 44fe3b889e..1a496def84 100644 --- a/src/core/mount.h +++ b/src/core/mount.h @@ -35,12 +35,13 @@ typedef enum MountExecCommand { typedef enum MountResult { MOUNT_SUCCESS, - MOUNT_FAILURE_RESOURCES, + MOUNT_FAILURE_RESOURCES, /* a bit of a misnomer, just our catch-all error for errnos we didn't expect */ MOUNT_FAILURE_TIMEOUT, MOUNT_FAILURE_EXIT_CODE, MOUNT_FAILURE_SIGNAL, MOUNT_FAILURE_CORE_DUMP, MOUNT_FAILURE_START_LIMIT_HIT, + MOUNT_FAILURE_PROTOCOL, _MOUNT_RESULT_MAX, _MOUNT_RESULT_INVALID = -1 } MountResult; diff --git a/src/core/unit.c b/src/core/unit.c index d736d274de..308fe3f3e8 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -4705,25 +4705,29 @@ void unit_warn_if_dir_nonempty(Unit *u, const char* where) { NULL); } -int unit_fail_if_symlink(Unit *u, const char* where) { +int unit_fail_if_noncanonical(Unit *u, const char* where) { + _cleanup_free_ char *canonical_where; int r; assert(u); assert(where); - r = is_symlink(where); + r = chase_symlinks(where, NULL, CHASE_NONEXISTENT, &canonical_where); if (r < 0) { - log_unit_debug_errno(u, r, "Failed to check symlink %s, ignoring: %m", where); + log_unit_debug_errno(u, r, "Failed to check %s for symlinks, ignoring: %m", where); return 0; } - if (r == 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_struct(LOG_ERR, "MESSAGE_ID=" SD_MESSAGE_OVERMOUNTING_STR, LOG_UNIT_ID(u), LOG_UNIT_INVOCATION_ID(u), - LOG_UNIT_MESSAGE(u, "Mount on symlink %s not allowed.", where), + LOG_UNIT_MESSAGE(u, "Mount path %s is not canonical (contains a symlink).", where), "WHERE=%s", where, NULL); diff --git a/src/core/unit.h b/src/core/unit.h index f8a12947cf..f3e991e79e 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -760,7 +760,7 @@ static inline bool unit_supported(Unit *u) { } void unit_warn_if_dir_nonempty(Unit *u, const char* where); -int unit_fail_if_symlink(Unit *u, const char* where); +int unit_fail_if_noncanonical(Unit *u, const char* where); int unit_start_limit_test(Unit *u);