ci: Replace codeql PotentiallyDangerousFunction query with clang-tidy (#41246)

.clang-tidy

@@ -15,6 +15,7 @@ Checks: '
     bugprone-suspicious-string-compare,
     bugprone-swapped-arguments,
     bugprone-tautological-type-limits,
+    bugprone-unsafe-functions,
     bugprone-unused-return-value,
     misc-header-include-cycle,
     misc-include-cleaner,
@@ -50,6 +51,23 @@ CheckOptions:
     varlink-io\.systemd\..*;
     varlink-idl-common\.h;
     unistd\.h
+'
+  bugprone-unsafe-functions.ReportDefaultFunctions: false
+  bugprone-unsafe-functions.CustomFunctions: '
+    ^fgets$,read_line(),is potentially dangerous;
+    ^strtok$,extract_first_word(),is potentially dangerous;
+    ^strsep$,extract_first_word(),is potentially dangerous;
+    ^dup$,fcntl() with F_DUPFD_CLOEXEC,is potentially dangerous;
+    ^htonl$,htobe32(),is confusing;
+    ^htons$,htobe16(),is confusing;
+    ^ntohl$,be32toh(),is confusing;
+    ^ntohs$,be16toh(),is confusing;
+    ^strerror$,STRERROR() or printf %m,is not thread-safe;
+    ^accept$,accept4(),is not O_CLOEXEC-safe;
+    ^dirname$,path_extract_directory(),is icky;
+    ^basename$,path_extract_filename(),is icky;
+    ^setmntent$,libmount_parse_fstab(),libmount parser should be used instead;
+    ^getmntent$,mnt_table_next_fs(),libmount parser should be used instead
 '
   misc-header-include-cycle.IgnoredFilesList: 'glib-2.0'
 WarningsAsErrors: '*'

.github/codeql-queries/PotentiallyDangerousFunction.ql

@@ -1,68 +0,0 @@
-/**
- * vi: sw=2 ts=2 et syntax=ql:
- *
- * Borrowed from
- * https://github.com/Semmle/ql/blob/master/cpp/ql/src/Security/CWE/CWE-676/PotentiallyDangerousFunction.ql
- *
- * @name Use of potentially dangerous function
- * @description Certain standard library functions are dangerous to call.
- * @id cpp/potentially-dangerous-function
- * @kind problem
- * @problem.severity error
- * @precision high
- * @tags reliability
- *       security
- */
-import cpp
-
-predicate potentiallyDangerousFunction(Function f, string message) {
-  (
-    f.getQualifiedName() = "fgets" and
-    message = "Call to fgets() is potentially dangerous. Use read_line() instead."
-  ) or (
-    f.getQualifiedName() = "strtok" and
-    message = "Call to strtok() is potentially dangerous. Use extract_first_word() instead."
-  ) or (
-    f.getQualifiedName() = "strsep" and
-    message = "Call to strsep() is potentially dangerous. Use extract_first_word() instead."
-  ) or (
-    f.getQualifiedName() = "dup" and
-    message = "Call to dup() is potentially dangerous. Use fcntl(fd, FD_DUPFD_CLOEXEC, 3) instead."
-  ) or (
-    f.getQualifiedName() = "htonl" and
-    message = "Call to htonl() is confusing. Use htobe32() instead."
-  ) or (
-    f.getQualifiedName() = "htons" and
-    message = "Call to htons() is confusing. Use htobe16() instead."
-  ) or (
-    f.getQualifiedName() = "ntohl" and
-    message = "Call to ntohl() is confusing. Use be32toh() instead."
-  ) or (
-    f.getQualifiedName() = "ntohs" and
-    message = "Call to ntohs() is confusing. Use be16toh() instead."
-  ) or (
-    f.getQualifiedName() = "strerror" and
-    message = "Call to strerror() is not thread-safe. Use printf()'s %m format string or STRERROR() instead."
-  ) or (
-    f.getQualifiedName() = "accept" and
-    message = "Call to accept() is not O_CLOEXEC-safe. Use accept4() instead."
-  ) or (
-    f.getQualifiedName() = "dirname" and
-    message = "Call dirname() is icky. Use path_extract_directory() instead."
-  ) or (
-    f.getQualifiedName() = "basename" and
-    message = "Call basename() is icky. Use path_extract_filename() instead."
-  ) or (
-    f.getQualifiedName() = "setmntent" and
-    message = "Libmount parser is used instead, specifically libmount_parse_fstab()."
-  ) or (
-    f.getQualifiedName() = "getmntent" and
-    message = "Libmount parser is used instead, specifically mnt_table_next_fs()."
-  )
-}
-
-from FunctionCall call, Function target, string message
-where
-  call.getTarget() = target and
-  potentiallyDangerousFunction(target, message)
-select call, message

src/core/dbus.c

@@ -39,7 +39,7 @@
 #include "path-util.h"
 #include "pidref.h"
 #include "process-util.h"
-#include "selinux-access.h"
+#include "selinux-access.h"     /* IWYU pragma: keep */
 #include "serialize.h"
 #include "set.h"
 #include "special.h"

src/core/load-fragment.c

@@ -57,7 +57,7 @@
 #include "reboot-util.h"
 #include "seccomp-util.h"
 #include "securebits-util.h"
-#include "selinux-util.h"
+#include "selinux-util.h"               /* IWYU pragma: keep */
 #include "set.h"
 #include "show-status.h"
 #include "signal-util.h"

src/core/meson.build

@@ -170,11 +170,14 @@ systemd_sources = files(
         'apparmor-setup.c',
         'ima-setup.c',
         'ipe-setup.c',
-        'selinux-setup.c',
         'smack-setup.c',
         'efi-random.c',
 )
 
+if conf.get('HAVE_SELINUX') == 1
+        systemd_sources += files('selinux-setup.c')
+endif
+
 systemd_executor_sources = files(
         'executor.c',
         'exec-invoke.c',

src/core/selinux-setup.c

@@ -13,11 +13,10 @@
 #include "time-util.h"
 
 int mac_selinux_setup(bool *loaded_policy) {
-        assert(loaded_policy);
-
-#if HAVE_SELINUX
         int r;
 
+        assert(loaded_policy);
+
         r = dlopen_libselinux();
         if (r < 0) {
                 log_debug_errno(r, "No SELinux library available, skipping setup.");
@@ -92,7 +91,6 @@ int mac_selinux_setup(bool *loaded_policy) {
                 } else
                         log_debug("Unable to load SELinux policy. Ignoring.");
         }
-#endif
 
         return 0;
 }

src/core/selinux-setup.h

@@ -3,4 +3,10 @@
 
 #include "core-forward.h"
 
+#if HAVE_SELINUX
 int mac_selinux_setup(bool *loaded_policy);
+#else
+static inline int mac_selinux_setup(bool *loaded_policy) {
+        return 0;
+}
+#endif

src/resolve/test-resolved-stream.c

@@ -111,7 +111,7 @@ static void *tcp_dns_server(void *p) {
         assert_se(setsockopt(bindfd, SOL_SOCKET, SO_REUSEADDR, &(int){1}, sizeof(int)) >= 0);
         assert_se(bind(bindfd, &server_address.sa, sockaddr_len(&server_address)) >= 0);
         assert_se(listen(bindfd, 1) >= 0);
-        assert_se((acceptfd = accept(bindfd, NULL, NULL)) >= 0);
+        assert_se((acceptfd = accept4(bindfd, NULL, NULL, SOCK_CLOEXEC)) >= 0);
         server_handle(acceptfd);
         return NULL;
 }

src/shared/reboot-util.c

@@ -1,21 +1,23 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 
 #include <stdint.h>
-#include <sys/ioctl.h>
-#include <sys/mman.h>
 #include <sys/syscall.h>
 #include <unistd.h>
 
 #if HAVE_XENCTRL
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+
 #define __XEN_INTERFACE_VERSION__ 0x00040900
 #include <xen/kexec.h>
 #include <xen/sys/privcmd.h>
 #include <xen/xen.h>
+
+#include "errno-util.h"
+#include "fd-util.h"
 #endif
 
 #include "alloc-util.h"
-#include "errno-util.h"
-#include "fd-util.h"
 #include "fileio.h"
 #include "log.h"
 #include "proc-cmdline.h"

src/shared/selinux-util.c

@@ -1,32 +1,34 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 
 #include <fcntl.h>
-#include <malloc.h>
-#include <string.h>
 #include <sys/socket.h>
 #include <sys/stat.h>
-#include <sys/un.h>
 #include <syslog.h>
 
 #if HAVE_SELINUX
+#include <malloc.h>
+#include <string.h>
+#include <sys/un.h>
+
 #include <selinux/avc.h>
 #include <selinux/context.h>
 #include <selinux/label.h>
 #include <selinux/selinux.h>
-#endif
 
 #include "sd-dlopen.h"
 
 #include "alloc-util.h"
-#include "errno-util.h"
 #include "fd-util.h"
 #include "label.h"
-#include "label-util.h"
 #include "log.h"
 #include "path-util.h"
-#include "selinux-util.h"
 #include "string-util.h"
 #include "time-util.h"
+#endif
+
+#include "errno-util.h"
+#include "label-util.h"
+#include "selinux-util.h"
 
 #if HAVE_SELINUX
 DEFINE_TRIVIAL_CLEANUP_FUNC_FULL_RENAME(context_t, sym_context_free, context_freep, NULL);

src/test/test-errno-util.c

@@ -6,10 +6,11 @@
 
 TEST(strerror_not_threadsafe) {
         /* Just check that strerror really is not thread-safe. */
-        log_info("strerror(%d) → %s", 200, strerror(200));
-        log_info("strerror(%d) → %s", 201, strerror(201));
-        log_info("strerror(%d) → %s", INT_MAX, strerror(INT_MAX));
+        log_info("strerror(%d) → %s", 200, strerror(200));              /* NOLINT(bugprone-unsafe-functions) */
+        log_info("strerror(%d) → %s", 201, strerror(201));              /* NOLINT(bugprone-unsafe-functions) */
+        log_info("strerror(%d) → %s", INT_MAX, strerror(INT_MAX));      /* NOLINT(bugprone-unsafe-functions) */
 
+        /* NOLINTNEXTLINE(bugprone-unsafe-functions) */
         log_info("strerror(%d), strerror(%d) → %p, %p", 200, 201, strerror(200), strerror(201));
 
         /* This call is not allowed, because the first returned string becomes invalid when

src/test/test-fd-util.c

@@ -151,7 +151,7 @@ TEST(fd_move_above_stdio) {
         new_fd = fd_move_above_stdio(new_fd);
         assert_se(new_fd >= 3);
 
-        assert_se(dup(original_stdin) == 0);
+        assert_se(fcntl(original_stdin, F_DUPFD, 0) == 0);
         assert_se(close_nointr(original_stdin) != EBADF);
         assert_se(close_nointr(new_fd) != EBADF);
 }