clang-tidy: flag direct open(), openat() and creat() calls

The tree opens files via the xopenat() family, which sets O_CLOEXEC on
its own and refuses to create files through dangling symlinks. Nothing
stops new code from calling open() directly though. The repart manifest
code added such a call after the migration. Let's add open(), openat()
and creat() to the custom functions of bugprone-unsafe-functions so that
clang-tidy reports direct calls.

The functions that implement xopenat() and fd_reopen() call the syscalls
directly, and so do the seccomp tests and the O_EMPTYPATH probe in
test-fd-util.c. We suppress the check in those places.

openat2() isn't on the list. override/fcntl.h defines openat2 as a macro
for openat2_shim(), and the check ignores calls that come from a macro
expansion. An entry for openat2() wouldn't report anything.

.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;

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

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

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,

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);

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;