From dc1752ea72ef9cda3fc4c9445a8dc436bd900884 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 10 Dec 2022 11:23:05 +0900 Subject: [PATCH 1/5] stat-util: introduce fd_is_read_only_fs() Then, reimplement path_is_read_only_fs() by the function to avoid race. --- src/basic/stat-util.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/src/basic/stat-util.c b/src/basic/stat-util.c index 97dbcaac66..0974f6178a 100644 --- a/src/basic/stat-util.c +++ b/src/basic/stat-util.c @@ -163,26 +163,37 @@ int null_or_empty_fd(int fd) { return null_or_empty(&st); } -int path_is_read_only_fs(const char *path) { +static int fd_is_read_only_fs(int fd) { struct statvfs st; - assert(path); + assert(fd >= 0); - if (statvfs(path, &st) < 0) + if (fstatvfs(fd, &st) < 0) return -errno; if (st.f_flag & ST_RDONLY) return true; - /* On NFS, statvfs() might not reflect whether we can actually - * write to the remote share. Let's try again with - * access(W_OK) which is more reliable, at least sometimes. */ - if (access(path, W_OK) < 0 && errno == EROFS) + /* On NFS, fstatvfs() might not reflect whether we can actually write to the remote share. Let's try + * again with access(W_OK) which is more reliable, at least sometimes. */ + if (access_fd(fd, W_OK) == -EROFS) return true; return false; } +int path_is_read_only_fs(const char *path) { + _cleanup_close_ int fd = -EBADFD; + + assert(path); + + fd = open(path, O_CLOEXEC | O_PATH); + if (fd < 0) + return -errno; + + return fd_is_read_only_fs(fd); +} + int files_same(const char *filea, const char *fileb, int flags) { struct stat a, b; From a51a9684c926af917ad1b8e2f34538aee3c7ce2d Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 10 Dec 2022 11:25:28 +0900 Subject: [PATCH 2/5] binfmt-util: split out binfmt_mounted() No functional changes, just refactoring and preparation for later commits. --- src/shared/binfmt-util.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/shared/binfmt-util.c b/src/shared/binfmt-util.c index 724d7f27d9..e813c3ebcb 100644 --- a/src/shared/binfmt-util.c +++ b/src/shared/binfmt-util.c @@ -5,10 +5,29 @@ #include #include "binfmt-util.h" +#include "errno-util.h" +#include "fd-util.h" #include "fileio.h" #include "missing_magic.h" #include "stat-util.h" +static int binfmt_mounted(void) { + _cleanup_close_ int fd = -EBADF; + int r; + + fd = RET_NERRNO(open("/proc/sys/fs/binfmt_misc", O_CLOEXEC | O_DIRECTORY | O_PATH)); + if (fd == -ENOENT) + return false; + if (fd < 0) + return fd; + + r = fd_is_fs_type(fd, BINFMTFS_MAGIC); + if (r <= 0) + return r; + + return true; +} + int disable_binfmt(void) { int r; @@ -18,13 +37,13 @@ int disable_binfmt(void) { * We are a bit careful here, since binfmt_misc might still be an autofs which we don't want to * trigger. */ - r = path_is_fs_type("/proc/sys/fs/binfmt_misc", BINFMTFS_MAGIC); - if (r == 0 || r == -ENOENT) { + r = binfmt_mounted(); + if (r < 0) + return log_warning_errno(r, "Failed to determine whether binfmt_misc is mounted: %m"); + if (r == 0) { log_debug("binfmt_misc is not mounted, not detaching entries."); return 0; } - if (r < 0) - return log_warning_errno(r, "Failed to determine whether binfmt_misc is mounted: %m"); r = write_string_file("/proc/sys/fs/binfmt_misc/status", "-1", WRITE_STRING_FILE_DISABLE_BUFFER); if (r < 0) From 5aaa79ce96997825b4dcdcf5be10d8f955fb3339 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 10 Dec 2022 11:32:24 +0900 Subject: [PATCH 3/5] binfmt-util: also check if binfmt is mounted in read-write --- src/shared/binfmt-util.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/shared/binfmt-util.c b/src/shared/binfmt-util.c index e813c3ebcb..51fc245c01 100644 --- a/src/shared/binfmt-util.c +++ b/src/shared/binfmt-util.c @@ -8,6 +8,7 @@ #include "errno-util.h" #include "fd-util.h" #include "fileio.h" +#include "fs-util.h" #include "missing_magic.h" #include "stat-util.h" @@ -25,7 +26,7 @@ static int binfmt_mounted(void) { if (r <= 0) return r; - return true; + return access_fd(fd, W_OK) >= 0; } int disable_binfmt(void) { @@ -41,7 +42,7 @@ int disable_binfmt(void) { if (r < 0) return log_warning_errno(r, "Failed to determine whether binfmt_misc is mounted: %m"); if (r == 0) { - log_debug("binfmt_misc is not mounted, not detaching entries."); + log_debug("binfmt_misc is not mounted in read-write mode, not detaching entries."); return 0; } From 94ba5b15329d9d19277ca418bfd0266da98f7b2a Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 10 Dec 2022 11:42:50 +0900 Subject: [PATCH 4/5] binfmt: check if binfmt is mounted before applying rules --- src/binfmt/binfmt.c | 24 ++++++++++++++++++++++-- src/shared/binfmt-util.c | 2 +- src/shared/binfmt-util.h | 1 + 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/binfmt/binfmt.c b/src/binfmt/binfmt.c index ba209a8d47..624c63ec98 100644 --- a/src/binfmt/binfmt.c +++ b/src/binfmt/binfmt.c @@ -190,6 +190,18 @@ static int parse_argv(int argc, char *argv[]) { return 1; } +static int binfmt_mounted_warn(void) { + int r; + + r = binfmt_mounted(); + if (r < 0) + return log_error_errno(r, "Failed to check if /proc/sys/fs/binfmt_misc is mounted: %m"); + if (r == 0) + log_debug("/proc/sys/fs/binfmt_misc is not mounted in read-write mode, skipping."); + + return r; +} + static int run(int argc, char *argv[]) { int r, k; @@ -206,13 +218,17 @@ static int run(int argc, char *argv[]) { if (arg_unregister) return disable_binfmt(); - if (argc > optind) + if (argc > optind) { + r = binfmt_mounted_warn(); + if (r <= 0) + return r; + for (int i = optind; i < argc; i++) { k = apply_file(argv[i], false); if (k < 0 && r >= 0) r = k; } - else { + } else { _cleanup_strv_free_ char **files = NULL; r = conf_files_list_strv(&files, ".conf", NULL, 0, (const char**) CONF_PATHS_STRV("binfmt.d")); @@ -225,6 +241,10 @@ static int run(int argc, char *argv[]) { return cat_files(NULL, files, 0); } + r = binfmt_mounted_warn(); + if (r <= 0) + return r; + /* Flush out all rules */ r = write_string_file("/proc/sys/fs/binfmt_misc/status", "-1", WRITE_STRING_FILE_DISABLE_BUFFER); if (r < 0) diff --git a/src/shared/binfmt-util.c b/src/shared/binfmt-util.c index 51fc245c01..a26175474b 100644 --- a/src/shared/binfmt-util.c +++ b/src/shared/binfmt-util.c @@ -12,7 +12,7 @@ #include "missing_magic.h" #include "stat-util.h" -static int binfmt_mounted(void) { +int binfmt_mounted(void) { _cleanup_close_ int fd = -EBADF; int r; diff --git a/src/shared/binfmt-util.h b/src/shared/binfmt-util.h index 2f008d12ff..13f4548b7c 100644 --- a/src/shared/binfmt-util.h +++ b/src/shared/binfmt-util.h @@ -1,4 +1,5 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once +int binfmt_mounted(void); int disable_binfmt(void); From f74a7cb45c2458f90de6d37c70fa3afc1a3be279 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Sat, 10 Dec 2022 11:46:45 +0900 Subject: [PATCH 5/5] unit: check more specific path to be written by systemd-binfmt Follow-up for 41807efb1594ae8e71e0255e154ea7d17be2251a. Replaces #25690. --- units/systemd-binfmt.service.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/units/systemd-binfmt.service.in b/units/systemd-binfmt.service.in index a702c0d9d7..b04412e037 100644 --- a/units/systemd-binfmt.service.in +++ b/units/systemd-binfmt.service.in @@ -18,7 +18,7 @@ After=proc-sys-fs-binfmt_misc.automount After=proc-sys-fs-binfmt_misc.mount After=local-fs.target Before=sysinit.target shutdown.target -ConditionPathIsReadWrite=/proc/sys/ +ConditionPathIsMountPoint=/proc/sys/fs/binfmt_misc ConditionDirectoryNotEmpty=|/lib/binfmt.d ConditionDirectoryNotEmpty=|/usr/lib/binfmt.d ConditionDirectoryNotEmpty=|/usr/local/lib/binfmt.d