From: Lennart Poettering Date: Mon, 13 Jan 2025 11:09:03 +0000 (+0100) Subject: machine-id-setup: rework writing of /etc/machine-id around chase() X-Git-Tag: v258-rc1~1548^2~3 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=64d52739c59e28a5eb453f749e24f23b2d2ec06a;p=thirdparty%2Fsystemd.git machine-id-setup: rework writing of /etc/machine-id around chase() Let's make sure we properly stay inside of the root fs if one is provided, i.e. stop using prefix_roota() at one more place. --- diff --git a/src/shared/machine-id-setup.c b/src/shared/machine-id-setup.c index 0659bced127..ee7e08b086a 100644 --- a/src/shared/machine-id-setup.c +++ b/src/shared/machine-id-setup.c @@ -131,54 +131,89 @@ static int acquire_machine_id(const char *root, bool machine_id_from_firmware, s } int machine_id_setup(const char *root, sd_id128_t machine_id, MachineIdSetupFlags flags, sd_id128_t *ret) { - const char *etc_machine_id, *run_machine_id; - _cleanup_close_ int fd = -EBADF; + _cleanup_free_ char *etc_machine_id = NULL, *run_machine_id = NULL; bool writable, write_run_machine_id = true; + _cleanup_close_ int fd = -EBADF, run_fd = -EBADF; + bool unlink_run_machine_id = false; int r; - etc_machine_id = prefix_roota(root, "/etc/machine-id"); - WITH_UMASK(0000) { - /* We create this 0444, to indicate that this isn't really - * something you should ever modify. Of course, since the file - * will be owned by root it doesn't matter much, but maybe - * people look. */ + _cleanup_close_ int inode_fd = -EBADF; - (void) mkdir_parents(etc_machine_id, 0755); - fd = open(etc_machine_id, O_RDWR|O_CREAT|O_CLOEXEC|O_NOCTTY, 0444); - if (fd < 0) { - int old_errno = errno; + r = chase("/etc/machine-id", root, CHASE_PREFIX_ROOT, &etc_machine_id, &inode_fd); + if (r == -ENOENT) { + _cleanup_close_ int etc_fd = -EBADF; + _cleanup_free_ char *etc = NULL; + + r = chase("/etc/", root, CHASE_PREFIX_ROOT|CHASE_MKDIR_0755, &etc, &etc_fd); + if (r < 0) + return log_error_errno(r, "Failed to open '/etc/': %m"); - fd = open(etc_machine_id, O_RDONLY|O_CLOEXEC|O_NOCTTY); + etc_machine_id = path_join(etc, "machine-id"); + if (!etc_machine_id) + return log_oom(); + + /* We create this 0444, to indicate that this isn't really something you should ever + * modify. Of course, since the file will be owned by root it doesn't matter much, but maybe + * people look. */ + + fd = openat(etc_fd, "machine-id", O_CREAT|O_EXCL|O_RDWR|O_NOFOLLOW|O_CLOEXEC, 0444); if (fd < 0) { - if (old_errno == EROFS && errno == ENOENT) + if (errno == EROFS) return log_error_errno(errno, - "System cannot boot: Missing /etc/machine-id and /etc is mounted read-only.\n" - "Booting up is supported only when:\n" - "1) /etc/machine-id exists and is populated.\n" - "2) /etc/machine-id exists and is empty.\n" - "3) /etc/machine-id is missing and /etc is writable.\n"); - else - return log_error_errno(errno, "Cannot open %s: %m", etc_machine_id); + "System cannot boot: Missing %s and %s/ is read-only.\n" + "Booting up is supported only when:\n" + "1) /etc/machine-id exists and is populated.\n" + "2) /etc/machine-id exists and is empty.\n" + "3) /etc/machine-id is missing and /etc/ is writable.", + etc_machine_id, + etc); + + return log_error_errno(errno, "Cannot create '%s': %m", etc_machine_id); } - writable = false; - } else + log_debug("Successfully opened new '%s' file.", etc_machine_id); writable = true; + } else if (r < 0) + return log_error_errno(r, "Cannot open '/etc/machine-id': %m"); + else { + /* We pinned the inode, now try to convert it into a writable file */ + + fd = xopenat_full(inode_fd, /* path= */ NULL, O_RDWR|O_CLOEXEC, XO_REGULAR, 0444); + if (fd < 0) { + log_debug_errno(fd, "Failed to topen '%s' in writable mode, retrying in read-only mode: %m", etc_machine_id); + + /* If that didn't work, convert it into a readable file */ + fd = xopenat_full(inode_fd, /* path= */ NULL, O_RDONLY|O_CLOEXEC, XO_REGULAR, MODE_INVALID); + if (fd < 0) + return log_error_errno(fd, "Cannot open '%s' in neither writable nor read-only mode: %m", etc_machine_id); + + log_debug("Successfully opened existing '%s' file in read-only mode.", etc_machine_id); + writable = false; + } else { + log_debug("Successfully opened existing '%s' file in writable mode.", etc_machine_id); + writable = true; + } + } } /* A we got a valid machine ID argument, that's what counts */ if (sd_id128_is_null(machine_id) || FLAGS_SET(flags, MACHINE_ID_SETUP_FORCE_FIRMWARE)) { /* Try to read any existing machine ID */ - if (id128_read_fd(fd, ID128_FORMAT_PLAIN, &machine_id) >= 0) + r = id128_read_fd(fd, ID128_FORMAT_PLAIN, &machine_id); + if (r >= 0) goto finish; + log_debug_errno(r, "Unable to read current machine ID, acquiring new one: %m"); + /* Hmm, so, the id currently stored is not useful, then let's acquire one. */ r = acquire_machine_id(root, FLAGS_SET(flags, MACHINE_ID_SETUP_FORCE_FIRMWARE), &machine_id); if (r < 0) return r; - write_run_machine_id = !r; + + write_run_machine_id = !r; /* acquire_machine_id() returns 1 in case we read this machine ID + * from /run/machine-id */ } if (writable) { @@ -209,37 +244,49 @@ int machine_id_setup(const char *root, sd_id128_t machine_id, MachineIdSetupFlag } } - fd = safe_close(fd); + /* Hmm, we couldn't or shouldn't write the machine-id to /etc/? So let's write it to /run/machine-id + * as a replacement */ - /* Hmm, we couldn't or shouldn't write the machine-id to /etc? - * So let's write it to /run/machine-id as a replacement */ + if (write_run_machine_id) { + _cleanup_free_ char *run = NULL; - run_machine_id = prefix_roota(root, "/run/machine-id"); + r = chase("/run/", root, CHASE_PREFIX_ROOT|CHASE_MKDIR_0755, &run, &run_fd); + if (r < 0) + return log_error_errno(r, "Failed to open '/run/': %m"); - if (write_run_machine_id) { - WITH_UMASK(0022) - r = id128_write(run_machine_id, ID128_FORMAT_PLAIN, machine_id); - if (r < 0) { - (void) unlink(run_machine_id); - return log_error_errno(r, "Cannot write %s: %m", run_machine_id); + run_machine_id = path_join(run, "machine-id"); + if (!run_machine_id) + return log_oom(); + + WITH_UMASK(0022) { + r = id128_write_at(run_fd, "machine-id", ID128_FORMAT_PLAIN, machine_id); + if (r < 0) + return log_error_errno(r, "Cannot write '%s': %m", run_machine_id); } + + unlink_run_machine_id = true; + } else { + r = chase("/run/machine-id", root, CHASE_PREFIX_ROOT, &run_machine_id, /* ret_inode_fd= */ NULL); + if (r < 0) + return log_error_errno(r, "Failed to open '/run/machine-id': %m"); } /* And now, let's mount it over */ - r = mount_follow_verbose(LOG_ERR, run_machine_id, etc_machine_id, NULL, MS_BIND, NULL); - if (r < 0) { - (void) unlink(run_machine_id); + r = mount_follow_verbose(LOG_ERR, run_machine_id, FORMAT_PROC_FD_PATH(fd), /* fstype= */ NULL, MS_BIND, /* options= */ NULL); + if (r < 0) return r; - } - log_full(FLAGS_SET(flags, MACHINE_ID_SETUP_FORCE_TRANSIENT) ? LOG_DEBUG : LOG_INFO, "Installed transient %s file.", etc_machine_id); + log_full(FLAGS_SET(flags, MACHINE_ID_SETUP_FORCE_TRANSIENT) ? LOG_DEBUG : LOG_INFO, "Installed transient '%s' file.", etc_machine_id); - /* Mark the mount read-only */ - r = mount_follow_verbose(LOG_WARNING, NULL, etc_machine_id, NULL, MS_BIND|MS_RDONLY|MS_REMOUNT, NULL); - if (r < 0) - return r; + unlink_run_machine_id = false; + + /* Mark the mount read-only (note: we are not going via FORMAT_PROC_FD_PATH() here because that fd is not updated to our new bind mount) */ + (void) mount_follow_verbose(LOG_WARNING, /* source= */ NULL, etc_machine_id, /* fstype= */ NULL, MS_BIND|MS_RDONLY|MS_REMOUNT, /* options= */ NULL); finish: + if (unlink_run_machine_id) + (void) unlinkat(ASSERT_FD(run_fd), "machine-id", /* flags= */ 0); + if (!in_initrd()) (void) sd_notifyf(/* unset_environment= */ false, "X_SYSTEMD_MACHINE_ID=" SD_ID128_FORMAT_STR, SD_ID128_FORMAT_VAL(machine_id));