From 8a9c44edf90d122f80a0234f5416eabb0d6061e0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 23 Nov 2018 18:51:30 +0100 Subject: [PATCH 1/7] gpt-auto: compare kernel cmdline args with proc_cmdline_key_streq() --- src/gpt-auto-generator/gpt-auto-generator.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c index a6d7bcc755..28479386b9 100644 --- a/src/gpt-auto-generator/gpt-auto-generator.c +++ b/src/gpt-auto-generator/gpt-auto-generator.c @@ -558,7 +558,8 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat assert(key); - if (STR_IN_SET(key, "systemd.gpt_auto", "rd.systemd.gpt_auto")) { + if (proc_cmdline_key_streq(key, "systemd.gpt_auto") || + proc_cmdline_key_streq(key, "rd.systemd.gpt_auto")) { r = value ? parse_boolean(value) : 1; if (r < 0) @@ -566,7 +567,7 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat else arg_enabled = r; - } else if (streq(key, "root")) { + } else if (proc_cmdline_key_streq(key, "root")) { if (proc_cmdline_value_missing(key, value)) return 0; @@ -576,7 +577,7 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat arg_root_enabled = streq(value, "gpt-auto"); - } else if (streq(key, "roothash")) { + } else if (proc_cmdline_key_streq(key, "roothash")) { if (proc_cmdline_value_missing(key, value)) return 0; @@ -585,9 +586,9 @@ static int parse_proc_cmdline_item(const char *key, const char *value, void *dat arg_root_enabled = false; - } else if (streq(key, "rw") && !value) + } else if (proc_cmdline_key_streq(key, "rw") && !value) arg_root_rw = true; - else if (streq(key, "ro") && !value) + else if (proc_cmdline_key_streq(key, "ro") && !value) arg_root_rw = false; return 0; From e0fe3a03ab423711b1e766e3d2980a1927b34d5e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 23 Nov 2018 19:40:12 +0100 Subject: [PATCH 2/7] remount-fs: use PATH_IN_SET() at one more place --- src/remount-fs/remount-fs.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/remount-fs/remount-fs.c b/src/remount-fs/remount-fs.c index af92ddb96c..bfc6ca67a1 100644 --- a/src/remount-fs/remount-fs.c +++ b/src/remount-fs/remount-fs.c @@ -56,8 +56,7 @@ static int run(int argc, char *argv[]) { /* Remount the root fs, /usr and all API VFS */ if (!mount_point_is_api(me->mnt_dir) && - !path_equal(me->mnt_dir, "/") && - !path_equal(me->mnt_dir, "/usr")) + !PATH_IN_SET(me->mnt_dir, "/", "/usr")) continue; log_debug("Remounting %s", me->mnt_dir); From 58b86fdf1dcb47676c9e82d38546b779fa9cb66b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 23 Nov 2018 19:41:11 +0100 Subject: [PATCH 3/7] remount-fs: split code for tracking PIDs in hashmap Just some refactoring, no change in behaviour. --- src/remount-fs/remount-fs.c | 47 +++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/src/remount-fs/remount-fs.c b/src/remount-fs/remount-fs.c index bfc6ca67a1..88e2b34acf 100644 --- a/src/remount-fs/remount-fs.c +++ b/src/remount-fs/remount-fs.c @@ -19,9 +19,32 @@ #include "strv.h" #include "util.h" -/* Goes through /etc/fstab and remounts all API file systems, applying - * options that are in /etc/fstab that systemd might not have - * respected */ +/* Goes through /etc/fstab and remounts all API file systems, applying options that are in /etc/fstab that systemd + * might not have respected */ + +static int track_pid(Hashmap **h, const char *path, pid_t pid) { + _cleanup_free_ char *c = NULL; + int r; + + assert(h); + assert(path); + assert(pid_is_valid(pid)); + + r = hashmap_ensure_allocated(h, NULL); + if (r < 0) + return log_oom(); + + c = strdup(path); + if (!c) + return log_oom(); + + r = hashmap_put(*h, PID_TO_PTR(pid), c); + if (r < 0) + return log_oom(); + + TAKE_PTR(c); + return 0; +} static int run(int argc, char *argv[]) { _cleanup_hashmap_free_free_ Hashmap *pids = NULL; @@ -45,14 +68,8 @@ static int run(int argc, char *argv[]) { return log_error_errno(errno, "Failed to open /etc/fstab: %m"); } - pids = hashmap_new(NULL); - if (!pids) - return log_oom(); - while ((me = getmntent(f))) { - _cleanup_free_ char *s = NULL; pid_t pid; - int k; /* Remount the root fs, /usr and all API VFS */ if (!mount_point_is_api(me->mnt_dir) && @@ -74,15 +91,9 @@ static int run(int argc, char *argv[]) { } /* Parent */ - - s = strdup(me->mnt_dir); - if (!s) - return log_oom(); - - k = hashmap_put(pids, PID_TO_PTR(pid), s); - if (k < 0) - return log_oom(); - TAKE_PTR(s); + r = track_pid(&pids, me->mnt_dir, pid); + if (r < 0) + return r; } r = 0; From 59f13dd6f801b2df4faea277a5de0115ed598e35 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 23 Nov 2018 19:47:41 +0100 Subject: [PATCH 4/7] remount-fs: optionally remount / writable, if we are told through an env var --- docs/ENVIRONMENT.md | 8 ++++ src/remount-fs/remount-fs.c | 80 +++++++++++++++++++++++++------------ 2 files changed, 63 insertions(+), 25 deletions(-) diff --git a/docs/ENVIRONMENT.md b/docs/ENVIRONMENT.md index 038f3d2d1d..619a57eb3f 100644 --- a/docs/ENVIRONMENT.md +++ b/docs/ENVIRONMENT.md @@ -165,3 +165,11 @@ systemd itself: * `$SYSTEMD_ACTIVATION_SCOPE` — closely related to `$SYSTEMD_ACTIVATION_UNIT`, it is either set to `system` or `user` depending on whether the NSS/PAM module is called by systemd in `--system` or `--user` mode. + +systemd-remount-fs: + +* `$SYSTEMD_REMOUNT_ROOT_RW=1` — if set and and no entry for the root directory + exists in /etc/fstab (this file always takes precedence), then the root + directory is remounted writable. This is primarily used by + systemd-gpt-auto-generator to ensure the root partition is mounted writable + in accordance to the GPT partition flags. diff --git a/src/remount-fs/remount-fs.c b/src/remount-fs/remount-fs.c index 88e2b34acf..0bac355e0f 100644 --- a/src/remount-fs/remount-fs.c +++ b/src/remount-fs/remount-fs.c @@ -8,6 +8,7 @@ #include #include +#include "env-util.h" #include "exit-status.h" #include "log.h" #include "main-func.h" @@ -49,6 +50,7 @@ static int track_pid(Hashmap **h, const char *path, pid_t pid) { static int run(int argc, char *argv[]) { _cleanup_hashmap_free_free_ Hashmap *pids = NULL; _cleanup_endmntent_ FILE *f = NULL; + bool has_root = false; struct mntent* me; int r; @@ -62,44 +64,72 @@ static int run(int argc, char *argv[]) { f = setmntent("/etc/fstab", "re"); if (!f) { - if (errno == ENOENT) - return 0; + if (errno != ENOENT) + return log_error_errno(errno, "Failed to open /etc/fstab: %m"); + } else { + while ((me = getmntent(f))) { + pid_t pid; - return log_error_errno(errno, "Failed to open /etc/fstab: %m"); + /* Remount the root fs, /usr and all API VFS */ + if (!mount_point_is_api(me->mnt_dir) && + !PATH_IN_SET(me->mnt_dir, "/", "/usr")) + continue; + + log_debug("Remounting %s...", me->mnt_dir); + + if (path_equal(me->mnt_dir, "/")) + has_root = true; + + r = safe_fork("(remount)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_RLIMIT_NOFILE_SAFE|FORK_LOG, &pid); + if (r < 0) + return r; + if (r == 0) { + /* Child */ + execv(MOUNT_PATH, STRV_MAKE(MOUNT_PATH, me->mnt_dir, "-o", "remount")); + log_error_errno(errno, "Failed to execute " MOUNT_PATH ": %m"); + _exit(EXIT_FAILURE); + } + + /* Parent */ + r = track_pid(&pids, me->mnt_dir, pid); + if (r < 0) + return r; + } } - while ((me = getmntent(f))) { - pid_t pid; + if (!has_root) { + /* The $SYSTEMD_REMOUNT_ROOT_RW environment variable is set by systemd-gpt-auto-generator to tell us + * whether to remount things. We honour it only if there's no explicit line in /etc/fstab configured + * which takes precedence. */ - /* Remount the root fs, /usr and all API VFS */ - if (!mount_point_is_api(me->mnt_dir) && - !PATH_IN_SET(me->mnt_dir, "/", "/usr")) - continue; + r = getenv_bool("SYSTEMD_REMOUNT_ROOT_RW"); + if (r > 0) { + pid_t pid; - log_debug("Remounting %s", me->mnt_dir); + log_debug("Remounting / writable..."); - r = safe_fork("(remount)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_RLIMIT_NOFILE_SAFE|FORK_LOG, &pid); - if (r < 0) - return r; - if (r == 0) { - /* Child */ + r = safe_fork("(remount-rw)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); + if (r < 0) + return r; + if (r == 0) { + /* Child */ + execv(MOUNT_PATH, STRV_MAKE(MOUNT_PATH, "/", "-o", "remount,rw")); + log_error_errno(errno, "Failed to execute " MOUNT_PATH ": %m"); + _exit(EXIT_FAILURE); + } - execv(MOUNT_PATH, STRV_MAKE(MOUNT_PATH, me->mnt_dir, "-o", "remount")); + r = track_pid(&pids, "/", pid); + if (r < 0) + return r; - log_error_errno(errno, "Failed to execute " MOUNT_PATH ": %m"); - _exit(EXIT_FAILURE); - } - - /* Parent */ - r = track_pid(&pids, me->mnt_dir, pid); - if (r < 0) - return r; + } else if (r < 0 && r != -ENXIO) + log_warning_errno(r, "Failed to parse $SYSTEMD_REMOUNT_ROOT_RW, ignoring: %m"); } r = 0; while (!hashmap_isempty(pids)) { - siginfo_t si = {}; _cleanup_free_ char *s = NULL; + siginfo_t si = {}; if (waitid(P_ALL, 0, &si, WEXITED) < 0) { if (errno == EINTR) From c94b2417770625d78705e5f55725f04c26398fce Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 23 Nov 2018 19:49:13 +0100 Subject: [PATCH 5/7] gpt-auto: make arg_root_rw a tri-state No change in behaviour, but let's track whether ro or rw are specified on the kernel cmdline at all. --- src/gpt-auto-generator/gpt-auto-generator.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c index 28479386b9..425f5421ca 100644 --- a/src/gpt-auto-generator/gpt-auto-generator.c +++ b/src/gpt-auto-generator/gpt-auto-generator.c @@ -39,7 +39,7 @@ static const char *arg_dest = NULL; static bool arg_enabled = true; static bool arg_root_enabled = true; -static bool arg_root_rw = false; +static int arg_root_rw = -1; static int add_cryptsetup(const char *id, const char *what, bool rw, bool require, char **device) { _cleanup_free_ char *e = NULL, *n = NULL, *d = NULL, *id_escaped = NULL, *what_escaped = NULL; @@ -640,7 +640,7 @@ static int add_root_mount(void) { "/dev/gpt-auto-root", in_initrd() ? "/sysroot" : "/", NULL, - arg_root_rw, + arg_root_rw > 0, NULL, "Root Partition", in_initrd() ? SPECIAL_INITRD_ROOT_FS_TARGET : SPECIAL_LOCAL_FS_TARGET); From fd89051ec3d2555c19d71d5fc6d76809ca3bb79c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 23 Nov 2018 22:16:57 +0100 Subject: [PATCH 6/7] gpt-auto: propagate gpt partition ro/rw flag into root mount MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This ensures that the read/write state of the root mount matches the read/write flag in the GPT partition table entry. This is only used as fallback in case no ro/rw flag is specified on the kernel cmdline, and there's no entry for the root partition in /etc/fstab. This is missing functionality of the GPT auto logic, as without this the root partition was always mounted read-only — when booting with zero configuration in /etc/fstab and /proc/cmdline —, as we defaulted to read-only behaviour for all mounts. Moreover we honoured the r/o flag in the partition table for all other partition types, except for the root partition. --- src/gpt-auto-generator/gpt-auto-generator.c | 43 +++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c index 425f5421ca..c6a801389e 100644 --- a/src/gpt-auto-generator/gpt-auto-generator.c +++ b/src/gpt-auto-generator/gpt-auto-generator.c @@ -446,6 +446,43 @@ static int add_esp(DissectedPartition *p) { } #endif +static int add_root_rw(DissectedPartition *p) { + const char *path; + int r; + + assert(p); + + if (in_initrd()) { + log_debug("In initrd, not generating drop-in for systemd-remount-fs.service."); + return 0; + } + + if (arg_root_rw >= 0) { + log_debug("Parameter ro/rw specified on kernel command line, not generating drop-in for systemd-remount-fs.service."); + return 0; + } + + if (!p->rw) { + log_debug("Root partition marked read-only in GPT partition table, not generating drop-in for systemd-remount-fs.service."); + return 0; + } + + path = strjoina(arg_dest, "/systemd-remount-fs.service.d/50-remount-rw.conf"); + (void) mkdir_parents(path, 0755); + + r = write_string_file(path, + "# Automatically generated by systemd-gpt-generator\n\n" + "[Unit]\n" + "ConditionPathExists=\n\n" /* We need to turn off the ConditionPathExist= in the main unit file */ + "[Service]\n" + "Environment=SYSTEMD_REMOUNT_ROOT_RW=1\n", + WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_NOFOLLOW); + if (r < 0) + return log_error_errno(r, "Failed to write drop-in file %s: %m", path); + + return 0; +} + static int open_parent(dev_t devnum, int *ret) { _cleanup_(sd_device_unrefp) sd_device *d = NULL; const char *name, *devtype, *node; @@ -550,6 +587,12 @@ static int enumerate_partitions(dev_t devnum) { r = k; } + if (m->partitions[PARTITION_ROOT].found) { + k = add_root_rw(m->partitions + PARTITION_ROOT); + if (k < 0) + r = k; + } + return r; } From 7d1353ccf21190d82030750df9d1150f9f894d3d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 23 Nov 2018 23:06:22 +0100 Subject: [PATCH 7/7] update TODO --- TODO | 3 --- 1 file changed, 3 deletions(-) diff --git a/TODO b/TODO index b7038ec7c0..08581b1244 100644 --- a/TODO +++ b/TODO @@ -48,9 +48,6 @@ Features: inserting them into the kernel keyring. Maybe SecretsDirectory= similar to ConfigurationDirectory=. -* systemd-gpt-auto: if we find the root dir mounted read-only and the gpt flag - doesn't say so generate job that remounts it writable - * when no locale is configured, default to UEFI's PlatformLang variable * When logind.conf contains HandleLidSwitch=suspend-then-hibernate and we can't