From f69c2926f896d628426447f2a0a81bfcd1f4a75d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 21 Jun 2021 22:56:58 +0200 Subject: [PATCH 01/15] meson: sort file list At least emacs thinks this is the right way. --- src/shared/meson.build | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/shared/meson.build b/src/shared/meson.build index 8d9cb32355..f35cec0a9c 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -142,10 +142,10 @@ shared_sources = files(''' install-printf.h install.c install.h - ipvlan-util.c - ipvlan-util.h ip-protocol-list.c ip-protocol-list.h + ipvlan-util.c + ipvlan-util.h journal-importer.c journal-importer.h journal-util.c @@ -264,10 +264,10 @@ shared_sources = files(''' user-record-show.h user-record.c user-record.h - userdb.c - userdb.h userdb-dropin.c userdb-dropin.h + userdb.c + userdb.h utmp-wtmp.h varlink.c varlink.h From 9c6535367d07650bb531beb6cce57497862a36da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 21 Jun 2021 19:44:35 +0200 Subject: [PATCH 02/15] basic,shared: move make_mount_point_inode_*() to shared/ Those pull in selinux for labelling, and we should avoid selinux in basic/. --- src/basic/mountpoint-util.c | 23 ---------------- src/basic/mountpoint-util.h | 4 --- src/shared/mount-util.c | 23 ++++++++++++++++ src/shared/mount-util.h | 5 ++++ src/test/test-mount-util.c | 49 +++++++++++++++++++++++++++++++++ src/test/test-mountpoint-util.c | 49 --------------------------------- 6 files changed, 77 insertions(+), 76 deletions(-) diff --git a/src/basic/mountpoint-util.c b/src/basic/mountpoint-util.c index 1d617e87b2..8c836a1b74 100644 --- a/src/basic/mountpoint-util.c +++ b/src/basic/mountpoint-util.c @@ -8,7 +8,6 @@ #include "fd-util.h" #include "fileio.h" #include "fs-util.h" -#include "label.h" #include "missing_stat.h" #include "missing_syscall.h" #include "mkdir.h" @@ -510,25 +509,3 @@ int mount_propagation_flags_from_string(const char *name, unsigned long *ret) { return -EINVAL; return 0; } - -int make_mount_point_inode_from_stat(const struct stat *st, const char *dest, mode_t mode) { - assert(st); - assert(dest); - - if (S_ISDIR(st->st_mode)) - return mkdir_label(dest, mode); - else - return mknod(dest, S_IFREG|(mode & ~0111), 0); -} - -int make_mount_point_inode_from_path(const char *source, const char *dest, mode_t mode) { - struct stat st; - - assert(source); - assert(dest); - - if (stat(source, &st) < 0) - return -errno; - - return make_mount_point_inode_from_stat(&st, dest, mode); -} diff --git a/src/basic/mountpoint-util.h b/src/basic/mountpoint-util.h index cebcec5e78..aadb2123d9 100644 --- a/src/basic/mountpoint-util.h +++ b/src/basic/mountpoint-util.h @@ -23,7 +23,3 @@ int dev_is_devtmpfs(void); const char *mount_propagation_flags_to_string(unsigned long flags); int mount_propagation_flags_from_string(const char *name, unsigned long *ret); - -/* Creates a mount point (not parents) based on the source path or stat - ie, a file or a directory */ -int make_mount_point_inode_from_stat(const struct stat *st, const char *dest, mode_t mode); -int make_mount_point_inode_from_path(const char *source, const char *dest, mode_t mode); diff --git a/src/shared/mount-util.c b/src/shared/mount-util.c index ef128dd595..ff95fbc569 100644 --- a/src/shared/mount-util.c +++ b/src/shared/mount-util.c @@ -16,6 +16,7 @@ #include "fileio.h" #include "fs-util.h" #include "hashmap.h" +#include "label.h" #include "libmount-util.h" #include "missing_mount.h" #include "missing_syscall.h" @@ -1071,3 +1072,25 @@ int remount_idmap( return 0; } + +int make_mount_point_inode_from_stat(const struct stat *st, const char *dest, mode_t mode) { + assert(st); + assert(dest); + + if (S_ISDIR(st->st_mode)) + return mkdir_label(dest, mode); + else + return mknod(dest, S_IFREG|(mode & ~0111), 0); +} + +int make_mount_point_inode_from_path(const char *source, const char *dest, mode_t mode) { + struct stat st; + + assert(source); + assert(dest); + + if (stat(source, &st) < 0) + return -errno; + + return make_mount_point_inode_from_stat(&st, dest, mode); +} diff --git a/src/shared/mount-util.h b/src/shared/mount-util.h index 0169083980..36501c2c4a 100644 --- a/src/shared/mount-util.h +++ b/src/shared/mount-util.h @@ -3,6 +3,7 @@ #include #include +#include #include #include "alloc-util.h" @@ -108,3 +109,7 @@ int mount_image_in_namespace(pid_t target, const char *propagate_path, const cha int make_mount_point(const char *path); int remount_idmap(const char *p, uid_t uid_shift, uid_t uid_range); + +/* Creates a mount point (not parents) based on the source path or stat - ie, a file or a directory */ +int make_mount_point_inode_from_stat(const struct stat *st, const char *dest, mode_t mode); +int make_mount_point_inode_from_path(const char *source, const char *dest, mode_t mode); diff --git a/src/test/test-mount-util.c b/src/test/test-mount-util.c index 0cbf68aa00..d3d004071b 100644 --- a/src/test/test-mount-util.c +++ b/src/test/test-mount-util.c @@ -7,7 +7,9 @@ #include "capability-util.h" #include "fd-util.h" #include "fileio.h" +#include "fs-util.h" #include "missing_mount.h" +#include "mkdir.h" #include "mount-util.h" #include "namespace-util.h" #include "path-util.h" @@ -217,6 +219,52 @@ static void test_bind_remount_one(void) { assert_se(wait_for_terminate_and_check("test-remount-one", pid, WAIT_LOG) == EXIT_SUCCESS); } +static void test_make_mount_point_inode(void) { + _cleanup_(rm_rf_physical_and_freep) char *d = NULL; + const char *src_file, *src_dir, *dst_file, *dst_dir; + struct stat st; + + log_info("/* %s */", __func__); + + assert_se(mkdtemp_malloc(NULL, &d) >= 0); + + src_file = strjoina(d, "/src/file"); + src_dir = strjoina(d, "/src/dir"); + dst_file = strjoina(d, "/dst/file"); + dst_dir = strjoina(d, "/dst/dir"); + + assert_se(mkdir_p(src_dir, 0755) >= 0); + assert_se(mkdir_parents(dst_file, 0755) >= 0); + assert_se(touch(src_file) >= 0); + + assert_se(make_mount_point_inode_from_path(src_file, dst_file, 0755) >= 0); + assert_se(make_mount_point_inode_from_path(src_dir, dst_dir, 0755) >= 0); + + assert_se(stat(dst_dir, &st) == 0); + assert_se(S_ISDIR(st.st_mode)); + assert_se(stat(dst_file, &st) == 0); + assert_se(S_ISREG(st.st_mode)); + assert_se(!(S_IXUSR & st.st_mode)); + assert_se(!(S_IXGRP & st.st_mode)); + assert_se(!(S_IXOTH & st.st_mode)); + + assert_se(unlink(dst_file) == 0); + assert_se(rmdir(dst_dir) == 0); + + assert_se(stat(src_file, &st) == 0); + assert_se(make_mount_point_inode_from_stat(&st, dst_file, 0755) >= 0); + assert_se(stat(src_dir, &st) == 0); + assert_se(make_mount_point_inode_from_stat(&st, dst_dir, 0755) >= 0); + + assert_se(stat(dst_dir, &st) == 0); + assert_se(S_ISDIR(st.st_mode)); + assert_se(stat(dst_file, &st) == 0); + assert_se(S_ISREG(st.st_mode)); + assert_se(!(S_IXUSR & st.st_mode)); + assert_se(!(S_IXGRP & st.st_mode)); + assert_se(!(S_IXOTH & st.st_mode)); +} + int main(int argc, char *argv[]) { test_setup_logging(LOG_DEBUG); @@ -224,6 +272,7 @@ int main(int argc, char *argv[]) { test_mount_flags_to_string(); test_bind_remount_recursive(); test_bind_remount_one(); + test_make_mount_point_inode(); return 0; } diff --git a/src/test/test-mountpoint-util.c b/src/test/test-mountpoint-util.c index 128daa6de8..983e1842d6 100644 --- a/src/test/test-mountpoint-util.c +++ b/src/test/test-mountpoint-util.c @@ -8,10 +8,8 @@ #include "def.h" #include "fd-util.h" #include "fileio.h" -#include "fs-util.h" #include "hashmap.h" #include "log.h" -#include "mkdir.h" #include "mountpoint-util.h" #include "path-util.h" #include "rm-rf.h" @@ -290,52 +288,6 @@ static void test_fd_is_mount_point(void) { assert_se(IN_SET(fd_is_mount_point(fd, "root/", 0), -ENOENT, 0)); } -static void test_make_mount_point_inode(void) { - _cleanup_(rm_rf_physical_and_freep) char *d = NULL; - const char *src_file, *src_dir, *dst_file, *dst_dir; - struct stat st; - - log_info("/* %s */", __func__); - - assert_se(mkdtemp_malloc(NULL, &d) >= 0); - - src_file = strjoina(d, "/src/file"); - src_dir = strjoina(d, "/src/dir"); - dst_file = strjoina(d, "/dst/file"); - dst_dir = strjoina(d, "/dst/dir"); - - assert_se(mkdir_p(src_dir, 0755) >= 0); - assert_se(mkdir_parents(dst_file, 0755) >= 0); - assert_se(touch(src_file) >= 0); - - assert_se(make_mount_point_inode_from_path(src_file, dst_file, 0755) >= 0); - assert_se(make_mount_point_inode_from_path(src_dir, dst_dir, 0755) >= 0); - - assert_se(stat(dst_dir, &st) == 0); - assert_se(S_ISDIR(st.st_mode)); - assert_se(stat(dst_file, &st) == 0); - assert_se(S_ISREG(st.st_mode)); - assert_se(!(S_IXUSR & st.st_mode)); - assert_se(!(S_IXGRP & st.st_mode)); - assert_se(!(S_IXOTH & st.st_mode)); - - assert_se(unlink(dst_file) == 0); - assert_se(rmdir(dst_dir) == 0); - - assert_se(stat(src_file, &st) == 0); - assert_se(make_mount_point_inode_from_stat(&st, dst_file, 0755) >= 0); - assert_se(stat(src_dir, &st) == 0); - assert_se(make_mount_point_inode_from_stat(&st, dst_dir, 0755) >= 0); - - assert_se(stat(dst_dir, &st) == 0); - assert_se(S_ISDIR(st.st_mode)); - assert_se(stat(dst_file, &st) == 0); - assert_se(S_ISREG(st.st_mode)); - assert_se(!(S_IXUSR & st.st_mode)); - assert_se(!(S_IXGRP & st.st_mode)); - assert_se(!(S_IXOTH & st.st_mode)); -} - int main(int argc, char *argv[]) { test_setup_logging(LOG_DEBUG); @@ -360,7 +312,6 @@ int main(int argc, char *argv[]) { test_mnt_id(); test_path_is_mount_point(); test_fd_is_mount_point(); - test_make_mount_point_inode(); return 0; } From 65ddc2c5ffd5b9d7798dfd4743423f08b4be6c2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 21 Jun 2021 20:24:00 +0200 Subject: [PATCH 03/15] basic: drop one btrfs-related function and move another This will become useful later, it is the first step to moving btrfs-util.[ch] out of src/basic/. --- src/basic/btrfs-util.c | 50 ++++++------------------ src/basic/btrfs-util.h | 3 -- src/basic/fd-util.c | 20 ++++++++++ src/basic/fd-util.h | 2 +- src/libsystemd/sd-journal/journal-file.c | 4 +- src/shared/discover-image.c | 4 +- src/shared/sleep-config.c | 12 +++--- 7 files changed, 45 insertions(+), 50 deletions(-) diff --git a/src/basic/btrfs-util.c b/src/basic/btrfs-util.c index adf9d03eb9..f4e291385b 100644 --- a/src/basic/btrfs-util.c +++ b/src/basic/btrfs-util.c @@ -69,17 +69,6 @@ static int extract_subvolume_name(const char *path, const char **subvolume) { return 0; } -int btrfs_is_filesystem(int fd) { - struct statfs sfs; - - assert(fd >= 0); - - if (fstatfs(fd, &sfs) < 0) - return -errno; - - return F_TYPE_EQUAL(sfs.f_type, BTRFS_SUPER_MAGIC); -} - int btrfs_is_subvol_fd(int fd) { struct stat st; @@ -93,7 +82,7 @@ int btrfs_is_subvol_fd(int fd) { if (!btrfs_might_be_subvol(&st)) return 0; - return btrfs_is_filesystem(fd); + return fd_is_fs_type(fd, BTRFS_SUPER_MAGIC); } int btrfs_is_subvol(const char *path) { @@ -286,7 +275,7 @@ int btrfs_get_block_device_fd(int fd, dev_t *dev) { assert(fd >= 0); assert(dev); - r = btrfs_is_filesystem(fd); + r = fd_is_fs_type(fd, BTRFS_SUPER_MAGIC); if (r < 0) return r; if (!r) @@ -361,7 +350,7 @@ int btrfs_subvol_get_id_fd(int fd, uint64_t *ret) { assert(fd >= 0); assert(ret); - r = btrfs_is_filesystem(fd); + r = fd_is_fs_type(fd, BTRFS_SUPER_MAGIC); if (r < 0) return r; if (!r) @@ -481,7 +470,7 @@ int btrfs_subvol_get_info_fd(int fd, uint64_t subvol_id, BtrfsSubvolInfo *ret) { if (r < 0) return r; } else { - r = btrfs_is_filesystem(fd); + r = fd_is_fs_type(fd, BTRFS_SUPER_MAGIC); if (r < 0) return r; if (!r) @@ -575,7 +564,7 @@ int btrfs_qgroup_get_quota_fd(int fd, uint64_t qgroupid, BtrfsQuotaInfo *ret) { if (r < 0) return r; } else { - r = btrfs_is_filesystem(fd); + r = fd_is_fs_type(fd, BTRFS_SUPER_MAGIC); if (r < 0) return r; if (!r) @@ -762,21 +751,6 @@ int btrfs_subvol_get_subtree_quota(const char *path, uint64_t subvol_id, BtrfsQu return btrfs_subvol_get_subtree_quota_fd(fd, subvol_id, ret); } -int btrfs_defrag_fd(int fd) { - int r; - - assert(fd >= 0); - - r = fd_verify_regular(fd); - if (r < 0) - return r; - - if (ioctl(fd, BTRFS_IOC_DEFRAG, NULL) < 0) - return -errno; - - return 0; -} - int btrfs_defrag(const char *p) { _cleanup_close_ int fd = -1; @@ -795,7 +769,7 @@ int btrfs_quota_enable_fd(int fd, bool b) { assert(fd >= 0); - r = btrfs_is_filesystem(fd); + r = fd_is_fs_type(fd, BTRFS_SUPER_MAGIC); if (r < 0) return r; if (!r) @@ -832,7 +806,7 @@ int btrfs_qgroup_set_limit_fd(int fd, uint64_t qgroupid, uint64_t referenced_max if (r < 0) return r; } else { - r = btrfs_is_filesystem(fd); + r = fd_is_fs_type(fd, BTRFS_SUPER_MAGIC); if (r < 0) return r; if (!r) @@ -924,7 +898,7 @@ static int qgroup_create_or_destroy(int fd, bool b, uint64_t qgroupid) { }; int r; - r = btrfs_is_filesystem(fd); + r = fd_is_fs_type(fd, BTRFS_SUPER_MAGIC); if (r < 0) return r; if (r == 0) @@ -1042,7 +1016,7 @@ static int qgroup_assign_or_unassign(int fd, bool b, uint64_t child, uint64_t pa }; int r; - r = btrfs_is_filesystem(fd); + r = fd_is_fs_type(fd, BTRFS_SUPER_MAGIC); if (r < 0) return r; if (r == 0) @@ -1269,7 +1243,7 @@ int btrfs_qgroup_copy_limits(int fd, uint64_t old_qgroupid, uint64_t new_qgroupi int r; - r = btrfs_is_filesystem(fd); + r = fd_is_fs_type(fd, BTRFS_SUPER_MAGIC); if (r < 0) return r; if (!r) @@ -1738,7 +1712,7 @@ int btrfs_qgroup_find_parents(int fd, uint64_t qgroupid, uint64_t **ret) { if (r < 0) return r; } else { - r = btrfs_is_filesystem(fd); + r = fd_is_fs_type(fd, BTRFS_SUPER_MAGIC); if (r < 0) return r; if (!r) @@ -1979,7 +1953,7 @@ int btrfs_subvol_get_parent(int fd, uint64_t subvol_id, uint64_t *ret) { if (r < 0) return r; } else { - r = btrfs_is_filesystem(fd); + r = fd_is_fs_type(fd, BTRFS_SUPER_MAGIC); if (r < 0) return r; if (!r) diff --git a/src/basic/btrfs-util.h b/src/basic/btrfs-util.h index 0f569b6f50..7b18f57719 100644 --- a/src/basic/btrfs-util.h +++ b/src/basic/btrfs-util.h @@ -42,8 +42,6 @@ typedef enum BtrfsRemoveFlags { BTRFS_REMOVE_QUOTA = 1 << 1, } BtrfsRemoveFlags; -int btrfs_is_filesystem(int fd); - int btrfs_is_subvol_fd(int fd); int btrfs_is_subvol(const char *path); @@ -53,7 +51,6 @@ int btrfs_clone_range(int infd, uint64_t in_offset, int ofd, uint64_t out_offset int btrfs_get_block_device_fd(int fd, dev_t *dev); int btrfs_get_block_device(const char *path, dev_t *dev); -int btrfs_defrag_fd(int fd); int btrfs_defrag(const char *p); int btrfs_quota_enable_fd(int fd, bool b); diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index 1a873601b2..ac6a37b567 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -2,6 +2,9 @@ #include #include +#include +#include +#include #include #include #include @@ -1057,3 +1060,20 @@ int read_nr_open(void) { /* If we fail, fall back to the hard-coded kernel limit of 1024 * 1024. */ return 1024 * 1024; } + +/* This is here because it's fd-related and is called from sd-journal code. Other btrfs-related utilities are + * in src/shared, but libsystemd must not link to libsystemd-shared, see docs/ARCHITECTURE.md. */ +int btrfs_defrag_fd(int fd) { + int r; + + assert(fd >= 0); + + r = fd_verify_regular(fd); + if (r < 0) + return r; + + if (ioctl(fd, BTRFS_IOC_DEFRAG, NULL) < 0) + return -errno; + + return 0; +} diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h index aa8e082b38..eb696762b6 100644 --- a/src/basic/fd-util.h +++ b/src/basic/fd-util.h @@ -107,5 +107,5 @@ static inline int make_null_stdio(void) { int fd_reopen(int fd, int flags); - int read_nr_open(void); +int btrfs_defrag_fd(int fd); diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 64aa132c7a..2c17435de2 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -13,7 +14,6 @@ #include "sd-event.h" #include "alloc-util.h" -#include "btrfs-util.h" #include "chattr-util.h" #include "compress.h" #include "env-util.h" @@ -3379,7 +3379,7 @@ static int journal_file_warn_btrfs(JournalFile *f) { * expense of data integrity features (which shouldn't be too * bad, given that we do our own checksumming). */ - r = btrfs_is_filesystem(f->fd); + r = fd_is_fs_type(f->fd, BTRFS_SUPER_MAGIC); if (r < 0) return log_warning_errno(r, "Failed to determine if journal is on btrfs: %m"); if (!r) diff --git a/src/shared/discover-image.c b/src/shared/discover-image.c index c572e8dae8..5c833afc78 100644 --- a/src/shared/discover-image.c +++ b/src/shared/discover-image.c @@ -4,6 +4,7 @@ #include #include #include +#include #include #include #include @@ -34,6 +35,7 @@ #include "os-util.h" #include "path-util.h" #include "rm-rf.h" +#include "stat-util.h" #include "string-table.h" #include "string-util.h" #include "strv.h" @@ -262,7 +264,7 @@ static int image_make( if (btrfs_might_be_subvol(st)) { - r = btrfs_is_filesystem(fd); + r = fd_is_fs_type(fd, BTRFS_SUPER_MAGIC); if (r < 0) return r; if (r) { diff --git a/src/shared/sleep-config.c b/src/shared/sleep-config.c index 645b7e242e..dbaecb3a0f 100644 --- a/src/shared/sleep-config.c +++ b/src/shared/sleep-config.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -28,6 +29,7 @@ #include "parse-util.h" #include "path-util.h" #include "sleep-config.h" +#include "stat-util.h" #include "stdio-util.h" #include "string-table.h" #include "string-util.h" @@ -232,7 +234,7 @@ static int calculate_swap_file_offset(const SwapEntry *swap, uint64_t *ret_offse _cleanup_close_ int fd = -1; _cleanup_free_ struct fiemap *fiemap = NULL; struct stat sb; - int r, btrfs; + int r; assert(swap); assert(swap->device); @@ -245,10 +247,10 @@ static int calculate_swap_file_offset(const SwapEntry *swap, uint64_t *ret_offse if (fstat(fd, &sb) < 0) return log_debug_errno(errno, "Failed to stat %s: %m", swap->device); - btrfs = btrfs_is_filesystem(fd); - if (btrfs < 0) - return log_debug_errno(btrfs, "Error checking %s for Btrfs filesystem: %m", swap->device); - if (btrfs > 0) { + r = fd_is_fs_type(fd, BTRFS_SUPER_MAGIC); + if (r < 0) + return log_debug_errno(r, "Error checking %s for Btrfs filesystem: %m", swap->device); + if (r > 0) { log_debug("%s: detection of swap file offset on Btrfs is not supported", swap->device); *ret_offset = UINT64_MAX; return 0; From 26861143329bc64dbc8801094a440732c3fbb1ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 21 Jun 2021 21:27:21 +0200 Subject: [PATCH 04/15] basic,shared: move quota-util.[ch] to src/shared/ No need for this to in basic/. --- src/basic/meson.build | 2 -- src/shared/meson.build | 2 ++ src/{basic => shared}/quota-util.c | 0 src/{basic => shared}/quota-util.h | 0 4 files changed, 2 insertions(+), 2 deletions(-) rename src/{basic => shared}/quota-util.c (100%) rename src/{basic => shared}/quota-util.h (100%) diff --git a/src/basic/meson.build b/src/basic/meson.build index d1aa86e7b5..3685b639ae 100644 --- a/src/basic/meson.build +++ b/src/basic/meson.build @@ -188,8 +188,6 @@ basic_sources = files(''' procfs-util.c procfs-util.h pthread-util.h - quota-util.c - quota-util.h random-util.c random-util.h ratelimit.c diff --git a/src/shared/meson.build b/src/shared/meson.build index f35cec0a9c..579abcc602 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -223,6 +223,8 @@ shared_sources = files(''' pwquality-util.h qrcode-util.c qrcode-util.h + quota-util.c + quota-util.h reboot-util.c reboot-util.h resize-fs.c diff --git a/src/basic/quota-util.c b/src/shared/quota-util.c similarity index 100% rename from src/basic/quota-util.c rename to src/shared/quota-util.c diff --git a/src/basic/quota-util.h b/src/shared/quota-util.h similarity index 100% rename from src/basic/quota-util.h rename to src/shared/quota-util.h From 37350b81b5ce7c94636ea637a84af479132b6726 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 21 Jun 2021 22:10:37 +0200 Subject: [PATCH 05/15] Move hwdb creation code to src/shared/ hwdb_update() is the main entry point, and it is called from "udevadm hwdb" and "systemd-hwdb", so it belongs in shared/. --- src/libsystemd/meson.build | 2 - src/libsystemd/sd-hwdb/hwdb-internal.h | 24 +++++++++ src/libsystemd/sd-hwdb/sd-hwdb.c | 49 ------------------- .../sd-hwdb => shared}/hwdb-util.c | 33 +++++++++++-- .../sd-hwdb => shared}/hwdb-util.h | 0 src/shared/meson.build | 2 + 6 files changed, 55 insertions(+), 55 deletions(-) rename src/{libsystemd/sd-hwdb => shared}/hwdb-util.c (96%) rename src/{libsystemd/sd-hwdb => shared}/hwdb-util.h (100%) diff --git a/src/libsystemd/meson.build b/src/libsystemd/meson.build index 5af8b75af2..489ed12a73 100644 --- a/src/libsystemd/meson.build +++ b/src/libsystemd/meson.build @@ -131,8 +131,6 @@ libsystemd_sources = files(''' sd-device/device-util.h sd-device/sd-device.c sd-hwdb/hwdb-internal.h - sd-hwdb/hwdb-util.c - sd-hwdb/hwdb-util.h sd-hwdb/sd-hwdb.c sd-netlink/generic-netlink.c sd-netlink/generic-netlink.h diff --git a/src/libsystemd/sd-hwdb/hwdb-internal.h b/src/libsystemd/sd-hwdb/hwdb-internal.h index 5c20688cd4..5ddc2211e6 100644 --- a/src/libsystemd/sd-hwdb/hwdb-internal.h +++ b/src/libsystemd/sd-hwdb/hwdb-internal.h @@ -3,10 +3,27 @@ #include +#include "def.h" +#include "hashmap.h" #include "sparse-endian.h" #define HWDB_SIG { 'K', 'S', 'L', 'P', 'H', 'H', 'R', 'H' } +struct sd_hwdb { + unsigned n_ref; + + FILE *f; + struct stat st; + union { + struct trie_header_f *head; + const char *map; + }; + + OrderedHashmap *properties; + Iterator properties_iterator; + bool properties_modified; +}; + /* on-disk trie objects */ struct trie_header_f { uint8_t signature[8]; @@ -63,3 +80,10 @@ struct trie_value_entry2_f { le16_t file_priority; le16_t padding; } _packed_; + +#define hwdb_bin_paths \ + "/etc/systemd/hwdb/hwdb.bin\0" \ + "/etc/udev/hwdb.bin\0" \ + "/usr/lib/systemd/hwdb/hwdb.bin\0" \ + _CONF_PATHS_SPLIT_USR_NULSTR("systemd/hwdb/hwdb.bin") \ + UDEVLIBEXECDIR "/hwdb.bin\0" diff --git a/src/libsystemd/sd-hwdb/sd-hwdb.c b/src/libsystemd/sd-hwdb/sd-hwdb.c index cb3c77ce96..53601765fe 100644 --- a/src/libsystemd/sd-hwdb/sd-hwdb.c +++ b/src/libsystemd/sd-hwdb/sd-hwdb.c @@ -17,26 +17,10 @@ #include "fd-util.h" #include "hashmap.h" #include "hwdb-internal.h" -#include "hwdb-util.h" #include "nulstr-util.h" #include "string-util.h" #include "time-util.h" -struct sd_hwdb { - unsigned n_ref; - - FILE *f; - struct stat st; - union { - struct trie_header_f *head; - const char *map; - }; - - OrderedHashmap *properties; - Iterator properties_iterator; - bool properties_modified; -}; - struct linebuf { char bytes[LINE_MAX]; size_t size; @@ -296,15 +280,6 @@ static int trie_search_f(sd_hwdb *hwdb, const char *search) { return 0; } -static const char hwdb_bin_paths[] = - "/etc/systemd/hwdb/hwdb.bin\0" - "/etc/udev/hwdb.bin\0" - "/usr/lib/systemd/hwdb/hwdb.bin\0" -#if HAVE_SPLIT_USR - "/lib/systemd/hwdb/hwdb.bin\0" -#endif - UDEVLIBEXECDIR "/hwdb.bin\0"; - _public_ int sd_hwdb_new(sd_hwdb **ret) { _cleanup_(sd_hwdb_unrefp) sd_hwdb *hwdb = NULL; const char *hwdb_bin_path; @@ -372,30 +347,6 @@ static sd_hwdb *hwdb_free(sd_hwdb *hwdb) { DEFINE_PUBLIC_TRIVIAL_REF_UNREF_FUNC(sd_hwdb, sd_hwdb, hwdb_free) -bool hwdb_validate(sd_hwdb *hwdb) { - bool found = false; - const char* p; - struct stat st; - - if (!hwdb) - return false; - if (!hwdb->f) - return false; - - /* if hwdb.bin doesn't exist anywhere, we need to update */ - NULSTR_FOREACH(p, hwdb_bin_paths) - if (stat(p, &st) >= 0) { - found = true; - break; - } - if (!found) - return true; - - if (timespec_load(&hwdb->st.st_mtim) != timespec_load(&st.st_mtim)) - return true; - return false; -} - static int properties_prepare(sd_hwdb *hwdb, const char *modalias) { assert(hwdb); assert(modalias); diff --git a/src/libsystemd/sd-hwdb/hwdb-util.c b/src/shared/hwdb-util.c similarity index 96% rename from src/libsystemd/sd-hwdb/hwdb-util.c rename to src/shared/hwdb-util.c index 8964a40e38..d7626aed95 100644 --- a/src/libsystemd/sd-hwdb/hwdb-util.c +++ b/src/shared/hwdb-util.c @@ -13,6 +13,7 @@ #include "hwdb-util.h" #include "label.h" #include "mkdir.h" +#include "nulstr-util.h" #include "path-util.h" #include "sort-util.h" #include "strbuf.h" @@ -586,10 +587,10 @@ int hwdb_update(const char *root, const char *hwdb_bin_dir, bool strict, bool co uint16_t file_priority = 1; int r = 0, err; - /* The argument 'compat' controls the format version of database. If false, then hwdb.bin will be created with - * additional information such that priority, line number, and filename of database source. If true, then hwdb.bin - * will be created without the information. systemd-hwdb command should set the argument false, and 'udevadm hwdb' - * command should set it true. */ + /* The argument 'compat' controls the format version of database. If false, then hwdb.bin will be + * created with additional information such that priority, line number, and filename of database + * source. If true, then hwdb.bin will be created without the information. systemd-hwdb command + * should set the argument false, and 'udevadm hwdb' command should set it true. */ trie = new0(struct trie, 1); if (!trie) @@ -666,3 +667,27 @@ int hwdb_query(const char *modalias) { return 0; } + +bool hwdb_validate(sd_hwdb *hwdb) { + bool found = false; + const char* p; + struct stat st; + + if (!hwdb) + return false; + if (!hwdb->f) + return false; + + /* if hwdb.bin doesn't exist anywhere, we need to update */ + NULSTR_FOREACH(p, hwdb_bin_paths) + if (stat(p, &st) >= 0) { + found = true; + break; + } + if (!found) + return true; + + if (timespec_load(&hwdb->st.st_mtim) != timespec_load(&st.st_mtim)) + return true; + return false; +} diff --git a/src/libsystemd/sd-hwdb/hwdb-util.h b/src/shared/hwdb-util.h similarity index 100% rename from src/libsystemd/sd-hwdb/hwdb-util.h rename to src/shared/hwdb-util.h diff --git a/src/shared/meson.build b/src/shared/meson.build index 579abcc602..c478d46f31 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -129,6 +129,8 @@ shared_sources = files(''' group-record.h hostname-setup.c hostname-setup.h + hwdb-util.c + hwdb-util.h id128-print.c id128-print.h idn-util.c From 6a818c3cb4cba768dc42efa84db2fbd938f5def0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 21 Jun 2021 22:54:12 +0200 Subject: [PATCH 06/15] basic: move acquire_data_fd() and fd_duplicate_data_fd() to new data-fd-util.c fd_duplicate_data_fd() is renamed to copy_data_fd(). This makes the two functions have nicely similar names. Now fd-util.[ch] is again about low-level file descriptor manipulations. copy_data_fd() is a complex function that internally wraps the other functions in copy.c. I want to move copy.c and the whole cluster of related code from basic/ to shared/ later on, and this is a preparatory step for that. --- src/basic/data-fd-util.c | 350 +++++++++++++++++++++++++++++++++++ src/basic/data-fd-util.h | 7 + src/basic/fd-util.c | 339 --------------------------------- src/basic/fd-util.h | 4 - src/basic/meson.build | 2 + src/basic/terminal-util.c | 1 - src/core/dbus-manager.c | 1 + src/core/execute.c | 1 + src/home/homed-home.c | 1 + src/oom/oomd-manager-bus.c | 1 + src/portable/portable.c | 3 +- src/test/meson.build | 2 + src/test/test-data-fd-util.c | 154 +++++++++++++++ src/test/test-fd-util.c | 138 +------------- 14 files changed, 522 insertions(+), 482 deletions(-) create mode 100644 src/basic/data-fd-util.c create mode 100644 src/basic/data-fd-util.h create mode 100644 src/test/test-data-fd-util.c diff --git a/src/basic/data-fd-util.c b/src/basic/data-fd-util.c new file mode 100644 index 0000000000..a827ef5a8b --- /dev/null +++ b/src/basic/data-fd-util.c @@ -0,0 +1,350 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include +#include +#include +#include + +#include "alloc-util.h" +#include "copy.h" +#include "data-fd-util.h" +#include "fd-util.h" +#include "fs-util.h" +#include "io-util.h" +#include "memfd-util.h" +#include "tmpfile-util.h" + +/* When the data is smaller or equal to 64K, try to place the copy in a memfd/pipe */ +#define DATA_FD_MEMORY_LIMIT (64U*1024U) + +/* If memfd/pipe didn't work out, then let's use a file in /tmp up to a size of 1M. If it's large than that use /var/tmp instead. */ +#define DATA_FD_TMP_LIMIT (1024U*1024U) + +int acquire_data_fd(const void *data, size_t size, unsigned flags) { + _cleanup_close_pair_ int pipefds[2] = { -1, -1 }; + char pattern[] = "/dev/shm/data-fd-XXXXXX"; + _cleanup_close_ int fd = -1; + int isz = 0, r; + ssize_t n; + off_t f; + + assert(data || size == 0); + + /* Acquire a read-only file descriptor that when read from returns the specified data. This is much more + * complex than I wish it was. But here's why: + * + * a) First we try to use memfds. They are the best option, as we can seal them nicely to make them + * read-only. Unfortunately they require kernel 3.17, and – at the time of writing – we still support 3.14. + * + * b) Then, we try classic pipes. They are the second best options, as we can close the writing side, retaining + * a nicely read-only fd in the reading side. However, they are by default quite small, and unprivileged + * clients can only bump their size to a system-wide limit, which might be quite low. + * + * c) Then, we try an O_TMPFILE file in /dev/shm (that dir is the only suitable one known to exist from + * earliest boot on). To make it read-only we open the fd a second time with O_RDONLY via + * /proc/self/. Unfortunately O_TMPFILE is not available on older kernels on tmpfs. + * + * d) Finally, we try creating a regular file in /dev/shm, which we then delete. + * + * It sucks a bit that depending on the situation we return very different objects here, but that's Linux I + * figure. */ + + if (size == 0 && ((flags & ACQUIRE_NO_DEV_NULL) == 0)) { + /* As a special case, return /dev/null if we have been called for an empty data block */ + r = open("/dev/null", O_RDONLY|O_CLOEXEC|O_NOCTTY); + if (r < 0) + return -errno; + + return r; + } + + if ((flags & ACQUIRE_NO_MEMFD) == 0) { + fd = memfd_new("data-fd"); + if (fd < 0) + goto try_pipe; + + n = write(fd, data, size); + if (n < 0) + return -errno; + if ((size_t) n != size) + return -EIO; + + f = lseek(fd, 0, SEEK_SET); + if (f != 0) + return -errno; + + r = memfd_set_sealed(fd); + if (r < 0) + return r; + + return TAKE_FD(fd); + } + +try_pipe: + if ((flags & ACQUIRE_NO_PIPE) == 0) { + if (pipe2(pipefds, O_CLOEXEC|O_NONBLOCK) < 0) + return -errno; + + isz = fcntl(pipefds[1], F_GETPIPE_SZ, 0); + if (isz < 0) + return -errno; + + if ((size_t) isz < size) { + isz = (int) size; + if (isz < 0 || (size_t) isz != size) + return -E2BIG; + + /* Try to bump the pipe size */ + (void) fcntl(pipefds[1], F_SETPIPE_SZ, isz); + + /* See if that worked */ + isz = fcntl(pipefds[1], F_GETPIPE_SZ, 0); + if (isz < 0) + return -errno; + + if ((size_t) isz < size) + goto try_dev_shm; + } + + n = write(pipefds[1], data, size); + if (n < 0) + return -errno; + if ((size_t) n != size) + return -EIO; + + (void) fd_nonblock(pipefds[0], false); + + return TAKE_FD(pipefds[0]); + } + +try_dev_shm: + if ((flags & ACQUIRE_NO_TMPFILE) == 0) { + fd = open("/dev/shm", O_RDWR|O_TMPFILE|O_CLOEXEC, 0500); + if (fd < 0) + goto try_dev_shm_without_o_tmpfile; + + n = write(fd, data, size); + if (n < 0) + return -errno; + if ((size_t) n != size) + return -EIO; + + /* Let's reopen the thing, in order to get an O_RDONLY fd for the original O_RDWR one */ + return fd_reopen(fd, O_RDONLY|O_CLOEXEC); + } + +try_dev_shm_without_o_tmpfile: + if ((flags & ACQUIRE_NO_REGULAR) == 0) { + fd = mkostemp_safe(pattern); + if (fd < 0) + return fd; + + n = write(fd, data, size); + if (n < 0) { + r = -errno; + goto unlink_and_return; + } + if ((size_t) n != size) { + r = -EIO; + goto unlink_and_return; + } + + /* Let's reopen the thing, in order to get an O_RDONLY fd for the original O_RDWR one */ + r = open(pattern, O_RDONLY|O_CLOEXEC); + if (r < 0) + r = -errno; + + unlink_and_return: + (void) unlink(pattern); + return r; + } + + return -EOPNOTSUPP; +} + +int copy_data_fd(int fd) { + _cleanup_close_ int copy_fd = -1, tmp_fd = -1; + _cleanup_free_ void *remains = NULL; + size_t remains_size = 0; + const char *td; + struct stat st; + int r; + + /* Creates a 'data' fd from the specified source fd, containing all the same data in a read-only fashion, but + * independent of it (i.e. the source fd can be closed and unmounted after this call succeeded). Tries to be + * somewhat smart about where to place the data. In the best case uses a memfd(). If memfd() are not supported + * uses a pipe instead. For larger data will use an unlinked file in /tmp, and for even larger data one in + * /var/tmp. */ + + if (fstat(fd, &st) < 0) + return -errno; + + /* For now, let's only accept regular files, sockets, pipes and char devices */ + if (S_ISDIR(st.st_mode)) + return -EISDIR; + if (S_ISLNK(st.st_mode)) + return -ELOOP; + if (!S_ISREG(st.st_mode) && !S_ISSOCK(st.st_mode) && !S_ISFIFO(st.st_mode) && !S_ISCHR(st.st_mode)) + return -EBADFD; + + /* If we have reason to believe the data is bounded in size, then let's use memfds or pipes as backing fd. Note + * that we use the reported regular file size only as a hint, given that there are plenty special files in + * /proc and /sys which report a zero file size but can be read from. */ + + if (!S_ISREG(st.st_mode) || st.st_size < DATA_FD_MEMORY_LIMIT) { + + /* Try a memfd first */ + copy_fd = memfd_new("data-fd"); + if (copy_fd >= 0) { + off_t f; + + r = copy_bytes(fd, copy_fd, DATA_FD_MEMORY_LIMIT, 0); + if (r < 0) + return r; + + f = lseek(copy_fd, 0, SEEK_SET); + if (f != 0) + return -errno; + + if (r == 0) { + /* Did it fit into the limit? If so, we are done. */ + r = memfd_set_sealed(copy_fd); + if (r < 0) + return r; + + return TAKE_FD(copy_fd); + } + + /* Hmm, pity, this didn't fit. Let's fall back to /tmp then, see below */ + + } else { + _cleanup_(close_pairp) int pipefds[2] = { -1, -1 }; + int isz; + + /* If memfds aren't available, use a pipe. Set O_NONBLOCK so that we will get EAGAIN rather + * then block indefinitely when we hit the pipe size limit */ + + if (pipe2(pipefds, O_CLOEXEC|O_NONBLOCK) < 0) + return -errno; + + isz = fcntl(pipefds[1], F_GETPIPE_SZ, 0); + if (isz < 0) + return -errno; + + /* Try to enlarge the pipe size if necessary */ + if ((size_t) isz < DATA_FD_MEMORY_LIMIT) { + + (void) fcntl(pipefds[1], F_SETPIPE_SZ, DATA_FD_MEMORY_LIMIT); + + isz = fcntl(pipefds[1], F_GETPIPE_SZ, 0); + if (isz < 0) + return -errno; + } + + if ((size_t) isz >= DATA_FD_MEMORY_LIMIT) { + + r = copy_bytes_full(fd, pipefds[1], DATA_FD_MEMORY_LIMIT, 0, &remains, &remains_size, NULL, NULL); + if (r < 0 && r != -EAGAIN) + return r; /* If we get EAGAIN it could be because of the source or because of + * the destination fd, we can't know, as sendfile() and friends won't + * tell us. Hence, treat this as reason to fall back, just to be + * sure. */ + if (r == 0) { + /* Everything fit in, yay! */ + (void) fd_nonblock(pipefds[0], false); + + return TAKE_FD(pipefds[0]); + } + + /* Things didn't fit in. But we read data into the pipe, let's remember that, so that + * when writing the new file we incorporate this first. */ + copy_fd = TAKE_FD(pipefds[0]); + } + } + } + + /* If we have reason to believe this will fit fine in /tmp, then use that as first fallback. */ + if ((!S_ISREG(st.st_mode) || st.st_size < DATA_FD_TMP_LIMIT) && + (DATA_FD_MEMORY_LIMIT + remains_size) < DATA_FD_TMP_LIMIT) { + off_t f; + + tmp_fd = open_tmpfile_unlinkable(NULL /* NULL as directory means /tmp */, O_RDWR|O_CLOEXEC); + if (tmp_fd < 0) + return tmp_fd; + + if (copy_fd >= 0) { + /* If we tried a memfd/pipe first and it ended up being too large, then copy this into the + * temporary file first. */ + + r = copy_bytes(copy_fd, tmp_fd, UINT64_MAX, 0); + if (r < 0) + return r; + + assert(r == 0); + } + + if (remains_size > 0) { + /* If there were remaining bytes (i.e. read into memory, but not written out yet) from the + * failed copy operation, let's flush them out next. */ + + r = loop_write(tmp_fd, remains, remains_size, false); + if (r < 0) + return r; + } + + r = copy_bytes(fd, tmp_fd, DATA_FD_TMP_LIMIT - DATA_FD_MEMORY_LIMIT - remains_size, COPY_REFLINK); + if (r < 0) + return r; + if (r == 0) + goto finish; /* Yay, it fit in */ + + /* It didn't fit in. Let's not forget to use what we already used */ + f = lseek(tmp_fd, 0, SEEK_SET); + if (f != 0) + return -errno; + + CLOSE_AND_REPLACE(copy_fd, tmp_fd); + + remains = mfree(remains); + remains_size = 0; + } + + /* As last fallback use /var/tmp */ + r = var_tmp_dir(&td); + if (r < 0) + return r; + + tmp_fd = open_tmpfile_unlinkable(td, O_RDWR|O_CLOEXEC); + if (tmp_fd < 0) + return tmp_fd; + + if (copy_fd >= 0) { + /* If we tried a memfd/pipe first, or a file in /tmp, and it ended up being too large, than copy this + * into the temporary file first. */ + r = copy_bytes(copy_fd, tmp_fd, UINT64_MAX, COPY_REFLINK); + if (r < 0) + return r; + + assert(r == 0); + } + + if (remains_size > 0) { + /* Then, copy in any read but not yet written bytes. */ + r = loop_write(tmp_fd, remains, remains_size, false); + if (r < 0) + return r; + } + + /* Copy in the rest */ + r = copy_bytes(fd, tmp_fd, UINT64_MAX, COPY_REFLINK); + if (r < 0) + return r; + + assert(r == 0); + +finish: + /* Now convert the O_RDWR file descriptor into an O_RDONLY one (and as side effect seek to the beginning of the + * file again */ + + return fd_reopen(tmp_fd, O_RDONLY|O_CLOEXEC); +} diff --git a/src/basic/data-fd-util.h b/src/basic/data-fd-util.h new file mode 100644 index 0000000000..827e149662 --- /dev/null +++ b/src/basic/data-fd-util.h @@ -0,0 +1,7 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +#pragma once + +#include + +int acquire_data_fd(const void *data, size_t size, unsigned flags); +int copy_data_fd(int fd); diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index ac6a37b567..008f474344 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -10,14 +10,12 @@ #include #include "alloc-util.h" -#include "copy.h" #include "dirent-util.h" #include "fd-util.h" #include "fileio.h" #include "fs-util.h" #include "io-util.h" #include "macro.h" -#include "memfd-util.h" #include "missing_fcntl.h" #include "missing_syscall.h" #include "parse-util.h" @@ -523,343 +521,6 @@ int move_fd(int from, int to, int cloexec) { return to; } -int acquire_data_fd(const void *data, size_t size, unsigned flags) { - - _cleanup_close_pair_ int pipefds[2] = { -1, -1 }; - char pattern[] = "/dev/shm/data-fd-XXXXXX"; - _cleanup_close_ int fd = -1; - int isz = 0, r; - ssize_t n; - off_t f; - - assert(data || size == 0); - - /* Acquire a read-only file descriptor that when read from returns the specified data. This is much more - * complex than I wish it was. But here's why: - * - * a) First we try to use memfds. They are the best option, as we can seal them nicely to make them - * read-only. Unfortunately they require kernel 3.17, and – at the time of writing – we still support 3.14. - * - * b) Then, we try classic pipes. They are the second best options, as we can close the writing side, retaining - * a nicely read-only fd in the reading side. However, they are by default quite small, and unprivileged - * clients can only bump their size to a system-wide limit, which might be quite low. - * - * c) Then, we try an O_TMPFILE file in /dev/shm (that dir is the only suitable one known to exist from - * earliest boot on). To make it read-only we open the fd a second time with O_RDONLY via - * /proc/self/. Unfortunately O_TMPFILE is not available on older kernels on tmpfs. - * - * d) Finally, we try creating a regular file in /dev/shm, which we then delete. - * - * It sucks a bit that depending on the situation we return very different objects here, but that's Linux I - * figure. */ - - if (size == 0 && ((flags & ACQUIRE_NO_DEV_NULL) == 0)) { - /* As a special case, return /dev/null if we have been called for an empty data block */ - r = open("/dev/null", O_RDONLY|O_CLOEXEC|O_NOCTTY); - if (r < 0) - return -errno; - - return r; - } - - if ((flags & ACQUIRE_NO_MEMFD) == 0) { - fd = memfd_new("data-fd"); - if (fd < 0) - goto try_pipe; - - n = write(fd, data, size); - if (n < 0) - return -errno; - if ((size_t) n != size) - return -EIO; - - f = lseek(fd, 0, SEEK_SET); - if (f != 0) - return -errno; - - r = memfd_set_sealed(fd); - if (r < 0) - return r; - - return TAKE_FD(fd); - } - -try_pipe: - if ((flags & ACQUIRE_NO_PIPE) == 0) { - if (pipe2(pipefds, O_CLOEXEC|O_NONBLOCK) < 0) - return -errno; - - isz = fcntl(pipefds[1], F_GETPIPE_SZ, 0); - if (isz < 0) - return -errno; - - if ((size_t) isz < size) { - isz = (int) size; - if (isz < 0 || (size_t) isz != size) - return -E2BIG; - - /* Try to bump the pipe size */ - (void) fcntl(pipefds[1], F_SETPIPE_SZ, isz); - - /* See if that worked */ - isz = fcntl(pipefds[1], F_GETPIPE_SZ, 0); - if (isz < 0) - return -errno; - - if ((size_t) isz < size) - goto try_dev_shm; - } - - n = write(pipefds[1], data, size); - if (n < 0) - return -errno; - if ((size_t) n != size) - return -EIO; - - (void) fd_nonblock(pipefds[0], false); - - return TAKE_FD(pipefds[0]); - } - -try_dev_shm: - if ((flags & ACQUIRE_NO_TMPFILE) == 0) { - fd = open("/dev/shm", O_RDWR|O_TMPFILE|O_CLOEXEC, 0500); - if (fd < 0) - goto try_dev_shm_without_o_tmpfile; - - n = write(fd, data, size); - if (n < 0) - return -errno; - if ((size_t) n != size) - return -EIO; - - /* Let's reopen the thing, in order to get an O_RDONLY fd for the original O_RDWR one */ - return fd_reopen(fd, O_RDONLY|O_CLOEXEC); - } - -try_dev_shm_without_o_tmpfile: - if ((flags & ACQUIRE_NO_REGULAR) == 0) { - fd = mkostemp_safe(pattern); - if (fd < 0) - return fd; - - n = write(fd, data, size); - if (n < 0) { - r = -errno; - goto unlink_and_return; - } - if ((size_t) n != size) { - r = -EIO; - goto unlink_and_return; - } - - /* Let's reopen the thing, in order to get an O_RDONLY fd for the original O_RDWR one */ - r = open(pattern, O_RDONLY|O_CLOEXEC); - if (r < 0) - r = -errno; - - unlink_and_return: - (void) unlink(pattern); - return r; - } - - return -EOPNOTSUPP; -} - -/* When the data is smaller or equal to 64K, try to place the copy in a memfd/pipe */ -#define DATA_FD_MEMORY_LIMIT (64U*1024U) - -/* If memfd/pipe didn't work out, then let's use a file in /tmp up to a size of 1M. If it's large than that use /var/tmp instead. */ -#define DATA_FD_TMP_LIMIT (1024U*1024U) - -int fd_duplicate_data_fd(int fd) { - - _cleanup_close_ int copy_fd = -1, tmp_fd = -1; - _cleanup_free_ void *remains = NULL; - size_t remains_size = 0; - const char *td; - struct stat st; - int r; - - /* Creates a 'data' fd from the specified source fd, containing all the same data in a read-only fashion, but - * independent of it (i.e. the source fd can be closed and unmounted after this call succeeded). Tries to be - * somewhat smart about where to place the data. In the best case uses a memfd(). If memfd() are not supported - * uses a pipe instead. For larger data will use an unlinked file in /tmp, and for even larger data one in - * /var/tmp. */ - - if (fstat(fd, &st) < 0) - return -errno; - - /* For now, let's only accept regular files, sockets, pipes and char devices */ - if (S_ISDIR(st.st_mode)) - return -EISDIR; - if (S_ISLNK(st.st_mode)) - return -ELOOP; - if (!S_ISREG(st.st_mode) && !S_ISSOCK(st.st_mode) && !S_ISFIFO(st.st_mode) && !S_ISCHR(st.st_mode)) - return -EBADFD; - - /* If we have reason to believe the data is bounded in size, then let's use memfds or pipes as backing fd. Note - * that we use the reported regular file size only as a hint, given that there are plenty special files in - * /proc and /sys which report a zero file size but can be read from. */ - - if (!S_ISREG(st.st_mode) || st.st_size < DATA_FD_MEMORY_LIMIT) { - - /* Try a memfd first */ - copy_fd = memfd_new("data-fd"); - if (copy_fd >= 0) { - off_t f; - - r = copy_bytes(fd, copy_fd, DATA_FD_MEMORY_LIMIT, 0); - if (r < 0) - return r; - - f = lseek(copy_fd, 0, SEEK_SET); - if (f != 0) - return -errno; - - if (r == 0) { - /* Did it fit into the limit? If so, we are done. */ - r = memfd_set_sealed(copy_fd); - if (r < 0) - return r; - - return TAKE_FD(copy_fd); - } - - /* Hmm, pity, this didn't fit. Let's fall back to /tmp then, see below */ - - } else { - _cleanup_(close_pairp) int pipefds[2] = { -1, -1 }; - int isz; - - /* If memfds aren't available, use a pipe. Set O_NONBLOCK so that we will get EAGAIN rather - * then block indefinitely when we hit the pipe size limit */ - - if (pipe2(pipefds, O_CLOEXEC|O_NONBLOCK) < 0) - return -errno; - - isz = fcntl(pipefds[1], F_GETPIPE_SZ, 0); - if (isz < 0) - return -errno; - - /* Try to enlarge the pipe size if necessary */ - if ((size_t) isz < DATA_FD_MEMORY_LIMIT) { - - (void) fcntl(pipefds[1], F_SETPIPE_SZ, DATA_FD_MEMORY_LIMIT); - - isz = fcntl(pipefds[1], F_GETPIPE_SZ, 0); - if (isz < 0) - return -errno; - } - - if ((size_t) isz >= DATA_FD_MEMORY_LIMIT) { - - r = copy_bytes_full(fd, pipefds[1], DATA_FD_MEMORY_LIMIT, 0, &remains, &remains_size, NULL, NULL); - if (r < 0 && r != -EAGAIN) - return r; /* If we get EAGAIN it could be because of the source or because of - * the destination fd, we can't know, as sendfile() and friends won't - * tell us. Hence, treat this as reason to fall back, just to be - * sure. */ - if (r == 0) { - /* Everything fit in, yay! */ - (void) fd_nonblock(pipefds[0], false); - - return TAKE_FD(pipefds[0]); - } - - /* Things didn't fit in. But we read data into the pipe, let's remember that, so that - * when writing the new file we incorporate this first. */ - copy_fd = TAKE_FD(pipefds[0]); - } - } - } - - /* If we have reason to believe this will fit fine in /tmp, then use that as first fallback. */ - if ((!S_ISREG(st.st_mode) || st.st_size < DATA_FD_TMP_LIMIT) && - (DATA_FD_MEMORY_LIMIT + remains_size) < DATA_FD_TMP_LIMIT) { - off_t f; - - tmp_fd = open_tmpfile_unlinkable(NULL /* NULL as directory means /tmp */, O_RDWR|O_CLOEXEC); - if (tmp_fd < 0) - return tmp_fd; - - if (copy_fd >= 0) { - /* If we tried a memfd/pipe first and it ended up being too large, then copy this into the - * temporary file first. */ - - r = copy_bytes(copy_fd, tmp_fd, UINT64_MAX, 0); - if (r < 0) - return r; - - assert(r == 0); - } - - if (remains_size > 0) { - /* If there were remaining bytes (i.e. read into memory, but not written out yet) from the - * failed copy operation, let's flush them out next. */ - - r = loop_write(tmp_fd, remains, remains_size, false); - if (r < 0) - return r; - } - - r = copy_bytes(fd, tmp_fd, DATA_FD_TMP_LIMIT - DATA_FD_MEMORY_LIMIT - remains_size, COPY_REFLINK); - if (r < 0) - return r; - if (r == 0) - goto finish; /* Yay, it fit in */ - - /* It didn't fit in. Let's not forget to use what we already used */ - f = lseek(tmp_fd, 0, SEEK_SET); - if (f != 0) - return -errno; - - CLOSE_AND_REPLACE(copy_fd, tmp_fd); - - remains = mfree(remains); - remains_size = 0; - } - - /* As last fallback use /var/tmp */ - r = var_tmp_dir(&td); - if (r < 0) - return r; - - tmp_fd = open_tmpfile_unlinkable(td, O_RDWR|O_CLOEXEC); - if (tmp_fd < 0) - return tmp_fd; - - if (copy_fd >= 0) { - /* If we tried a memfd/pipe first, or a file in /tmp, and it ended up being too large, than copy this - * into the temporary file first. */ - r = copy_bytes(copy_fd, tmp_fd, UINT64_MAX, COPY_REFLINK); - if (r < 0) - return r; - - assert(r == 0); - } - - if (remains_size > 0) { - /* Then, copy in any read but not yet written bytes. */ - r = loop_write(tmp_fd, remains, remains_size, false); - if (r < 0) - return r; - } - - /* Copy in the rest */ - r = copy_bytes(fd, tmp_fd, UINT64_MAX, COPY_REFLINK); - if (r < 0) - return r; - - assert(r == 0); - -finish: - /* Now convert the O_RDWR file descriptor into an O_RDONLY one (and as side effect seek to the beginning of the - * file again */ - - return fd_reopen(tmp_fd, O_RDONLY|O_CLOEXEC); -} - int fd_move_above_stdio(int fd) { int flags, copy; PROTECT_ERRNO; diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h index eb696762b6..9529a4723d 100644 --- a/src/basic/fd-util.h +++ b/src/basic/fd-util.h @@ -76,10 +76,6 @@ enum { ACQUIRE_NO_REGULAR = 1 << 4, }; -int acquire_data_fd(const void *data, size_t size, unsigned flags); - -int fd_duplicate_data_fd(int fd); - int fd_move_above_stdio(int fd); int rearrange_stdio(int original_input_fd, int original_output_fd, int original_error_fd); diff --git a/src/basic/meson.build b/src/basic/meson.build index 3685b639ae..7cf2ff09ca 100644 --- a/src/basic/meson.build +++ b/src/basic/meson.build @@ -37,6 +37,8 @@ basic_sources = files(''' copy.h creds-util.c creds-util.h + data-fd-util.c + data-fd-util.h def.h dirent-util.c dirent-util.h diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index ed0632b02b..d769423d6e 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -21,7 +21,6 @@ #include #include "alloc-util.h" -#include "copy.h" #include "def.h" #include "env-util.h" #include "fd-util.h" diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 758dd8f1b8..de057a0245 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -11,6 +11,7 @@ #include "bus-common-errors.h" #include "bus-get-properties.h" #include "bus-log-control-api.h" +#include "data-fd-util.h" #include "dbus-cgroup.h" #include "dbus-execute.h" #include "dbus-job.h" diff --git a/src/core/execute.c b/src/core/execute.c index f9d23272c4..42d76a346d 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -46,6 +46,7 @@ #include "cgroup-setup.h" #include "chown-recursive.h" #include "cpu-set-util.h" +#include "data-fd-util.h" #include "def.h" #include "env-file.h" #include "env-util.h" diff --git a/src/home/homed-home.c b/src/home/homed-home.c index 39dd501a32..104427dabe 100644 --- a/src/home/homed-home.c +++ b/src/home/homed-home.c @@ -11,6 +11,7 @@ #include "blockdev-util.h" #include "btrfs-util.h" #include "bus-common-errors.h" +#include "data-fd-util.h" #include "env-util.h" #include "errno-list.h" #include "errno-util.h" diff --git a/src/oom/oomd-manager-bus.c b/src/oom/oomd-manager-bus.c index 4ea2a338fc..b41e366309 100644 --- a/src/oom/oomd-manager-bus.c +++ b/src/oom/oomd-manager-bus.c @@ -4,6 +4,7 @@ #include "bus-common-errors.h" #include "bus-polkit.h" +#include "data-fd-util.h" #include "fd-util.h" #include "oomd-manager-bus.h" #include "oomd-manager.h" diff --git a/src/portable/portable.c b/src/portable/portable.c index 9e33299ed5..a9ced3c865 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -6,6 +6,7 @@ #include "bus-error.h" #include "conf-files.h" #include "copy.h" +#include "data-fd-util.h" #include "def.h" #include "dirent-util.h" #include "discover-image.h" @@ -153,7 +154,7 @@ static int send_item( assert(name); assert(fd >= 0); - data_fd = fd_duplicate_data_fd(fd); + data_fd = copy_data_fd(fd); if (data_fd < 0) return data_fd; diff --git a/src/test/meson.build b/src/test/meson.build index 29f488f4d8..03f08673bb 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -159,6 +159,8 @@ tests += [ [['src/test/test-copy.c']], + [['src/test/test-data-fd-util.c']], + [['src/test/test-static-destruct.c']], [['src/test/test-sigbus.c']], diff --git a/src/test/test-data-fd-util.c b/src/test/test-data-fd-util.c new file mode 100644 index 0000000000..22a57b5155 --- /dev/null +++ b/src/test/test-data-fd-util.c @@ -0,0 +1,154 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include +#include +#include + +#include "data-fd-util.h" +#include "fd-util.h" +#include "memory-util.h" +#include "process-util.h" +#include "tests.h" +#include "random-util.h" + +static void test_acquire_data_fd_one(unsigned flags) { + char wbuffer[196*1024 - 7]; + char rbuffer[sizeof(wbuffer)]; + int fd; + + fd = acquire_data_fd("foo", 3, flags); + assert_se(fd >= 0); + + zero(rbuffer); + assert_se(read(fd, rbuffer, sizeof(rbuffer)) == 3); + assert_se(streq(rbuffer, "foo")); + + fd = safe_close(fd); + + fd = acquire_data_fd("", 0, flags); + assert_se(fd >= 0); + + zero(rbuffer); + assert_se(read(fd, rbuffer, sizeof(rbuffer)) == 0); + assert_se(streq(rbuffer, "")); + + fd = safe_close(fd); + + random_bytes(wbuffer, sizeof(wbuffer)); + + fd = acquire_data_fd(wbuffer, sizeof(wbuffer), flags); + assert_se(fd >= 0); + + zero(rbuffer); + assert_se(read(fd, rbuffer, sizeof(rbuffer)) == sizeof(rbuffer)); + assert_se(memcmp(rbuffer, wbuffer, sizeof(rbuffer)) == 0); + + fd = safe_close(fd); +} + +static void test_acquire_data_fd(void) { + test_acquire_data_fd_one(0); + test_acquire_data_fd_one(ACQUIRE_NO_DEV_NULL); + test_acquire_data_fd_one(ACQUIRE_NO_MEMFD); + test_acquire_data_fd_one(ACQUIRE_NO_DEV_NULL|ACQUIRE_NO_MEMFD); + test_acquire_data_fd_one(ACQUIRE_NO_PIPE); + test_acquire_data_fd_one(ACQUIRE_NO_DEV_NULL|ACQUIRE_NO_PIPE); + test_acquire_data_fd_one(ACQUIRE_NO_MEMFD|ACQUIRE_NO_PIPE); + test_acquire_data_fd_one(ACQUIRE_NO_DEV_NULL|ACQUIRE_NO_MEMFD|ACQUIRE_NO_PIPE); + test_acquire_data_fd_one(ACQUIRE_NO_DEV_NULL|ACQUIRE_NO_MEMFD|ACQUIRE_NO_PIPE|ACQUIRE_NO_TMPFILE); +} + +static void assert_equal_fd(int fd1, int fd2) { + for (;;) { + uint8_t a[4096], b[4096]; + ssize_t x, y; + + x = read(fd1, a, sizeof(a)); + assert_se(x >= 0); + + y = read(fd2, b, sizeof(b)); + assert_se(y >= 0); + + assert_se(x == y); + + if (x == 0) + break; + + assert_se(memcmp(a, b, x) == 0); + } +} + +static void test_copy_data_fd(void) { + _cleanup_close_ int fd1 = -1, fd2 = -1; + _cleanup_(close_pairp) int sfd[2] = { -1, -1 }; + _cleanup_(sigkill_waitp) pid_t pid = -1; + int r; + + fd1 = open("/etc/fstab", O_RDONLY|O_CLOEXEC); + if (fd1 >= 0) { + + fd2 = copy_data_fd(fd1); + assert_se(fd2 >= 0); + + assert_se(lseek(fd1, 0, SEEK_SET) == 0); + assert_equal_fd(fd1, fd2); + } + + fd1 = safe_close(fd1); + fd2 = safe_close(fd2); + + fd1 = acquire_data_fd("hallo", 6, 0); + assert_se(fd1 >= 0); + + fd2 = copy_data_fd(fd1); + assert_se(fd2 >= 0); + + safe_close(fd1); + fd1 = acquire_data_fd("hallo", 6, 0); + assert_se(fd1 >= 0); + + assert_equal_fd(fd1, fd2); + + fd1 = safe_close(fd1); + fd2 = safe_close(fd2); + + assert_se(socketpair(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0, sfd) >= 0); + + r = safe_fork("(sd-pipe)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); + assert_se(r >= 0); + + if (r == 0) { + /* child */ + + sfd[0] = safe_close(sfd[0]); + + for (uint64_t i = 0; i < 1536*1024 / sizeof(uint64_t); i++) + assert_se(write(sfd[1], &i, sizeof(i)) == sizeof(i)); + + sfd[1] = safe_close(sfd[1]); + + _exit(EXIT_SUCCESS); + } + + sfd[1] = safe_close(sfd[1]); + + fd2 = copy_data_fd(sfd[0]); + assert_se(fd2 >= 0); + + uint64_t j; + for (uint64_t i = 0; i < 1536*1024 / sizeof(uint64_t); i++) { + assert_se(read(fd2, &j, sizeof(j)) == sizeof(j)); + assert_se(i == j); + } + + assert_se(read(fd2, &j, sizeof(j)) == 0); +} + +int main(int argc, char *argv[]) { + test_setup_logging(LOG_DEBUG); + + test_acquire_data_fd(); + test_copy_data_fd(); + + return 0; +} diff --git a/src/test/test-fd-util.c b/src/test/test-fd-util.c index bece89aef2..1cd3bdfbbe 100644 --- a/src/test/test-fd-util.c +++ b/src/test/test-fd-util.c @@ -4,6 +4,7 @@ #include #include "alloc-util.h" +#include "data-fd-util.h" #include "fd-util.h" #include "fileio.h" #include "macro.h" @@ -95,54 +96,6 @@ static void test_open_serialization_fd(void) { assert_se(write(fd, "test\n", 5) == 5); } -static void test_acquire_data_fd_one(unsigned flags) { - char wbuffer[196*1024 - 7]; - char rbuffer[sizeof(wbuffer)]; - int fd; - - fd = acquire_data_fd("foo", 3, flags); - assert_se(fd >= 0); - - zero(rbuffer); - assert_se(read(fd, rbuffer, sizeof(rbuffer)) == 3); - assert_se(streq(rbuffer, "foo")); - - fd = safe_close(fd); - - fd = acquire_data_fd("", 0, flags); - assert_se(fd >= 0); - - zero(rbuffer); - assert_se(read(fd, rbuffer, sizeof(rbuffer)) == 0); - assert_se(streq(rbuffer, "")); - - fd = safe_close(fd); - - random_bytes(wbuffer, sizeof(wbuffer)); - - fd = acquire_data_fd(wbuffer, sizeof(wbuffer), flags); - assert_se(fd >= 0); - - zero(rbuffer); - assert_se(read(fd, rbuffer, sizeof(rbuffer)) == sizeof(rbuffer)); - assert_se(memcmp(rbuffer, wbuffer, sizeof(rbuffer)) == 0); - - fd = safe_close(fd); -} - -static void test_acquire_data_fd(void) { - - test_acquire_data_fd_one(0); - test_acquire_data_fd_one(ACQUIRE_NO_DEV_NULL); - test_acquire_data_fd_one(ACQUIRE_NO_MEMFD); - test_acquire_data_fd_one(ACQUIRE_NO_DEV_NULL|ACQUIRE_NO_MEMFD); - test_acquire_data_fd_one(ACQUIRE_NO_PIPE); - test_acquire_data_fd_one(ACQUIRE_NO_DEV_NULL|ACQUIRE_NO_PIPE); - test_acquire_data_fd_one(ACQUIRE_NO_MEMFD|ACQUIRE_NO_PIPE); - test_acquire_data_fd_one(ACQUIRE_NO_DEV_NULL|ACQUIRE_NO_MEMFD|ACQUIRE_NO_PIPE); - test_acquire_data_fd_one(ACQUIRE_NO_DEV_NULL|ACQUIRE_NO_MEMFD|ACQUIRE_NO_PIPE|ACQUIRE_NO_TMPFILE); -} - static void test_fd_move_above_stdio(void) { int original_stdin, new_fd; @@ -227,93 +180,6 @@ static void test_rearrange_stdio(void) { } } -static void assert_equal_fd(int fd1, int fd2) { - - for (;;) { - uint8_t a[4096], b[4096]; - ssize_t x, y; - - x = read(fd1, a, sizeof(a)); - assert_se(x >= 0); - - y = read(fd2, b, sizeof(b)); - assert_se(y >= 0); - - assert_se(x == y); - - if (x == 0) - break; - - assert_se(memcmp(a, b, x) == 0); - } -} - -static void test_fd_duplicate_data_fd(void) { - _cleanup_close_ int fd1 = -1, fd2 = -1; - _cleanup_(close_pairp) int sfd[2] = { -1, -1 }; - _cleanup_(sigkill_waitp) pid_t pid = -1; - uint64_t i, j; - int r; - - fd1 = open("/etc/fstab", O_RDONLY|O_CLOEXEC); - if (fd1 >= 0) { - - fd2 = fd_duplicate_data_fd(fd1); - assert_se(fd2 >= 0); - - assert_se(lseek(fd1, 0, SEEK_SET) == 0); - assert_equal_fd(fd1, fd2); - } - - fd1 = safe_close(fd1); - fd2 = safe_close(fd2); - - fd1 = acquire_data_fd("hallo", 6, 0); - assert_se(fd1 >= 0); - - fd2 = fd_duplicate_data_fd(fd1); - assert_se(fd2 >= 0); - - safe_close(fd1); - fd1 = acquire_data_fd("hallo", 6, 0); - assert_se(fd1 >= 0); - - assert_equal_fd(fd1, fd2); - - fd1 = safe_close(fd1); - fd2 = safe_close(fd2); - - assert_se(socketpair(AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0, sfd) >= 0); - - r = safe_fork("(sd-pipe)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); - assert_se(r >= 0); - - if (r == 0) { - /* child */ - - sfd[0] = safe_close(sfd[0]); - - for (i = 0; i < 1536*1024 / sizeof(uint64_t); i++) - assert_se(write(sfd[1], &i, sizeof(i)) == sizeof(i)); - - sfd[1] = safe_close(sfd[1]); - - _exit(EXIT_SUCCESS); - } - - sfd[1] = safe_close(sfd[1]); - - fd2 = fd_duplicate_data_fd(sfd[0]); - assert_se(fd2 >= 0); - - for (i = 0; i < 1536*1024 / sizeof(uint64_t); i++) { - assert_se(read(fd2, &j, sizeof(j)) == sizeof(j)); - assert_se(i == j); - } - - assert_se(read(fd2, &j, sizeof(j)) == 0); -} - static void test_read_nr_open(void) { log_info("nr-open: %i", read_nr_open()); } @@ -420,10 +286,8 @@ int main(int argc, char *argv[]) { test_close_nointr(); test_same_fd(); test_open_serialization_fd(); - test_acquire_data_fd(); test_fd_move_above_stdio(); test_rearrange_stdio(); - test_fd_duplicate_data_fd(); test_read_nr_open(); test_close_all_fds(); From 2d32453bc808a061a45bc5d345746a7c99b4b52f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 21 Jun 2021 23:28:46 +0200 Subject: [PATCH 07/15] basic,shared: move dlopen helpers to shared/ This was added in 88d775b734644f26fb490836769c2bc275498fde, with the apparent intent of using in shared/ and the rest of our code. It doesn't matter much for our code, since libdl is part of glibc anyway, but moving it removes one linkage from libsystemd. (libshared was already linking to libdl explicitly). --- src/basic/meson.build | 5 +---- src/{basic => shared}/dlfcn-util.c | 0 src/{basic => shared}/dlfcn-util.h | 0 src/shared/meson.build | 2 ++ 4 files changed, 3 insertions(+), 4 deletions(-) rename src/{basic => shared}/dlfcn-util.c (100%) rename src/{basic => shared}/dlfcn-util.h (100%) diff --git a/src/basic/meson.build b/src/basic/meson.build index 7cf2ff09ca..9512221368 100644 --- a/src/basic/meson.build +++ b/src/basic/meson.build @@ -42,8 +42,6 @@ basic_sources = files(''' def.h dirent-util.c dirent-util.h - dlfcn-util.c - dlfcn-util.h dns-def.h efivars.c efivars.h @@ -396,8 +394,7 @@ libbasic = static_library( libcap, libseccomp, libselinux, - libm, - libdl], + libm], c_args : ['-fvisibility=default'], install : false) diff --git a/src/basic/dlfcn-util.c b/src/shared/dlfcn-util.c similarity index 100% rename from src/basic/dlfcn-util.c rename to src/shared/dlfcn-util.c diff --git a/src/basic/dlfcn-util.h b/src/shared/dlfcn-util.h similarity index 100% rename from src/basic/dlfcn-util.h rename to src/shared/dlfcn-util.h diff --git a/src/shared/meson.build b/src/shared/meson.build index c478d46f31..9de167f4cf 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -87,6 +87,8 @@ shared_sources = files(''' discover-image.h dissect-image.c dissect-image.h + dlfcn-util.c + dlfcn-util.h dm-util.c dm-util.h dns-domain.c From b25a930f0e2ebe77bc8b0f0acfac8a3b27ef1f0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 21 Jun 2021 23:13:10 +0200 Subject: [PATCH 08/15] basic,shared: move a bunch of files to src/shared/ The goal is to move everything that requires selinux or smack away from src/basic/. This means that src/basic/label.[ch] must move, which implies btrfs-util.[ch], copy.[ch], and a bunch of other files which form a cluster of internal use. This is just moving text around, so there should be no functional difference. test-blockdev-util is new, because path_is_encrypted() is moved to blockdev-util.c, and so far we didn't have any tests for code there. --- src/basic/fs-util.c | 86 --------------------------- src/basic/fs-util.h | 2 - src/basic/meson.build | 18 ------ src/home/homework.c | 1 + src/libsystemd/sd-bus/bus-socket.c | 1 - src/{basic => shared}/blockdev-util.c | 85 ++++++++++++++++++++++++++ src/{basic => shared}/blockdev-util.h | 2 + src/{basic => shared}/btrfs-util.c | 0 src/{basic => shared}/btrfs-util.h | 0 src/shared/condition.c | 1 + src/{basic => shared}/copy.c | 0 src/{basic => shared}/copy.h | 0 src/{basic => shared}/data-fd-util.c | 0 src/{basic => shared}/data-fd-util.h | 0 src/{basic => shared}/label.c | 0 src/{basic => shared}/label.h | 0 src/shared/meson.build | 18 ++++++ src/{basic => shared}/mkdir-label.c | 0 src/{basic => shared}/rm-rf.c | 0 src/{basic => shared}/rm-rf.h | 0 src/{basic => shared}/selinux-util.c | 0 src/{basic => shared}/selinux-util.h | 0 src/{basic => shared}/smack-util.c | 0 src/{basic => shared}/smack-util.h | 0 src/{basic => shared}/socket-label.c | 0 src/test/meson.build | 2 + src/test/test-blockdev-util.c | 43 ++++++++++++++ src/test/test-fs-util.c | 33 ---------- 28 files changed, 152 insertions(+), 140 deletions(-) rename src/{basic => shared}/blockdev-util.c (76%) rename src/{basic => shared}/blockdev-util.h (95%) rename src/{basic => shared}/btrfs-util.c (100%) rename src/{basic => shared}/btrfs-util.h (100%) rename src/{basic => shared}/copy.c (100%) rename src/{basic => shared}/copy.h (100%) rename src/{basic => shared}/data-fd-util.c (100%) rename src/{basic => shared}/data-fd-util.h (100%) rename src/{basic => shared}/label.c (100%) rename src/{basic => shared}/label.h (100%) rename src/{basic => shared}/mkdir-label.c (100%) rename src/{basic => shared}/rm-rf.c (100%) rename src/{basic => shared}/rm-rf.h (100%) rename src/{basic => shared}/selinux-util.c (100%) rename src/{basic => shared}/selinux-util.h (100%) rename src/{basic => shared}/smack-util.c (100%) rename src/{basic => shared}/smack-util.h (100%) rename src/{basic => shared}/socket-label.c (100%) create mode 100644 src/test/test-blockdev-util.c diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 5fe8fbab98..bcec603f88 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -8,7 +8,6 @@ #include #include "alloc-util.h" -#include "blockdev-util.h" #include "dirent-util.h" #include "fd-util.h" #include "fileio.h" @@ -1504,91 +1503,6 @@ int open_parent(const char *path, int flags, mode_t mode) { return fd; } -static int blockdev_is_encrypted(const char *sysfs_path, unsigned depth_left) { - _cleanup_free_ char *p = NULL, *uuids = NULL; - _cleanup_closedir_ DIR *d = NULL; - int r, found_encrypted = false; - - assert(sysfs_path); - - if (depth_left == 0) - return -EINVAL; - - p = path_join(sysfs_path, "dm/uuid"); - if (!p) - return -ENOMEM; - - r = read_one_line_file(p, &uuids); - if (r != -ENOENT) { - if (r < 0) - return r; - - /* The DM device's uuid attribute is prefixed with "CRYPT-" if this is a dm-crypt device. */ - if (startswith(uuids, "CRYPT-")) - return true; - } - - /* Not a dm-crypt device itself. But maybe it is on top of one? Follow the links in the "slaves/" - * subdir. */ - - p = mfree(p); - p = path_join(sysfs_path, "slaves"); - if (!p) - return -ENOMEM; - - d = opendir(p); - if (!d) { - if (errno == ENOENT) /* Doesn't have underlying devices */ - return false; - - return -errno; - } - - for (;;) { - _cleanup_free_ char *q = NULL; - struct dirent *de; - - errno = 0; - de = readdir_no_dot(d); - if (!de) { - if (errno != 0) - return -errno; - - break; /* No more underlying devices */ - } - - q = path_join(p, de->d_name); - if (!q) - return -ENOMEM; - - r = blockdev_is_encrypted(q, depth_left - 1); - if (r < 0) - return r; - if (r == 0) /* we found one that is not encrypted? then propagate that immediately */ - return false; - - found_encrypted = true; - } - - return found_encrypted; -} - -int path_is_encrypted(const char *path) { - char p[SYS_BLOCK_PATH_MAX(NULL)]; - dev_t devt; - int r; - - r = get_block_device(path, &devt); - if (r < 0) - return r; - if (r == 0) /* doesn't have a block device */ - return false; - - xsprintf_sys_block_path(p, NULL, devt); - - return blockdev_is_encrypted(p, 10 /* safety net: maximum recursion depth */); -} - int conservative_renameat( int olddirfd, const char *oldpath, int newdirfd, const char *newpath) { diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h index 85bdea64df..7f15b558ca 100644 --- a/src/basic/fs-util.h +++ b/src/basic/fs-util.h @@ -145,8 +145,6 @@ int syncfs_path(int atfd, const char *path); int open_parent(const char *path, int flags, mode_t mode); -int path_is_encrypted(const char *path); - int conservative_renameat(int olddirfd, const char *oldpath, int newdirfd, const char *newpath); static inline int conservative_rename(const char *oldpath, const char *newpath) { return conservative_renameat(AT_FDCWD, oldpath, AT_FDCWD, newpath); diff --git a/src/basic/meson.build b/src/basic/meson.build index 9512221368..f7beafa022 100644 --- a/src/basic/meson.build +++ b/src/basic/meson.build @@ -15,10 +15,6 @@ basic_sources = files(''' async.h audit-util.c audit-util.h - blockdev-util.c - blockdev-util.h - btrfs-util.c - btrfs-util.h build.c build.h bus-label.c @@ -33,12 +29,8 @@ basic_sources = files(''' chattr-util.h conf-files.c conf-files.h - copy.c - copy.h creds-util.c creds-util.h - data-fd-util.c - data-fd-util.h def.h dirent-util.c dirent-util.h @@ -85,8 +77,6 @@ basic_sources = files(''' ioprio.h khash.c khash.h - label.c - label.h limits-util.c limits-util.h linux/btrfs.h @@ -157,7 +147,6 @@ basic_sources = files(''' missing_syscall.h missing_timerfd.h missing_type.h - mkdir-label.c mkdir.c mkdir.h mountpoint-util.c @@ -200,10 +189,6 @@ basic_sources = files(''' replace-var.h rlimit-util.c rlimit-util.h - rm-rf.c - rm-rf.h - selinux-util.c - selinux-util.h set.h sigbus.c sigbus.h @@ -211,9 +196,6 @@ basic_sources = files(''' signal-util.h siphash24.c siphash24.h - smack-util.c - smack-util.h - socket-label.c socket-util.c socket-util.h sort-util.c diff --git a/src/home/homework.c b/src/home/homework.c index 073d12e50e..bdd9ac649e 100644 --- a/src/home/homework.c +++ b/src/home/homework.c @@ -3,6 +3,7 @@ #include #include +#include "blockdev-util.h" #include "chown-recursive.h" #include "copy.h" #include "fd-util.h" diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c index 16e61e1e89..378774fe8b 100644 --- a/src/libsystemd/sd-bus/bus-socket.c +++ b/src/libsystemd/sd-bus/bus-socket.c @@ -22,7 +22,6 @@ #include "path-util.h" #include "process-util.h" #include "rlimit-util.h" -#include "selinux-util.h" #include "signal-util.h" #include "stdio-util.h" #include "string-util.h" diff --git a/src/basic/blockdev-util.c b/src/shared/blockdev-util.c similarity index 76% rename from src/basic/blockdev-util.c rename to src/shared/blockdev-util.c index 676ad9351b..4d545dfa58 100644 --- a/src/basic/blockdev-util.c +++ b/src/shared/blockdev-util.c @@ -256,3 +256,88 @@ int blockdev_partscan_enabled(int fd) { return !FLAGS_SET(ull, GENHD_FL_NO_PART_SCAN); } + +static int blockdev_is_encrypted(const char *sysfs_path, unsigned depth_left) { + _cleanup_free_ char *p = NULL, *uuids = NULL; + _cleanup_closedir_ DIR *d = NULL; + int r, found_encrypted = false; + + assert(sysfs_path); + + if (depth_left == 0) + return -EINVAL; + + p = path_join(sysfs_path, "dm/uuid"); + if (!p) + return -ENOMEM; + + r = read_one_line_file(p, &uuids); + if (r != -ENOENT) { + if (r < 0) + return r; + + /* The DM device's uuid attribute is prefixed with "CRYPT-" if this is a dm-crypt device. */ + if (startswith(uuids, "CRYPT-")) + return true; + } + + /* Not a dm-crypt device itself. But maybe it is on top of one? Follow the links in the "slaves/" + * subdir. */ + + p = mfree(p); + p = path_join(sysfs_path, "slaves"); + if (!p) + return -ENOMEM; + + d = opendir(p); + if (!d) { + if (errno == ENOENT) /* Doesn't have underlying devices */ + return false; + + return -errno; + } + + for (;;) { + _cleanup_free_ char *q = NULL; + struct dirent *de; + + errno = 0; + de = readdir_no_dot(d); + if (!de) { + if (errno != 0) + return -errno; + + break; /* No more underlying devices */ + } + + q = path_join(p, de->d_name); + if (!q) + return -ENOMEM; + + r = blockdev_is_encrypted(q, depth_left - 1); + if (r < 0) + return r; + if (r == 0) /* we found one that is not encrypted? then propagate that immediately */ + return false; + + found_encrypted = true; + } + + return found_encrypted; +} + +int path_is_encrypted(const char *path) { + char p[SYS_BLOCK_PATH_MAX(NULL)]; + dev_t devt; + int r; + + r = get_block_device(path, &devt); + if (r < 0) + return r; + if (r == 0) /* doesn't have a block device */ + return false; + + xsprintf_sys_block_path(p, NULL, devt); + + return blockdev_is_encrypted(p, 10 /* safety net: maximum recursion depth */); +} diff --git a/src/basic/blockdev-util.h b/src/shared/blockdev-util.h similarity index 95% rename from src/basic/blockdev-util.h rename to src/shared/blockdev-util.h index 10048ff313..56c50cecba 100644 --- a/src/basic/blockdev-util.h +++ b/src/shared/blockdev-util.h @@ -22,3 +22,5 @@ int get_block_device_harder(const char *path, dev_t *dev); int lock_whole_block_device(dev_t devt, int operation); int blockdev_partscan_enabled(int fd); + +int path_is_encrypted(const char *path); diff --git a/src/basic/btrfs-util.c b/src/shared/btrfs-util.c similarity index 100% rename from src/basic/btrfs-util.c rename to src/shared/btrfs-util.c diff --git a/src/basic/btrfs-util.h b/src/shared/btrfs-util.h similarity index 100% rename from src/basic/btrfs-util.h rename to src/shared/btrfs-util.h diff --git a/src/shared/condition.c b/src/shared/condition.c index b86312548d..55fb636673 100644 --- a/src/shared/condition.c +++ b/src/shared/condition.c @@ -17,6 +17,7 @@ #include "apparmor-util.h" #include "architecture.h" #include "audit-util.h" +#include "blockdev-util.h" #include "cap-list.h" #include "cgroup-util.h" #include "condition.h" diff --git a/src/basic/copy.c b/src/shared/copy.c similarity index 100% rename from src/basic/copy.c rename to src/shared/copy.c diff --git a/src/basic/copy.h b/src/shared/copy.h similarity index 100% rename from src/basic/copy.h rename to src/shared/copy.h diff --git a/src/basic/data-fd-util.c b/src/shared/data-fd-util.c similarity index 100% rename from src/basic/data-fd-util.c rename to src/shared/data-fd-util.c diff --git a/src/basic/data-fd-util.h b/src/shared/data-fd-util.h similarity index 100% rename from src/basic/data-fd-util.h rename to src/shared/data-fd-util.h diff --git a/src/basic/label.c b/src/shared/label.c similarity index 100% rename from src/basic/label.c rename to src/shared/label.c diff --git a/src/basic/label.h b/src/shared/label.h similarity index 100% rename from src/basic/label.h rename to src/shared/label.h diff --git a/src/shared/meson.build b/src/shared/meson.build index 9de167f4cf..5008cda500 100644 --- a/src/shared/meson.build +++ b/src/shared/meson.build @@ -17,6 +17,8 @@ shared_sources = files(''' bitmap.c bitmap.h blkid-util.h + blockdev-util.c + blockdev-util.h bond-util.c bond-util.h boot-timestamps.c @@ -29,6 +31,8 @@ shared_sources = files(''' bpf-program.h bridge-util.c bridge-util.h + btrfs-util.c + btrfs-util.h bus-get-properties.c bus-get-properties.h bus-locator.c @@ -71,6 +75,8 @@ shared_sources = files(''' condition.h conf-parser.c conf-parser.h + copy.c + copy.h coredump-util.c coredump-util.h cpu-set-util.c @@ -78,6 +84,8 @@ shared_sources = files(''' cryptsetup-util.c cryptsetup-util.h daemon-util.h + data-fd-util.c + data-fd-util.h dev-setup.c dev-setup.h device-nodes.c @@ -161,6 +169,8 @@ shared_sources = files(''' kbd-util.h killall.c killall.h + label.c + label.h libcrypt-util.c libcrypt-util.h libfido2-util.c @@ -190,6 +200,7 @@ shared_sources = files(''' macvlan-util.c macvlan-util.h main-func.h + mkdir-label.c mkfs-util.c mkfs-util.h module-util.h @@ -235,15 +246,22 @@ shared_sources = files(''' resize-fs.h resolve-util.c resolve-util.h + rm-rf.c + rm-rf.h seccomp-util.h securebits-util.c securebits-util.h + selinux-util.c + selinux-util.h serialize.c serialize.h service-util.c service-util.h sleep-config.c sleep-config.h + smack-util.c + smack-util.h + socket-label.c socket-netlink.c socket-netlink.h spawn-ask-password-agent.c diff --git a/src/basic/mkdir-label.c b/src/shared/mkdir-label.c similarity index 100% rename from src/basic/mkdir-label.c rename to src/shared/mkdir-label.c diff --git a/src/basic/rm-rf.c b/src/shared/rm-rf.c similarity index 100% rename from src/basic/rm-rf.c rename to src/shared/rm-rf.c diff --git a/src/basic/rm-rf.h b/src/shared/rm-rf.h similarity index 100% rename from src/basic/rm-rf.h rename to src/shared/rm-rf.h diff --git a/src/basic/selinux-util.c b/src/shared/selinux-util.c similarity index 100% rename from src/basic/selinux-util.c rename to src/shared/selinux-util.c diff --git a/src/basic/selinux-util.h b/src/shared/selinux-util.h similarity index 100% rename from src/basic/selinux-util.h rename to src/shared/selinux-util.h diff --git a/src/basic/smack-util.c b/src/shared/smack-util.c similarity index 100% rename from src/basic/smack-util.c rename to src/shared/smack-util.c diff --git a/src/basic/smack-util.h b/src/shared/smack-util.h similarity index 100% rename from src/basic/smack-util.h rename to src/shared/smack-util.h diff --git a/src/basic/socket-label.c b/src/shared/socket-label.c similarity index 100% rename from src/basic/socket-label.c rename to src/shared/socket-label.c diff --git a/src/test/meson.build b/src/test/meson.build index 03f08673bb..cf49990b83 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -146,6 +146,8 @@ tests += [ [['src/test/test-utf8.c']], + [['src/test/test-blockdev-util.c']], + [['src/test/test-dev-setup.c']], [['src/test/test-capability.c'], diff --git a/src/test/test-blockdev-util.c b/src/test/test-blockdev-util.c new file mode 100644 index 0000000000..ab5169c43a --- /dev/null +++ b/src/test/test-blockdev-util.c @@ -0,0 +1,43 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include "blockdev-util.h" +#include "errno-util.h" +#include "tests.h" + +static void test_path_is_encrypted_one(const char *p, int expect) { + int r; + + r = path_is_encrypted(p); + if (r == -ENOENT || ERRNO_IS_PRIVILEGE(r)) /* This might fail, if btrfs is used and we run in a + * container. In that case we cannot resolve the device node paths that + * BTRFS_IOC_DEV_INFO returns, because the device nodes are unlikely to exist in + * the container. But if we can't stat() them we cannot determine the dev_t of + * them, and thus cannot figure out if they are enrypted. Hence let's just ignore + * ENOENT here. Also skip the test if we lack privileges. */ + return; + assert_se(r >= 0); + + log_info("%s encrypted: %s", p, yes_no(r)); + + assert_se(expect < 0 || ((r > 0) == (expect > 0))); +} + +static void test_path_is_encrypted(void) { + int booted = sd_booted(); /* If this is run in build environments such as koji, /dev might be a + * reguar fs. Don't assume too much if not running under systemd. */ + + log_info("/* %s (sd_booted=%d) */", __func__, booted); + + test_path_is_encrypted_one("/home", -1); + test_path_is_encrypted_one("/var", -1); + test_path_is_encrypted_one("/", -1); + test_path_is_encrypted_one("/proc", false); + test_path_is_encrypted_one("/sys", false); + test_path_is_encrypted_one("/dev", booted > 0 ? false : -1); +} + +int main(int argc, char **argv) { + test_setup_logging(LOG_INFO); + + test_path_is_encrypted(); +} diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index a0f91c35d1..08bebcf0e8 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -805,38 +805,6 @@ static void test_chmod_and_chown(void) { assert_se(S_ISLNK(st.st_mode)); } -static void test_path_is_encrypted_one(const char *p, int expect) { - int r; - - r = path_is_encrypted(p); - if (r == -ENOENT || ERRNO_IS_PRIVILEGE(r)) /* This might fail, if btrfs is used and we run in a - * container. In that case we cannot resolve the device node paths that - * BTRFS_IOC_DEV_INFO returns, because the device nodes are unlikely to exist in - * the container. But if we can't stat() them we cannot determine the dev_t of - * them, and thus cannot figure out if they are enrypted. Hence let's just ignore - * ENOENT here. Also skip the test if we lack privileges. */ - return; - assert_se(r >= 0); - - log_info("%s encrypted: %s", p, yes_no(r)); - - assert_se(expect < 0 || ((r > 0) == (expect > 0))); -} - -static void test_path_is_encrypted(void) { - int booted = sd_booted(); /* If this is run in build environments such as koji, /dev might be a - * reguar fs. Don't assume too much if not running under systemd. */ - - log_info("/* %s (sd_booted=%d) */", __func__, booted); - - test_path_is_encrypted_one("/home", -1); - test_path_is_encrypted_one("/var", -1); - test_path_is_encrypted_one("/", -1); - test_path_is_encrypted_one("/proc", false); - test_path_is_encrypted_one("/sys", false); - test_path_is_encrypted_one("/dev", booted > 0 ? false : -1); -} - static void create_binary_file(const char *p, const void *data, size_t l) { _cleanup_close_ int fd = -1; @@ -914,7 +882,6 @@ int main(int argc, char *argv[]) { test_fsync_directory_of_file(); test_rename_noreplace(); test_chmod_and_chown(); - test_path_is_encrypted(); test_conservative_rename(); return 0; From 87501ac0ebacf4df3a7eb1a066f5e359de2da4ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 21 Jun 2021 23:31:52 +0200 Subject: [PATCH 09/15] meson: drop libseccomp and libselinux from libbasic linkage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This means libsystemd.so is without them now. This is important because countless programs link to libsystemd.so, and do not need to pull in selinux now. And libselinux.so pulls in libpcre2, so we trim a nice dependency tree. I'm not sure why libseccomp was listed there. No code seems to refer to it. $ diff -u <(ldd ../systemd/build/libsystemd.so|sed 's/0x.*/0x…/') <(ldd build/libsystemd.so|sed 's/0x.*/0x…/') @@ -4,11 +4,9 @@ libzstd.so.1 => /lib64/libzstd.so.1 (0x… liblz4.so.1 => /lib64/liblz4.so.1 (0x… libcap.so.2 => /lib64/libcap.so.2 (0x… - libselinux.so.1 => /lib64/libselinux.so.1 (0x… libgcrypt.so.20 => /lib64/libgcrypt.so.20 (0x… libpthread.so.0 => /lib64/libpthread.so.0 (0x… libc.so.6 => /lib64/libc.so.6 (0x… /lib64/ld-linux-x86-64.so.2 (0x… - libpcre2-8.so.0 => /lib64/libpcre2-8.so.0 (0x… libdl.so.2 => /lib64/libdl.so.2 (0x… libgpg-error.so.0 => /lib64/libgpg-error.so.0 (0x… $ diff -u <(ldd ../systemd/build/libudev.so|sed 's/0x.*/0x…/') <(ldd build/libudev.so|sed 's/0x.*/0x…/') @@ -1,8 +1,5 @@ linux-vdso.so.1 (0x… librt.so.1 => /lib64/librt.so.1 (0x… - libselinux.so.1 => /lib64/libselinux.so.1 (0x… libpthread.so.0 => /lib64/libpthread.so.0 (0x… libc.so.6 => /lib64/libc.so.6 (0x… /lib64/ld-linux-x86-64.so.2 (0x… - libpcre2-8.so.0 => /lib64/libpcre2-8.so.0 (0x… - libdl.so.2 => /lib64/libdl.so.2 (0x… --- meson.build | 1 - src/basic/meson.build | 2 -- 2 files changed, 3 deletions(-) diff --git a/meson.build b/meson.build index 3634ce0a3c..32e5413a62 100644 --- a/meson.build +++ b/meson.build @@ -1707,7 +1707,6 @@ install_libsystemd_static = static_library( libcap, libblkid, libmount, - libselinux, libgcrypt], c_args : libsystemd_c_args + (static_libsystemd_pic ? [] : ['-fno-PIC'])) diff --git a/src/basic/meson.build b/src/basic/meson.build index f7beafa022..9b016ce5e8 100644 --- a/src/basic/meson.build +++ b/src/basic/meson.build @@ -374,8 +374,6 @@ libbasic = static_library( dependencies : [versiondep, threads, libcap, - libseccomp, - libselinux, libm], c_args : ['-fvisibility=default'], install : false) From fff25ab22eec825a15c2647a84247221abd6b0c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 22 Jun 2021 08:48:41 +0200 Subject: [PATCH 10/15] dlfcn-util: invert function naming and add helper that does the whole job We warn when the operation fails, not when it succeeds. Hence this should be "_or_", not "_and_". We *could* use whatever convention we want, but rust and perl are rather consistent in using the logical convention. We don't care about perl that much, but having a naming convention inverted wrt. rust would be rather confusing. Also, pretty much every implementation does similar steps, so add a nice wrapper which combines opening of the library and loading of the symbols. Also add missing sentinel attribute in dlopen_or_warn(). --- src/journal/pcre2-dlopen.c | 2 +- src/shared/bpf-dlopen.c | 2 +- src/shared/cryptsetup-util.c | 2 +- src/shared/dlfcn-util.c | 70 ++++++++++++++++++++++++------------ src/shared/dlfcn-util.h | 8 +++-- src/shared/idn-util.c | 4 +-- src/shared/libfido2-util.c | 2 +- src/shared/pwquality-util.c | 2 +- src/shared/qrcode-util.c | 2 +- src/shared/tpm2-util.c | 6 ++-- 10 files changed, 64 insertions(+), 36 deletions(-) diff --git a/src/journal/pcre2-dlopen.c b/src/journal/pcre2-dlopen.c index 5f78f76672..210e39a0ae 100644 --- a/src/journal/pcre2-dlopen.c +++ b/src/journal/pcre2-dlopen.c @@ -35,7 +35,7 @@ int dlopen_pcre2(void) { * string actually contains the "_8" suffix already due to that and we don't have to append it * manually anymore. C is weird. 🤯 */ - r = dlsym_many_and_warn( + r = dlsym_many_or_warn( dl, LOG_ERR, DLSYM_ARG(pcre2_match_data_create), diff --git a/src/shared/bpf-dlopen.c b/src/shared/bpf-dlopen.c index 0556148458..0d222ae19e 100644 --- a/src/shared/bpf-dlopen.c +++ b/src/shared/bpf-dlopen.c @@ -35,7 +35,7 @@ int dlopen_bpf(void) { return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "libbpf is not installed: %s", dlerror()); - r = dlsym_many_and_warn( + r = dlsym_many_or_warn( dl, LOG_DEBUG, DLSYM_ARG(bpf_link__destroy), diff --git a/src/shared/cryptsetup-util.c b/src/shared/cryptsetup-util.c index 9deae39dea..99c78f6858 100644 --- a/src/shared/cryptsetup-util.c +++ b/src/shared/cryptsetup-util.c @@ -62,7 +62,7 @@ int dlopen_cryptsetup(void) { return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "libcryptsetup support is not installed: %s", dlerror()); - r = dlsym_many_and_warn( + r = dlsym_many_or_warn( dl, LOG_DEBUG, DLSYM_ARG(crypt_activate_by_passphrase), diff --git a/src/shared/dlfcn-util.c b/src/shared/dlfcn-util.c index 2dbff0e358..c027e3396b 100644 --- a/src/shared/dlfcn-util.c +++ b/src/shared/dlfcn-util.c @@ -2,39 +2,63 @@ #include "dlfcn-util.h" -int dlsym_many_and_warn(void *dl, int level, ...) { - va_list ap; - int r; +static int dlsym_many_or_warnv(void *dl, int log_level, va_list ap) { + void (**fn)(void); - /* Tries to resolve a bunch of function symbols, and logs errors about the ones it cannot - * resolve. Note that this function possibly modifies the supplied function pointers if the whole - * operation fails */ + /* Tries to resolve a bunch of function symbols, and logs an error about if it cannot resolve one of + * them. Note that this function possibly modifies the supplied function pointers if the whole + * operation fails. */ - va_start(ap, level); - - for (;;) { - void (**fn)(void); + while ((fn = va_arg(ap, typeof(fn)))) { void (*tfn)(void); const char *symbol; - fn = va_arg(ap, typeof(fn)); - if (!fn) - break; - symbol = va_arg(ap, typeof(symbol)); tfn = (typeof(tfn)) dlsym(dl, symbol); - if (!tfn) { - r = log_full_errno(level, - SYNTHETIC_ERRNO(ELIBBAD), - "Can't find symbol %s: %s", symbol, dlerror()); - va_end(ap); - return r; - } - + if (!tfn) + return log_full_errno(log_level, + SYNTHETIC_ERRNO(ELIBBAD), + "Can't find symbol %s: %s", symbol, dlerror()); *fn = tfn; } - va_end(ap); return 0; } + +int dlsym_many_or_warn(void *dl, int log_level, ...) { + va_list ap; + int r; + + va_start(ap, log_level); + r = dlsym_many_or_warnv(dl, log_level, ap); + va_end(ap); + + return r; +} + +int dlopen_many_sym_or_warn_sentinel(void **dlp, const char *filename, int log_level, ...) { + _cleanup_(dlclosep) void *dl = NULL; + int r; + + if (*dlp) + return 0; /* Already loaded */ + + dl = dlopen(filename, RTLD_LAZY); + if (!dl) + return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), + "%s is not installed: %s", filename, dlerror()); + + va_list ap; + va_start(ap, log_level); + r = dlsym_many_or_warnv(dl, log_level, ap); + va_end(ap); + + if (r < 0) + return r; + + /* Note that we never release the reference here, because there's no real reason to. After all this + * was traditionally a regular shared library dependency which lives forever too. */ + *dlp = TAKE_PTR(dl); + return 1; +} diff --git a/src/shared/dlfcn-util.h b/src/shared/dlfcn-util.h index aa713d328b..87585b406a 100644 --- a/src/shared/dlfcn-util.h +++ b/src/shared/dlfcn-util.h @@ -7,9 +7,13 @@ DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(void*, dlclose, NULL); -int dlsym_many_and_warn(void *dl, int level, ...); +int dlsym_many_or_warn(void *dl, int log_level, ...) _sentinel_; +int dlopen_many_sym_or_warn_sentinel(void **dlp, const char *filename, int log_level, ...) _sentinel_; -/* Macro useful for putting together variable/symbol name pairs when calling dlsym_many_and_warn(). Assumes +#define dlopen_many_sym_or_warn(dlp, filename, log_level, ...) \ + dlopen_many_sym_or_warn_sentinel(dlp, filename, log_level, __VA_ARGS__, NULL) + +/* Macro useful for putting together variable/symbol name pairs when calling dlsym_many_or_warn(). Assumes * that each library symbol to resolve will be placed in a variable with the "sym_" prefix, i.e. a symbol * "foobar" is loaded into a variable "sym_foobar". */ #define DLSYM_ARG(arg) \ diff --git a/src/shared/idn-util.c b/src/shared/idn-util.c index 1c4472dc66..63a9ebf99d 100644 --- a/src/shared/idn-util.c +++ b/src/shared/idn-util.c @@ -32,7 +32,7 @@ int dlopen_idn(void) { return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "libidn2 support is not installed: %s", dlerror()); - r = dlsym_many_and_warn( + r = dlsym_many_or_warn( dl, LOG_DEBUG, DLSYM_ARG(idn2_lookup_u8), @@ -73,7 +73,7 @@ int dlopen_idn(void) { "libidn support is not installed: %s", dlerror()); } - r = dlsym_many_and_warn( + r = dlsym_many_or_warn( dl, LOG_DEBUG, DLSYM_ARG(idna_to_ascii_4i), diff --git a/src/shared/libfido2-util.c b/src/shared/libfido2-util.c index ec09937d83..c69943262d 100644 --- a/src/shared/libfido2-util.c +++ b/src/shared/libfido2-util.c @@ -72,7 +72,7 @@ int dlopen_libfido2(void) { return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "libfido2 support is not installed: %s", dlerror()); - r = dlsym_many_and_warn( + r = dlsym_many_or_warn( dl, LOG_DEBUG, DLSYM_ARG(fido_assert_allow_cred), diff --git a/src/shared/pwquality-util.c b/src/shared/pwquality-util.c index 5bd33eee4c..d7776e7eff 100644 --- a/src/shared/pwquality-util.c +++ b/src/shared/pwquality-util.c @@ -35,7 +35,7 @@ int dlopen_pwquality(void) { return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "libpwquality support is not installed: %s", dlerror()); - r = dlsym_many_and_warn( + r = dlsym_many_or_warn( dl, LOG_DEBUG, DLSYM_ARG(pwquality_check), diff --git a/src/shared/qrcode-util.c b/src/shared/qrcode-util.c index 79ac640672..5345b7288b 100644 --- a/src/shared/qrcode-util.c +++ b/src/shared/qrcode-util.c @@ -29,7 +29,7 @@ int dlopen_qrencode(void) { return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "libqrcode support is not installed: %s", dlerror()); - r = dlsym_many_and_warn( + r = dlsym_many_or_warn( dl, LOG_DEBUG, DLSYM_ARG(QRcode_encodeString), diff --git a/src/shared/tpm2-util.c b/src/shared/tpm2-util.c index b2540c62fe..da78e1f406 100644 --- a/src/shared/tpm2-util.c +++ b/src/shared/tpm2-util.c @@ -52,7 +52,7 @@ int dlopen_tpm2(void) { return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "TPM2 support is not installed: %s", dlerror()); - r = dlsym_many_and_warn( + r = dlsym_many_or_warn( dl, LOG_DEBUG, DLSYM_ARG(Esys_Create), @@ -84,7 +84,7 @@ int dlopen_tpm2(void) { return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "TPM2 support is not installed: %s", dlerror()); - r = dlsym_many_and_warn( + r = dlsym_many_or_warn( dl, LOG_DEBUG, DLSYM_ARG(Tss2_RC_Decode), @@ -104,7 +104,7 @@ int dlopen_tpm2(void) { return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "TPM2 support is not installed: %s", dlerror()); - r = dlsym_many_and_warn( + r = dlsym_many_or_warn( dl, LOG_DEBUG, DLSYM_ARG(Tss2_MU_TPM2B_PRIVATE_Marshal), From 1622ef77ee4466074e7d785feabf6bc9115297c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 22 Jun 2021 09:24:02 +0200 Subject: [PATCH 11/15] various: convert to the new dlopen_or_warn() helper --- src/journal/pcre2-dlopen.c | 29 ++++------------------------- src/shared/bpf-dlopen.c | 28 ++++------------------------ src/shared/cryptsetup-util.c | 23 ++++------------------- src/shared/idn-util.c | 27 +++------------------------ src/shared/libfido2-util.c | 26 +++----------------------- src/shared/pwquality-util.c | 26 +++----------------------- src/shared/qrcode-util.c | 28 ++++------------------------ 7 files changed, 25 insertions(+), 162 deletions(-) diff --git a/src/journal/pcre2-dlopen.c b/src/journal/pcre2-dlopen.c index 210e39a0ae..475d7eb26d 100644 --- a/src/journal/pcre2-dlopen.c +++ b/src/journal/pcre2-dlopen.c @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ -#include "alloc-util.h" #include "dlfcn-util.h" +#include "log.h" #include "pcre2-dlopen.h" #if HAVE_PCRE2 @@ -16,17 +16,6 @@ int (*sym_pcre2_match)(const pcre2_code *, PCRE2_SPTR, PCRE2_SIZE, PCRE2_SIZE, u PCRE2_SIZE* (*sym_pcre2_get_ovector_pointer)(pcre2_match_data *); int dlopen_pcre2(void) { - _cleanup_(dlclosep) void *dl = NULL; - int r; - - if (pcre2_dl) - return 0; /* Already loaded */ - - dl = dlopen("libpcre2-8.so.0", RTLD_LAZY); - if (!dl) - return log_error_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), - "PCRE2 support is not installed: %s", dlerror()); - /* So here's something weird: PCRE2 actually renames the symbols exported by the library via C * macros, so that the exported symbols carry a suffix "_8" but when used from C the suffix is * gone. In the argument list below we ignore this mangling. Surprisingly (at least to me), we @@ -35,25 +24,15 @@ int dlopen_pcre2(void) { * string actually contains the "_8" suffix already due to that and we don't have to append it * manually anymore. C is weird. 🤯 */ - r = dlsym_many_or_warn( - dl, - LOG_ERR, + return dlopen_many_sym_or_warn( + &pcre2_dl, "libpcre2-8.so.0", LOG_ERR, DLSYM_ARG(pcre2_match_data_create), DLSYM_ARG(pcre2_match_data_free), DLSYM_ARG(pcre2_code_free), DLSYM_ARG(pcre2_compile), DLSYM_ARG(pcre2_get_error_message), DLSYM_ARG(pcre2_match), - DLSYM_ARG(pcre2_get_ovector_pointer), - NULL); - if (r < 0) - return r; - - /* Note that we never release the reference here, because there's no real reason to, after all this - * was traditionally a regular shared library dependency which lives forever too. */ - pcre2_dl = TAKE_PTR(dl); - - return 1; + DLSYM_ARG(pcre2_get_ovector_pointer)); } #else diff --git a/src/shared/bpf-dlopen.c b/src/shared/bpf-dlopen.c index 0d222ae19e..45ab986ab2 100644 --- a/src/shared/bpf-dlopen.c +++ b/src/shared/bpf-dlopen.c @@ -1,8 +1,8 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ -#include "alloc-util.h" #include "dlfcn-util.h" #include "bpf-dlopen.h" +#include "log.h" #if HAVE_LIBBPF static void *bpf_dl = NULL; @@ -24,20 +24,8 @@ bool (*sym_bpf_probe_prog_type)(enum bpf_prog_type, __u32); const char* (*sym_bpf_program__name)(const struct bpf_program *); int dlopen_bpf(void) { - _cleanup_(dlclosep) void *dl = NULL; - int r; - - if (bpf_dl) - return 0; /* Already loaded */ - - dl = dlopen("libbpf.so.0", RTLD_LAZY); - if (!dl) - return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), - "libbpf is not installed: %s", dlerror()); - - r = dlsym_many_or_warn( - dl, - LOG_DEBUG, + return dlopen_many_sym_or_warn( + &bpf_dl, "libbpf.so.0", LOG_DEBUG, DLSYM_ARG(bpf_link__destroy), DLSYM_ARG(bpf_link__fd), DLSYM_ARG(bpf_map__fd), @@ -52,15 +40,7 @@ int dlopen_bpf(void) { DLSYM_ARG(bpf_probe_prog_type), DLSYM_ARG(bpf_program__attach_cgroup), DLSYM_ARG(bpf_program__name), - DLSYM_ARG(libbpf_get_error), - NULL); - if (r < 0) - return r; - - /* Note that we never release the reference here, because there's no real reason to, after all this - * was traditionally a regular shared library dependency which lives forever too. */ - bpf_dl = TAKE_PTR(dl); - return 1; + DLSYM_ARG(libbpf_get_error)); } #else diff --git a/src/shared/cryptsetup-util.c b/src/shared/cryptsetup-util.c index 99c78f6858..b93f702aff 100644 --- a/src/shared/cryptsetup-util.c +++ b/src/shared/cryptsetup-util.c @@ -51,20 +51,10 @@ crypt_token_info (*sym_crypt_token_status)(struct crypt_device *cd, int token, c int (*sym_crypt_volume_key_get)(struct crypt_device *cd, int keyslot, char *volume_key, size_t *volume_key_size, const char *passphrase, size_t passphrase_size); int dlopen_cryptsetup(void) { - _cleanup_(dlclosep) void *dl = NULL; int r; - if (cryptsetup_dl) - return 0; /* Already loaded */ - - dl = dlopen("libcryptsetup.so.12", RTLD_LAZY); - if (!dl) - return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), - "libcryptsetup support is not installed: %s", dlerror()); - - r = dlsym_many_or_warn( - dl, - LOG_DEBUG, + r = dlopen_many_sym_or_warn( + &cryptsetup_dl, "libcryptsetup.so.12", LOG_DEBUG, DLSYM_ARG(crypt_activate_by_passphrase), #if HAVE_CRYPT_ACTIVATE_BY_SIGNED_KEY DLSYM_ARG(crypt_activate_by_signed_key), @@ -104,15 +94,10 @@ int dlopen_cryptsetup(void) { DLSYM_ARG(crypt_token_max), #endif DLSYM_ARG(crypt_token_status), - DLSYM_ARG(crypt_volume_key_get), - NULL); - if (r < 0) + DLSYM_ARG(crypt_volume_key_get)); + if (r <= 0) return r; - /* Note that we never release the reference here, because there's no real reason to, after all this - * was traditionally a regular shared library dependency which lives forever too. */ - cryptsetup_dl = TAKE_PTR(dl); - /* Redirect the default logging calls of libcryptsetup to our own logging infra. (Note that * libcryptsetup also maintains per-"struct crypt_device" log functions, which we'll also set * whenever allocating a "struct crypt_device" context. Why set both? To be defensive: maybe some diff --git a/src/shared/idn-util.c b/src/shared/idn-util.c index 63a9ebf99d..6dda3af54c 100644 --- a/src/shared/idn-util.c +++ b/src/shared/idn-util.c @@ -21,32 +21,11 @@ const char *(*sym_idn2_strerror)(int rc) = NULL; int (*sym_idn2_to_unicode_8z8z)(const char * input, char ** output, int flags) = NULL; int dlopen_idn(void) { - _cleanup_(dlclosep) void *dl = NULL; - int r; - - if (idn_dl) - return 0; /* Already loaded */ - - dl = dlopen("libidn2.so.0", RTLD_LAZY); - if (!dl) - return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), - "libidn2 support is not installed: %s", dlerror()); - - r = dlsym_many_or_warn( - dl, - LOG_DEBUG, + return dlopen_many_sym_or_warn( + &idn_dl, "libidn2.so.0", LOG_DEBUG, DLSYM_ARG(idn2_lookup_u8), DLSYM_ARG(idn2_strerror), - DLSYM_ARG(idn2_to_unicode_8z8z), - NULL); - if (r < 0) - return r; - - /* Note that we never release the reference here, because there's no real reason to, after all this - * was traditionally a regular shared library dependency which lives forever too. */ - idn_dl = TAKE_PTR(dl); - - return 1; + DLSYM_ARG(idn2_to_unicode_8z8z)); } #endif diff --git a/src/shared/libfido2-util.c b/src/shared/libfido2-util.c index c69943262d..12c644dcfc 100644 --- a/src/shared/libfido2-util.c +++ b/src/shared/libfido2-util.c @@ -61,20 +61,8 @@ int (*sym_fido_dev_open)(fido_dev_t *, const char *) = NULL; const char* (*sym_fido_strerr)(int) = NULL; int dlopen_libfido2(void) { - _cleanup_(dlclosep) void *dl = NULL; - int r; - - if (libfido2_dl) - return 0; /* Already loaded */ - - dl = dlopen("libfido2.so.1", RTLD_LAZY); - if (!dl) - return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), - "libfido2 support is not installed: %s", dlerror()); - - r = dlsym_many_or_warn( - dl, - LOG_DEBUG, + return dlopen_many_sym_or_warn( + &libfido2_dl, "libfido2.so.1", LOG_DEBUG, DLSYM_ARG(fido_assert_allow_cred), DLSYM_ARG(fido_assert_free), DLSYM_ARG(fido_assert_hmac_secret_len), @@ -118,15 +106,7 @@ int dlopen_libfido2(void) { DLSYM_ARG(fido_dev_make_cred), DLSYM_ARG(fido_dev_new), DLSYM_ARG(fido_dev_open), - DLSYM_ARG(fido_strerr), - NULL); - if (r < 0) - return r; - - /* Note that we never release the reference here, because there's no real reason to, after all this - * was traditionally a regular shared library dependency which lives forever too. */ - libfido2_dl = TAKE_PTR(dl); - return 1; + DLSYM_ARG(fido_strerr)); } static int verify_features( diff --git a/src/shared/pwquality-util.c b/src/shared/pwquality-util.c index d7776e7eff..12efe62608 100644 --- a/src/shared/pwquality-util.c +++ b/src/shared/pwquality-util.c @@ -24,20 +24,8 @@ int (*sym_pwquality_set_int_value)(pwquality_settings_t *pwq, int setting, int v const char* (*sym_pwquality_strerror)(char *buf, size_t len, int errcode, void *auxerror); int dlopen_pwquality(void) { - _cleanup_(dlclosep) void *dl = NULL; - int r; - - if (pwquality_dl) - return 0; /* Already loaded */ - - dl = dlopen("libpwquality.so.1", RTLD_LAZY); - if (!dl) - return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), - "libpwquality support is not installed: %s", dlerror()); - - r = dlsym_many_or_warn( - dl, - LOG_DEBUG, + return dlopen_many_sym_or_warn( + &pwquality_dl, "libpwquality.so.1", LOG_DEBUG, DLSYM_ARG(pwquality_check), DLSYM_ARG(pwquality_default_settings), DLSYM_ARG(pwquality_free_settings), @@ -45,15 +33,7 @@ int dlopen_pwquality(void) { DLSYM_ARG(pwquality_get_str_value), DLSYM_ARG(pwquality_read_config), DLSYM_ARG(pwquality_set_int_value), - DLSYM_ARG(pwquality_strerror), - NULL); - if (r < 0) - return r; - - /* Note that we never release the reference here, because there's no real reason to, after all this - * was traditionally a regular shared library dependency which lives forever too. */ - pwquality_dl = TAKE_PTR(dl); - return 1; + DLSYM_ARG(pwquality_strerror)); } void pwq_maybe_disable_dictionary(pwquality_settings_t *pwq) { diff --git a/src/shared/qrcode-util.c b/src/shared/qrcode-util.c index 5345b7288b..db48c73610 100644 --- a/src/shared/qrcode-util.c +++ b/src/shared/qrcode-util.c @@ -5,9 +5,9 @@ #if HAVE_QRENCODE #include -#include "alloc-util.h" #include "dlfcn-util.h" #include "locale-util.h" +#include "log.h" #include "terminal-util.h" #define ANSI_WHITE_ON_BLACK "\033[40;37;1m" @@ -18,30 +18,10 @@ static QRcode* (*sym_QRcode_encodeString)(const char *string, int version, QRecL static void (*sym_QRcode_free)(QRcode *qrcode) = NULL; int dlopen_qrencode(void) { - _cleanup_(dlclosep) void *dl = NULL; - int r; - - if (qrcode_dl) - return 0; /* Already loaded */ - - dl = dlopen("libqrencode.so.4", RTLD_LAZY); - if (!dl) - return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), - "libqrcode support is not installed: %s", dlerror()); - - r = dlsym_many_or_warn( - dl, - LOG_DEBUG, + return dlopen_many_sym_or_warn( + &qrcode_dl, "libqrencode.so.4", LOG_DEBUG, DLSYM_ARG(QRcode_encodeString), - DLSYM_ARG(QRcode_free), - NULL); - if (r < 0) - return r; - - /* Note that we never release the reference here, because there's no real reason to, after all this - * was traditionally a regular shared library dependency which lives forever too. */ - qrcode_dl = TAKE_PTR(dl); - return 1; + DLSYM_ARG(QRcode_free)); } static void print_border(FILE *output, unsigned width) { From d32f7a8e9bca4170be6a3425bc5e6ce20612c636 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 22 Jun 2021 09:25:12 +0200 Subject: [PATCH 12/15] shared/tpm2-util: simplify and convert to the new helper The function would return 0 or 3. I don't think the return code was used for anything, so let's avoid the explicit calculation and return 0 or 1. --- src/shared/tpm2-util.c | 104 ++++++++++++----------------------------- 1 file changed, 29 insertions(+), 75 deletions(-) diff --git a/src/shared/tpm2-util.c b/src/shared/tpm2-util.c index da78e1f406..df6d2eef58 100644 --- a/src/shared/tpm2-util.c +++ b/src/shared/tpm2-util.c @@ -42,84 +42,38 @@ TSS2_RC (*sym_Tss2_MU_TPM2B_PUBLIC_Marshal)(TPM2B_PUBLIC const *src, uint8_t buf TSS2_RC (*sym_Tss2_MU_TPM2B_PUBLIC_Unmarshal)(uint8_t const buffer[], size_t buffer_size, size_t *offset, TPM2B_PUBLIC *dest) = NULL; int dlopen_tpm2(void) { - int r, k = 0; + int r; - if (!libtss2_esys_dl) { - _cleanup_(dlclosep) void *dl = NULL; + r = dlopen_many_sym_or_warn( + &libtss2_esys_dl, "libtss2-esys.so.0", LOG_DEBUG, + DLSYM_ARG(Esys_Create), + DLSYM_ARG(Esys_CreatePrimary), + DLSYM_ARG(Esys_Finalize), + DLSYM_ARG(Esys_FlushContext), + DLSYM_ARG(Esys_Free), + DLSYM_ARG(Esys_GetRandom), + DLSYM_ARG(Esys_Initialize), + DLSYM_ARG(Esys_Load), + DLSYM_ARG(Esys_PolicyGetDigest), + DLSYM_ARG(Esys_PolicyPCR), + DLSYM_ARG(Esys_StartAuthSession), + DLSYM_ARG(Esys_Startup), + DLSYM_ARG(Esys_Unseal)); + if (r < 0) + return r; - dl = dlopen("libtss2-esys.so.0", RTLD_LAZY); - if (!dl) - return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), - "TPM2 support is not installed: %s", dlerror()); + r = dlopen_many_sym_or_warn( + &libtss2_rc_dl, "libtss2-rc.so.0", LOG_DEBUG, + DLSYM_ARG(Tss2_RC_Decode)); + if (r < 0) + return r; - r = dlsym_many_or_warn( - dl, - LOG_DEBUG, - DLSYM_ARG(Esys_Create), - DLSYM_ARG(Esys_CreatePrimary), - DLSYM_ARG(Esys_Finalize), - DLSYM_ARG(Esys_FlushContext), - DLSYM_ARG(Esys_Free), - DLSYM_ARG(Esys_GetRandom), - DLSYM_ARG(Esys_Initialize), - DLSYM_ARG(Esys_Load), - DLSYM_ARG(Esys_PolicyGetDigest), - DLSYM_ARG(Esys_PolicyPCR), - DLSYM_ARG(Esys_StartAuthSession), - DLSYM_ARG(Esys_Startup), - DLSYM_ARG(Esys_Unseal), - NULL); - if (r < 0) - return r; - - libtss2_esys_dl = TAKE_PTR(dl); - k++; - } - - if (!libtss2_rc_dl) { - _cleanup_(dlclosep) void *dl = NULL; - - dl = dlopen("libtss2-rc.so.0", RTLD_LAZY); - if (!dl) - return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), - "TPM2 support is not installed: %s", dlerror()); - - r = dlsym_many_or_warn( - dl, - LOG_DEBUG, - DLSYM_ARG(Tss2_RC_Decode), - NULL); - if (r < 0) - return r; - - libtss2_rc_dl = TAKE_PTR(dl); - k++; - } - - if (!libtss2_mu_dl) { - _cleanup_(dlclosep) void *dl = NULL; - - dl = dlopen("libtss2-mu.so.0", RTLD_LAZY); - if (!dl) - return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), - "TPM2 support is not installed: %s", dlerror()); - - r = dlsym_many_or_warn( - dl, - LOG_DEBUG, - DLSYM_ARG(Tss2_MU_TPM2B_PRIVATE_Marshal), - DLSYM_ARG(Tss2_MU_TPM2B_PRIVATE_Unmarshal), - DLSYM_ARG(Tss2_MU_TPM2B_PUBLIC_Marshal), - DLSYM_ARG(Tss2_MU_TPM2B_PUBLIC_Unmarshal), - NULL); - if (r < 0) - return r; - - libtss2_mu_dl = TAKE_PTR(dl); - k++; - } - - return k; + return dlopen_many_sym_or_warn( + &libtss2_mu_dl, "libtss2-mu.so.0", LOG_DEBUG, + DLSYM_ARG(Tss2_MU_TPM2B_PRIVATE_Marshal), + DLSYM_ARG(Tss2_MU_TPM2B_PRIVATE_Unmarshal), + DLSYM_ARG(Tss2_MU_TPM2B_PUBLIC_Marshal), + DLSYM_ARG(Tss2_MU_TPM2B_PUBLIC_Unmarshal)); } struct tpm2_context { From cd503dbb6b4a6a6d505ce3ba2d449e418e5c415c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 22 Jun 2021 09:26:09 +0200 Subject: [PATCH 13/15] shared/dlfcn-util: add sentinel helper or for dlsym_many_or_warn() I didn't do this before to avoid churn in all the users. --- src/shared/dlfcn-util.c | 2 +- src/shared/dlfcn-util.h | 4 +++- src/shared/idn-util.c | 3 +-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/shared/dlfcn-util.c b/src/shared/dlfcn-util.c index c027e3396b..a321df3c67 100644 --- a/src/shared/dlfcn-util.c +++ b/src/shared/dlfcn-util.c @@ -26,7 +26,7 @@ static int dlsym_many_or_warnv(void *dl, int log_level, va_list ap) { return 0; } -int dlsym_many_or_warn(void *dl, int log_level, ...) { +int dlsym_many_or_warn_sentinel(void *dl, int log_level, ...) { va_list ap; int r; diff --git a/src/shared/dlfcn-util.h b/src/shared/dlfcn-util.h index 87585b406a..d786d035d7 100644 --- a/src/shared/dlfcn-util.h +++ b/src/shared/dlfcn-util.h @@ -7,9 +7,11 @@ DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(void*, dlclose, NULL); -int dlsym_many_or_warn(void *dl, int log_level, ...) _sentinel_; +int dlsym_many_or_warn_sentinel(void *dl, int log_level, ...) _sentinel_; int dlopen_many_sym_or_warn_sentinel(void **dlp, const char *filename, int log_level, ...) _sentinel_; +#define dlsym_many_or_warn(dl, log_level, ...) \ + dlsym_many_or_warn_sentinel(dl, log_level, __VA_ARGS__, NULL) #define dlopen_many_sym_or_warn(dlp, filename, log_level, ...) \ dlopen_many_sym_or_warn_sentinel(dlp, filename, log_level, __VA_ARGS__, NULL) diff --git a/src/shared/idn-util.c b/src/shared/idn-util.c index 6dda3af54c..d4108d0c8e 100644 --- a/src/shared/idn-util.c +++ b/src/shared/idn-util.c @@ -58,8 +58,7 @@ int dlopen_idn(void) { DLSYM_ARG(idna_to_ascii_4i), DLSYM_ARG(idna_to_unicode_44i), DLSYM_ARG(stringprep_ucs4_to_utf8), - DLSYM_ARG(stringprep_utf8_to_ucs4), - NULL); + DLSYM_ARG(stringprep_utf8_to_ucs4)); if (r < 0) return r; From c3b8bacd7bdf5ca3fcd5d4df6b3f2987e9e820c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 22 Jun 2021 10:32:30 +0200 Subject: [PATCH 14/15] shared/selinux-util: rework switching of the getenforce() function The approach with function pointer was neat, but it gets in the way when we want to resolve the symbol dynamically: static initialization is not possible. It also makes the code more complicated than necessary. In this case, a simple boolean is sufficient. --- src/shared/selinux-util.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/shared/selinux-util.c b/src/shared/selinux-util.c index 30229509b3..03cee76f64 100644 --- a/src/shared/selinux-util.c +++ b/src/shared/selinux-util.c @@ -36,9 +36,9 @@ static int mac_selinux_reload(int seqno); static int cached_use = -1; static bool initialized = false; -static int (*enforcing_status_func)(void) = security_getenforce; static int last_policyload = 0; static struct selabel_handle *label_hnd = NULL; +static bool have_status_page = false; #define log_enforcing(...) \ log_full(mac_selinux_enforcing() ? LOG_ERR : LOG_WARNING, __VA_ARGS__) @@ -70,11 +70,19 @@ bool mac_selinux_use(void) { } bool mac_selinux_enforcing(void) { + int r = 0; #if HAVE_SELINUX - return enforcing_status_func() != 0; -#else - return false; + + /* If the SELinux status page has been successfully opened, retrieve the enforcing + * status over it to avoid system calls in security_getenforce(). */ + + if (have_status_page) + r = selinux_status_getenforce(); + else + r = security_getenforce(); + #endif + return r != 0; } void mac_selinux_retest(void) { @@ -142,7 +150,6 @@ static int open_label_db(void) { int mac_selinux_init(void) { #if HAVE_SELINUX int r; - bool have_status_page = false; if (initialized) return 0; @@ -170,11 +177,6 @@ int mac_selinux_init(void) { * first call without any actual change. */ last_policyload = selinux_status_policyload(); - if (have_status_page) - /* Now that the SELinux status page has been successfully opened, retrieve the enforcing - * status over it (to avoid system calls in security_getenforce()). */ - enforcing_status_func = selinux_status_getenforce; - initialized = true; #endif return 0; @@ -215,9 +217,8 @@ void mac_selinux_finish(void) { label_hnd = NULL; } - enforcing_status_func = security_getenforce; - selinux_status_close(); + have_status_page = false; initialized = false; #endif From da90c261afdfba89bfca14950e08a7ad300c915f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 22 Jun 2021 16:17:01 +0200 Subject: [PATCH 15/15] gitignore: add jekyll cache directory Follow-up for 2d4efd1dba568e59b149fbb82b51201951e8e178. --- docs/.gitignore | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/.gitignore b/docs/.gitignore index ca35be08d4..ab2d6770b8 100644 --- a/docs/.gitignore +++ b/docs/.gitignore @@ -1 +1,2 @@ -_site +/_site/ +/.jekyll-cache/