diff --git a/.github/codeql-queries/PotentiallyDangerousFunction.ql b/.github/codeql-queries/PotentiallyDangerousFunction.ql index 40e2bbb6f9..abd3f87a34 100644 --- a/.github/codeql-queries/PotentiallyDangerousFunction.ql +++ b/.github/codeql-queries/PotentiallyDangerousFunction.ql @@ -52,6 +52,12 @@ predicate potentiallyDangerousFunction(Function f, string message) { ) or ( f.getQualifiedName() = "basename" and message = "Call basename() is icky. Use path_extract_filename() instead." + ) or ( + f.getQualifiedName() = "setmntent" and + message = "Libmount parser is used instead, specifically libmount_parse_fstab()." + ) or ( + f.getQualifiedName() = "getmntent" and + message = "Libmount parser is used instead, specifically mnt_table_next_fs()." ) } diff --git a/TODO b/TODO index 0aa861c918..0b73aa4375 100644 --- a/TODO +++ b/TODO @@ -522,11 +522,6 @@ Features: * port copy.c over to use LabelOps for all labelling. -* port remaining getmntent() users over to libmount. There are subtle - differences in the parsers (see #25371 for example), and it hence makes sense - if we stick to one set of parsers on this, not mix both. Specifically: - systemd-fstab-generator, cryptsetup, remount-fs. - * get rid of compat with libidn.so.11 (retain only for libidn.so.12) * get rid of compat with libbpf.so.0 (retainly only for libbpf.so.1) diff --git a/src/cryptsetup/cryptsetup.c b/src/cryptsetup/cryptsetup.c index 357ee78695..c7897a2f20 100644 --- a/src/cryptsetup/cryptsetup.c +++ b/src/cryptsetup/cryptsetup.c @@ -1,7 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #include -#include #include #include #include @@ -33,6 +32,7 @@ #include "hexdecoct.h" #include "json-util.h" #include "libfido2-util.h" +#include "libmount-util.h" #include "log.h" #include "main-func.h" #include "memory-util.h" @@ -732,26 +732,37 @@ static char* disk_description(const char *path) { return NULL; } -static char *disk_mount_point(const char *label) { +static char* disk_mount_point(const char *label) { + _cleanup_(mnt_free_tablep) struct libmnt_table *table = NULL; + _cleanup_(mnt_free_iterp) struct libmnt_iter *iter = NULL; _cleanup_free_ char *device = NULL; - _cleanup_endmntent_ FILE *f = NULL; - struct mntent *m; + int r; /* Yeah, we don't support native systemd unit files here for now */ + assert(label); + device = strjoin("/dev/mapper/", label); if (!device) return NULL; - f = setmntent(fstab_path(), "re"); - if (!f) + r = libmount_parse_fstab(&table, &iter); + if (r < 0) return NULL; - while ((m = getmntent(f))) - if (path_equal(m->mnt_fsname, device)) - return strdup(m->mnt_dir); + for (;;) { + struct libmnt_fs *fs; - return NULL; + r = mnt_table_next_fs(table, iter, &fs); + if (r != 0) + return NULL; + + if (path_equal(mnt_fs_get_source(fs), device)) { + const char *target = mnt_fs_get_target(fs); + if (target) + return strdup(target); + } + } } static char *friendly_disk_name(const char *src, const char *vol) { diff --git a/src/cryptsetup/meson.build b/src/cryptsetup/meson.build index c2f4dc9065..cb47fd9417 100644 --- a/src/cryptsetup/meson.build +++ b/src/cryptsetup/meson.build @@ -19,6 +19,7 @@ executables += [ 'sources' : systemd_cryptsetup_sources, 'dependencies' : [ libcryptsetup, + libmount, libopenssl, libp11kit_cflags, ], diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c index 14a53cd572..1b68689bf9 100644 --- a/src/fstab-generator/fstab-generator.c +++ b/src/fstab-generator/fstab-generator.c @@ -24,10 +24,10 @@ #include "generator.h" #include "in-addr-util.h" #include "initrd-util.h" +#include "libmount-util.h" #include "log.h" #include "main-func.h" #include "mount-setup.h" -#include "mount-util.h" #include "mountpoint-util.h" #include "parse-util.h" #include "path-util.h" @@ -1025,9 +1025,9 @@ static int parse_fstab_one( } static int parse_fstab(bool prefix_sysroot) { - _cleanup_endmntent_ FILE *f = NULL; + _cleanup_(mnt_free_tablep) struct libmnt_table *table = NULL; + _cleanup_(mnt_free_iterp) struct libmnt_iter *iter = NULL; const char *fstab; - struct mntent *me; int r, ret = 0; if (prefix_sysroot) @@ -1039,27 +1039,31 @@ static int parse_fstab(bool prefix_sysroot) { log_debug("Parsing %s...", fstab); - f = setmntent(fstab, "re"); - if (!f) { - if (errno == ENOENT) - return 0; + r = libmount_parse_full(fstab, /* source = */ NULL, &table, &iter); + if (r == -ENOENT) + return 0; + if (r < 0) + return log_error_errno(r, "Failed to parse '%s': %m", fstab); - return log_error_errno(errno, "Failed to open %s: %m", fstab); - } + for (;;) { + struct libmnt_fs *fs; + + r = mnt_table_next_fs(table, iter, &fs); + if (r < 0) + return log_error_errno(r, "Failed to get next entry from '%s': %m", fstab); + if (r > 0) /* EOF */ + return ret; - while ((me = getmntent(f))) { r = parse_fstab_one(fstab, - me->mnt_fsname, me->mnt_dir, me->mnt_type, me->mnt_opts, me->mnt_passno, + mnt_fs_get_source(fs), mnt_fs_get_target(fs), + mnt_fs_get_fstype(fs), mnt_fs_get_options(fs), mnt_fs_get_passno(fs), prefix_sysroot, /* accept_root = */ false, /* use_swap_enabled = */ true); - if (r < 0 && ret >= 0) - ret = r; if (arg_sysroot_check && r > 0) return true; /* We found a mount or swap that would be started… */ + RET_GATHER(ret, r); } - - return ret; } static int mount_source_is_nfsroot(const char *what) { @@ -1414,15 +1418,15 @@ static int add_mounts_from_cmdline(void) { static int add_mounts_from_creds(bool prefix_sysroot) { _cleanup_free_ void *b = NULL; - struct mntent *me; size_t bs; - int r; + const char *cred; + int r, ret = 0; assert(in_initrd() || !prefix_sysroot); - r = read_credential_with_decryption( - in_initrd() && !prefix_sysroot ? "fstab.extra.initrd" : "fstab.extra", - &b, &bs); + cred = in_initrd() && !prefix_sysroot ? "fstab.extra.initrd" : "fstab.extra"; + + r = read_credential_with_decryption(cred, &b, &bs); if (r <= 0) return r; @@ -1431,20 +1435,29 @@ static int add_mounts_from_creds(bool prefix_sysroot) { if (!f) return log_oom(); - r = 0; + _cleanup_(mnt_free_tablep) struct libmnt_table *table = NULL; + _cleanup_(mnt_free_iterp) struct libmnt_iter *iter = NULL; - while ((me = getmntent(f))) - RET_GATHER(r, parse_fstab_one("/run/credentials", - me->mnt_fsname, - me->mnt_dir, - me->mnt_type, - me->mnt_opts, - me->mnt_passno, - /* prefix_sysroot = */ prefix_sysroot, - /* accept_root = */ true, - /* use_swap_enabled = */ true)); + r = libmount_parse_full(cred, f, &table, &iter); + if (r < 0) + return log_error_errno(r, "Failed to parse credential '%s' (as fstab): %m", cred); - return r; + for (;;) { + struct libmnt_fs *fs; + + r = mnt_table_next_fs(table, iter, &fs); + if (r < 0) + return log_error_errno(r, "Failed to get next fstab entry from credential '%s': %m", cred); + if (r > 0) /* EOF */ + return ret; + + RET_GATHER(ret, parse_fstab_one("/run/credentials", + mnt_fs_get_source(fs), mnt_fs_get_target(fs), + mnt_fs_get_fstype(fs), mnt_fs_get_options(fs), mnt_fs_get_passno(fs), + prefix_sysroot, + /* accept_root = */ true, + /* use_swap_enabled = */ true)); + } } static int parse_proc_cmdline_item(const char *key, const char *value, void *data) { diff --git a/src/fstab-generator/meson.build b/src/fstab-generator/meson.build index fb492158fc..fb5dccdd41 100644 --- a/src/fstab-generator/meson.build +++ b/src/fstab-generator/meson.build @@ -4,6 +4,7 @@ executables += [ generator_template + { 'name' : 'systemd-fstab-generator', 'sources' : files('fstab-generator.c'), + 'dependencies' : libmount, }, ] diff --git a/src/remount-fs/meson.build b/src/remount-fs/meson.build index 8761d25418..9ff7c9b8e4 100644 --- a/src/remount-fs/meson.build +++ b/src/remount-fs/meson.build @@ -4,5 +4,6 @@ executables += [ libexec_template + { 'name' : 'systemd-remount-fs', 'sources' : files('remount-fs.c'), + 'dependencies' : libmount, }, ] diff --git a/src/remount-fs/remount-fs.c b/src/remount-fs/remount-fs.c index 36bc6b7ef2..ceaa2676ca 100644 --- a/src/remount-fs/remount-fs.c +++ b/src/remount-fs/remount-fs.c @@ -1,6 +1,5 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ -#include #include #include #include @@ -11,6 +10,7 @@ #include "format-util.h" #include "fstab-util.h" #include "hashmap.h" +#include "libmount-util.h" #include "log.h" #include "main-func.h" #include "mount-setup.h" @@ -35,10 +35,8 @@ static int track_pid(Hashmap **h, const char *path, pid_t pid) { return log_oom(); r = hashmap_ensure_put(h, &trivial_hash_ops_value_free, PID_TO_PTR(pid), c); - if (r == -ENOMEM) - return log_oom(); if (r < 0) - return log_error_errno(r, "Failed to store pid " PID_FMT, pid); + return log_error_errno(r, "Failed to store pid " PID_FMT " for mount '%s': %m", pid, path); TAKE_PTR(c); return 0; @@ -48,6 +46,8 @@ static int do_remount(const char *path, bool force_rw, Hashmap **pids) { pid_t pid; int r; + assert(path); + log_debug("Remounting %s...", path); r = safe_fork(force_rw ? "(remount-rw)" : "(remount)", @@ -70,10 +70,10 @@ static int do_remount(const char *path, bool force_rw, Hashmap **pids) { } static int remount_by_fstab(Hashmap **ret_pids) { + _cleanup_(mnt_free_tablep) struct libmnt_table *table = NULL; + _cleanup_(mnt_free_iterp) struct libmnt_iter *iter = NULL; _cleanup_hashmap_free_ Hashmap *pids = NULL; - _cleanup_endmntent_ FILE *f = NULL; bool has_root = false; - struct mntent* me; int r; assert(ret_pids); @@ -81,24 +81,33 @@ static int remount_by_fstab(Hashmap **ret_pids) { if (!fstab_enabled()) return 0; - f = setmntent(fstab_path(), "re"); - if (!f) { - if (errno != ENOENT) - return log_error_errno(errno, "Failed to open %s: %m", fstab_path()); - + r = libmount_parse_fstab(&table, &iter); + if (r == -ENOENT) return 0; - } + if (r < 0) + return log_error_errno(r, "Failed to parse fstab: %m"); - while ((me = getmntent(f))) { - /* Remount the root fs, /usr, and all API VFSs */ - if (!mount_point_is_api(me->mnt_dir) && - !PATH_IN_SET(me->mnt_dir, "/", "/usr")) + for (;;) { + struct libmnt_fs *fs; + + r = mnt_table_next_fs(table, iter, &fs); + if (r < 0) + return log_error_errno(r, "Failed to get next entry from fstab: %m"); + if (r > 0) /* EOF */ + break; + + const char *target = mnt_fs_get_target(fs); + if (!target) continue; - if (path_equal(me->mnt_dir, "/")) - has_root = true; + /* Remount the root fs, /usr/, and all API VFSs */ - r = do_remount(me->mnt_dir, false, &pids); + if (path_equal(target, "/")) + has_root = true; + else if (!path_equal(target, "/usr") && !mount_point_is_api(target)) + continue; + + r = do_remount(target, /* force_rw = */ false, &pids); if (r < 0) return r; } @@ -132,7 +141,7 @@ static int run(int argc, char *argv[]) { log_warning_errno(r, "Failed to parse $SYSTEMD_REMOUNT_ROOT_RW, ignoring: %m"); if (r > 0) { - r = do_remount("/", true, &pids); + r = do_remount("/", /* force_rw = */ true, &pids); if (r < 0) return r; } diff --git a/src/shared/mount-util.h b/src/shared/mount-util.h index 3140762498..e45139dcf3 100644 --- a/src/shared/mount-util.h +++ b/src/shared/mount-util.h @@ -1,8 +1,6 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once -#include - #include "forward.h" typedef struct SubMount { @@ -37,9 +35,6 @@ static inline int mount_switch_root(const char *path, unsigned long mount_propag return mount_switch_root_full(path, mount_propagation_flag, false); } -DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(FILE*, endmntent, NULL); -#define _cleanup_endmntent_ _cleanup_(endmntentp) - int mount_verbose_full( int error_log_level, const char *what, diff --git a/test/units/TEST-81-GENERATORS.fstab-generator.sh b/test/units/TEST-81-GENERATORS.fstab-generator.sh index 95eba8a9f6..34a79c3d1b 100755 --- a/test/units/TEST-81-GENERATORS.fstab-generator.sh +++ b/test/units/TEST-81-GENERATORS.fstab-generator.sh @@ -49,10 +49,9 @@ FSTAB_GENERAL=( "/dev/test25 /x-systemd.validatefs xfs x-systemd.validatefs 0 0" # Incomplete, but valid entries - "/dev/incomplete1 /incomplete1" - "/dev/incomplete2 /incomplete2 ext4" - "/dev/incomplete3 /incomplete3 ext4 defaults" - "/dev/incomplete4 /incomplete4 ext4 defaults 0" + "/dev/incomplete1 /incomplete1 ext4" + "/dev/incomplete2 /incomplete2 ext4 defaults" + "/dev/incomplete3 /incomplete3 ext4 defaults 0" # Remote filesystems "/dev/remote1 /nfs nfs bg 0 0"