diff --git a/.clang-tidy b/.clang-tidy index c784c9532b1..ccd2d12069d 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -63,6 +63,9 @@ CheckOptions: ^ntohs$,be16toh(),is confusing; ^strerror$,STRERROR() or printf %m,is not thread-safe; ^accept$,accept4(),is not O_CLOEXEC-safe; + ^open$,xopenat(),does not set O_CLOEXEC by default; + ^openat$,xopenat(),does not set O_CLOEXEC by default; + ^creat$,xopenat(),is an obsolete relic that does not set O_CLOEXEC; ^dirname$,path_extract_directory(),is icky; ^basename$,path_extract_filename(),is icky; ^setmntent$,libmount_parse_fstab(),libmount parser should be used instead; diff --git a/docs/CODING_STYLE.md b/docs/CODING_STYLE.md index df3730e493b..378de8248e2 100644 --- a/docs/CODING_STYLE.md +++ b/docs/CODING_STYLE.md @@ -911,6 +911,10 @@ SPDX-License-Identifier: LGPL-2.1-or-later `execve()` the `O_CLOEXEC` bit must be cleared explicitly via `fd_cloexec(fd, false)` right before the `execve()`. +- Open files with `xopenat()` or one of the other helpers above instead of + calling `open()`, `openat()` or `creat()` directly. This also applies to + tests. clang-tidy reports direct calls. + - It's a good idea to use `O_NONBLOCK` when opening 'foreign' regular files, i.e. file system objects that are supposed to be regular files whose paths were specified by the user and hence might actually refer to other types of diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index b791d895da6..93a1bd5137f 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -783,6 +783,8 @@ finish: return r; } +/* fd_reopen() calls open() and openat() directly. clang-tidy reports direct calls in all other code. */ +/* NOLINTBEGIN(bugprone-unsafe-functions) */ int fd_reopen(int fd, int flags) { assert(wildcard_fd_is_valid(fd)); assert(!FLAGS_SET(flags, O_CREAT)); @@ -854,6 +856,7 @@ int fd_reopen(int fd, int flags) { return new_fd; } +/* NOLINTEND(bugprone-unsafe-functions) */ static int fd_reopen_propagate_append_and_position(int fd, int flags) { /* Invokes fd_reopen(fd, flags), but propagates O_APPEND if set on original fd, and also tries to diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 125079ae8fa..94b340fa9a3 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -1081,6 +1081,9 @@ int open_mkdir_at_full_label(int dirfd, const char *path, int flags, XOpenFlags return TAKE_FD(fd); } +/* openat_report_new() and xopenat_full_label() call open() and openat() directly. clang-tidy reports + * direct calls in all other code. */ +/* NOLINTBEGIN(bugprone-unsafe-functions) */ int openat_report_new(int dirfd, const char *pathname, int flags, mode_t mode, bool *ret_newly_created) { int fd, r; @@ -1531,6 +1534,7 @@ error: return r; } +/* NOLINTEND(bugprone-unsafe-functions) */ int xopenat_lock_full_label( int dir_fd, diff --git a/src/test/test-fd-util.c b/src/test/test-fd-util.c index d68f06c30eb..13625f6e732 100644 --- a/src/test/test-fd-util.c +++ b/src/test/test-fd-util.c @@ -997,8 +997,9 @@ TEST(fd_reopen_emptypath) { ASSERT_OK(tfd = mkdtemp_open(/* template= */ NULL, /* flags= */ 0, &t)); - /* Older kernels ignore the flag and miss the empty path */ - fd = openat(tfd, "", O_PATH|O_CLOEXEC|O_EMPTYPATH); + /* Older kernels ignore the flag and miss the empty path. xopenat() returns -ENOENT for an empty path + * without calling openat(), so the probe calls openat() directly. */ + fd = openat(tfd, "", O_PATH|O_CLOEXEC|O_EMPTYPATH); /* NOLINT(bugprone-unsafe-functions) */ if (fd < 0 && errno == ENOENT) return (void) log_tests_skipped("kernel lacks O_EMPTYPATH"); ASSERT_OK_ERRNO(fd); diff --git a/src/test/test-seccomp.c b/src/test/test-seccomp.c index 39c6752814b..063aa905751 100644 --- a/src/test/test-seccomp.c +++ b/src/test/test-seccomp.c @@ -1013,7 +1013,7 @@ static int real_open(const char *path, int flags, mode_t mode) { #if defined __NR_open && __NR_open >= 0 return (int) syscall(__NR_open, path, flags, mode); #else - return open(path, flags, mode); + return open(path, flags, mode); /* NOLINT(bugprone-unsafe-functions) */ #endif } @@ -1035,6 +1035,9 @@ static int try_fchmodat2(int dirfd, const char *path, mode_t mode, int flags) { return RET_NERRNO(fchmodat(dirfd, path, mode, flags)); } +/* restrict_suid_sgid tests the seccomp filter on the creat() and openat() syscalls. The test has to call + * creat() and openat() directly. */ +/* NOLINTBEGIN(bugprone-unsafe-functions) */ TEST(restrict_suid_sgid) { int r; @@ -1228,7 +1231,11 @@ TEST(restrict_suid_sgid) { _exit(EXIT_SUCCESS); } } +/* NOLINTEND(bugprone-unsafe-functions) */ +/* After seccomp_suppress_sync(), the open() syscall fails with EINVAL when O_SYNC is set. The test calls + * open() directly to check that. */ +/* NOLINTBEGIN(bugprone-unsafe-functions) */ static void test_seccomp_suppress_sync_child(void) { _cleanup_(unlink_and_freep) char *path = NULL; _cleanup_close_ int fd = -EBADF; @@ -1260,6 +1267,7 @@ static void test_seccomp_suppress_sync_child(void) { ASSERT_ERROR_ERRNO(fsync(-1), EBADF); ASSERT_ERROR_ERRNO(syncfs(-1), EBADF); } +/* NOLINTEND(bugprone-unsafe-functions) */ TEST(seccomp_suppress_sync) { int r;