dissect: Various cleanups and fixes

- Only call have_effective_cap() if needed
- Fix comments and logging

Follow up for #40212

man/portablectl.xml

@@ -459,10 +459,10 @@
     <title>Files and Directories</title>
 
     <para>Portable service images are preferably stored in <filename>/var/lib/portables/</filename>, but are also
-    searched for in <filename>/etc/portables/</filename>, <filename>/run/systemd/portables/</filename>,
+    searched for in <filename>/etc/portables/</filename>, <filename>/run/portables/</filename>,
     <filename>/usr/local/lib/portables/</filename> and <filename>/usr/lib/portables/</filename>. It's recommended not
     to place image files directly in <filename>/etc/portables/</filename> or
-    <filename>/run/systemd/portables/</filename> (as these are generally not suitable for storing large or non-textual
+    <filename>/run/portables/</filename> (as these are generally not suitable for storing large or non-textual
     data), but use these directories only for linking images located elsewhere into the image search path.</para>
 
     <para>When a portable service image is attached, matching unit files are copied onto the host into the

src/basic/meson.build

@@ -124,7 +124,7 @@ sources += basic_sources
 
 generated_gperf_headers = []
 foreach item : [
-        # name,        source,             prefix,    headers
+        # name,             source,             prefix,    headers
         ['af',              af_sources,         '',        ['<sys/socket.h>'],       ],
         ['arphrd',          arphrd_sources,     'ARPHRD_', ['<linux/if_arp.h>'],     ],
         ['capability',      capability_sources, '',        ['<linux/capability.h>'], ],

src/core/kmod-setup.c

@@ -74,25 +74,26 @@ static bool has_virtio_rng(void) {
         return has_virtio_feature("virtio-rng", STRV_MAKE("pci:v00001AF4d00001005", "pci:v00001AF4d00001044"));
 }
 
-static bool has_virtio_console(void) {
-        return has_virtio_feature("virtio-console", STRV_MAKE("virtio:d00000003v", "virtio:d0000000Bv"));
-}
-
-static bool has_virtio_vsock(void) {
-        return has_virtio_feature("virtio-vsock", STRV_MAKE("virtio:d00000013v"));
-}
-
-static bool has_virtiofs(void) {
-        return has_virtio_feature("virtiofs", STRV_MAKE("virtio:d0000001Av"));
-}
-
 static bool has_virtio_pci(void) {
         return has_virtio_feature("virtio-pci", STRV_MAKE("pci:v00001AF4d"));
 }
 
+static bool may_have_virtio(void) {
+        /* FIXME: strictly speaking, other virtio features, e.g. vsock, are independent of the virtio PCI device. */
+        return has_virtio_pci();
+}
+
 static bool in_qemu(void) {
         return IN_SET(detect_vm(), VIRTUALIZATION_KVM, VIRTUALIZATION_QEMU);
 }
+
+static bool in_vmware(void) {
+        return detect_vm() == VIRTUALIZATION_VMWARE;
+}
+
+static bool in_hyperv(void) {
+        return detect_vm() == VIRTUALIZATION_MICROSOFT;
+}
 #endif
 
 int kmod_setup(void) {
@@ -117,10 +118,12 @@ int kmod_setup(void) {
 
                 /* we want early logging to hvc consoles if possible, and make sure systemd-getty-generator
                  * can rely on all consoles being probed already. */
-                { "virtio_console",             NULL,                        false, false, has_virtio_console },
+                { "virtio_console",             NULL,                        false, false, may_have_virtio    },
 
                 /* Make sure we can send sd-notify messages over vsock as early as possible. */
-                { "vmw_vsock_virtio_transport", NULL,                        false, false, has_virtio_vsock   },
+                { "vmw_vsock_virtio_transport", NULL,                        false, false, may_have_virtio    },
+                { "vmw_vsock_vmci_transport",   NULL,                        false, false, in_vmware          },
+                { "hv_sock",                    NULL,                        false, false, in_hyperv          },
 
                 /* We can't wait for specific virtiofs tags to show up as device nodes so we have to load the
                  * virtiofs and virtio_pci modules early to make sure the virtiofs tags are found when
@@ -128,7 +131,7 @@ int kmod_setup(void) {
                  *
                  * TODO: Remove these again once https://gitlab.com/virtio-fs/virtiofsd/-/issues/128 is
                  * resolved and the kernel fix is widely available. */
-                { "virtiofs",                   "/sys/module/virtiofs",      false, false, has_virtiofs       },
+                { "virtiofs",                   "/sys/module/virtiofs",      false, false, may_have_virtio    },
                 { "virtio_pci",                 "/sys/module/virtio_pci",    false, false, has_virtio_pci     },
 
                 /* qemu_fw_cfg would be loaded by udev later, but we want to import credentials from it super early */

src/core/service.c

@@ -2886,7 +2886,7 @@ static void service_enter_refresh_credentials(Service *s) {
                                           FORK_ALLOW_DLOPEN, /* allow loading libacl to avoid doing so in pid1 */
                                           &worker);
         if (r < 0) {
-                log_unit_error_errno(UNIT(s), r, "Failed to fork process to refresh credentials in unit's namespace: %m");
+                log_unit_error_errno(UNIT(s), r, "Failed to fork process to refresh credentials: %m");
                 goto fail;
         }
         if (r == 0) {
@@ -3917,7 +3917,7 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
         else if (streq(key, "refreshed-mask")) {
                 r = service_refresh_on_reload_from_string_many(value, &s->refreshed_mask);
                 if (r < 0)
-                        log_unit_debug_errno(u, r, "Failed to parse refresh-mask value: %s", value);
+                        log_unit_debug_errno(u, r, "Failed to parse refreshed-mask value: %s", value);
         } else
                 log_unit_debug(u, "Unknown serialization key: %s", key);
 

src/dissect/dissect.c

@@ -849,19 +849,21 @@ static int parse_argv(int argc, char *argv[]) {
         } else
                 arg_via_service = r;
 
-        r = have_effective_cap(CAP_SYS_ADMIN);
-        if (r < 0)
-                return log_error_errno(r, "Failed to determine if we have CAP_SYS_ADMIN: %m");
+        if (IN_SET(arg_action, ACTION_MOUNT, ACTION_UMOUNT)) {
+                r = have_effective_cap(CAP_SYS_ADMIN);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to determine if we have CAP_SYS_ADMIN: %m");
+                if (r == 0)
+                        return log_error_errno(SYNTHETIC_ERRNO(EPERM), "Need to have CAP_SYS_ADMIN to mount/unmount images");
+        }
 
-        if (IN_SET(arg_action, ACTION_MOUNT, ACTION_UMOUNT) && r == 0)
-                return log_error_errno(SYNTHETIC_ERRNO(EPERM), "Need to have CAP_SYS_ADMIN to mount/unmount images");
-
-        r = have_effective_cap(CAP_CHOWN);
-        if (r < 0)
-                return log_error_errno(r, "Failed to determine if we have CAP_CHOWN: %m");
-
-        if (arg_action == ACTION_SHIFT && r == 0)
-                return log_error_errno(SYNTHETIC_ERRNO(EPERM), "Need to have CAP_CHOWN to shift UID ranges");
+        if (arg_action == ACTION_SHIFT) {
+                r = have_effective_cap(CAP_CHOWN);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to determine if we have CAP_CHOWN: %m");
+                if (r == 0)
+                        return log_error_errno(SYNTHETIC_ERRNO(EPERM), "Need to have CAP_CHOWN to shift UID ranges");
+        }
 
         if (IN_SET(arg_action, ACTION_ATTACH, ACTION_DETACH)) {
                 r = must_be_root();
@@ -1470,10 +1472,10 @@ static int action_list_or_mtree_or_copy_or_make_archive(DissectedImage *m, LoopD
 
         /* Determine whether to copy ownership:
          * --copy-ownership=yes: always try to preserve ownership
-         * --copy-ownership=no: never preserve ownership, use current user
-         * --copy-ownership=auto (default): preserve ownership for directory trees,
-         *                              but not for regular files (since DDI password tables are typically
-         *                              distinct from the host ones, individual file ownership is less meaningful) */
+         *  --copy-ownership=no: never preserve ownership, use current user
+         *              default: preserve ownership for directory trees,
+         *                       but not for regular files (since DDI password tables are typically
+         *                       distinct from the host ones, individual file ownership is less meaningful) */
 
         if (arg_image) {
                 assert(m);
@@ -1778,8 +1780,12 @@ static int action_umount(const char *path) {
                 return log_error_errno(r, "Failed to find backing block device for '%s': %m", canonical);
 
         r = loop_device_open(dev, 0, LOCK_EX, &d);
-        if (r < 0 && !ERRNO_IS_PRIVILEGE(r))
-                return log_device_error_errno(dev, r, "Failed to open loopback block device: %m");
+        if (r < 0) {
+                if (!ERRNO_IS_PRIVILEGE(r))
+                        return log_device_error_errno(dev, r, "Failed to open loopback block device: %m");
+
+                log_device_debug_errno(dev, r, "Lacking privileges to open loopback block device, ignoring.");
+        }
 
         /* We've locked the loop device, now we're ready to unmount. To allow the unmount to succeed, we have
          * to close the O_PATH fd we opened earlier. */

src/login/meson.build

@@ -109,7 +109,7 @@ custom_target(
         input : 'logind.conf.in',
         output : 'logind.conf',
         command : [jinja2_cmdline, '@INPUT@', '@OUTPUT@'],
-        install : install_sysconfdir_samples and pkgsysconfdir != 'no',
+        install : install_sysconfdir_samples,
         install_dir : pkgconfigfiledir)
 
 custom_target(

src/portable/portabled.c

@@ -37,7 +37,7 @@ static int manager_new(RuntimeScope scope, Manager **ret) {
                 .runtime_scope = scope,
         };
 
-        r = runtime_directory_generic(scope, "systemd/portables", &m->state_dir);
+        r = runtime_directory_generic(scope, "portables", &m->state_dir);
         if (r < 0)
                 return r;
 

src/report/report.c

@@ -85,12 +85,9 @@ static int metrics_on_query_reply(
                         log_info("Varlink timed out");
                 else
                         log_error("Varlink error: %s", error_id);
-        } else {
-                if (context->n_metrics >= METRICS_MAX) {
-                        context->n_skipped_metrics++;
-                        return 0;
-                }
-
+        } else if (context->n_metrics >= METRICS_MAX)
+                context->n_skipped_metrics++;
+        else {
                 /* Collect metrics for later sorting */
                 if (!GREEDY_REALLOC(context->metrics, context->n_metrics + 1))
                         return log_oom();
@@ -200,7 +197,7 @@ static int metrics_query(void) {
         if (!d) {
                 if (errno != ENOENT)
                         return log_error_errno(errno, "Failed to open metrics directory %s: %m", metrics_path);
-        } else {
+        } else
                 FOREACH_DIRENT(de, d,
                                return log_warning_errno(errno, "Failed to read %s: %m", metrics_path)) {
 
@@ -218,7 +215,6 @@ static int metrics_query(void) {
 
                         (void) metrics_call(&context, p);
                 }
-        }
 
         if (set_isempty(context.links))
                 log_info("No metrics sources found.");
@@ -230,18 +226,17 @@ static int metrics_query(void) {
                 r = metrics_output_sorted(&context);
                 if (r < 0)
                         return r;
-
-                if (n_skipped_sources > 0)
-                        log_warning("Too many metrics sources, only %u sources contacted, %zu sources skipped.", set_size(context.links), n_skipped_sources);
-                if (context.n_skipped_metrics > 0)
-                        log_warning("Too many metrics, only %zu metrics collected, %zu metrics skipped.", context.n_metrics, context.n_skipped_metrics);
-
-                if (n_skipped_sources > 0 ||
-                    context.n_skipped_metrics > 0)
-                        return EXIT_FAILURE;
         }
 
-        return EXIT_SUCCESS;
+        if (n_skipped_sources > 0)
+                return log_warning_errno(SYNTHETIC_ERRNO(EUCLEAN),
+                                         "Too many metrics sources, only %u sources contacted, %zu sources skipped.",
+                                         set_size(context.links), n_skipped_sources);
+        if (context.n_skipped_metrics > 0)
+                return log_warning_errno(SYNTHETIC_ERRNO(EUCLEAN),
+                                         "Too many metrics, only %zu metrics collected, %zu metrics skipped.",
+                                         context.n_metrics, context.n_skipped_metrics);
+        return 0;
 }
 
 static int help(void) {
@@ -342,4 +337,4 @@ static int run(int argc, char *argv[]) {
         return metrics_query();
 }
 
-DEFINE_MAIN_FUNCTION_WITH_POSITIVE_FAILURE(run);
+DEFINE_MAIN_FUNCTION(run);

src/resolve/resolved-mdns.c

@@ -413,14 +413,6 @@ static int on_mdns_packet(sd_event_source *s, int fd, uint32_t revents, void *us
         if (r <= 0)
                 return r;
 
-        /* Refuse traffic from the local host, to avoid query loops. However, allow legacy mDNS
-         * unicast queries through anyway (we never send those ourselves, hence no risk).
-         * i.e. check for the source port nr. */
-        if (p->sender_port == MDNS_PORT && manager_packet_from_local_address(m, p)) {
-                log_debug("Got mDNS UDP packet from local host, ignoring.");
-                return 0;
-        }
-
         scope = manager_find_scope(m, p);
         if (!scope) {
                 log_debug("Got mDNS UDP packet on unknown scope. Ignoring.");
@@ -537,6 +529,14 @@ static int on_mdns_packet(sd_event_source *s, int fd, uint32_t revents, void *us
                 if (unsolicited_packet)
                         mdns_notify_browsers_unsolicited_updates(m, p->answer, p->family);
         } else if (dns_packet_validate_query(p) > 0)  {
+                /* Refuse traffic from the local host, to avoid query loops. However, allow legacy mDNS
+                 * unicast queries through anyway (we never send those ourselves, hence no risk).
+                 * i.e. check for the source port nr. */
+                if (p->sender_port == MDNS_PORT && manager_packet_from_local_address(m, p)) {
+                        log_debug("Got mDNS UDP packet from local host, ignoring.");
+                        return 0;
+                }
+
                 log_debug("Got mDNS query packet for id %u", DNS_PACKET_ID(p));
 
                 r = mdns_scope_process_query(scope, p);

src/ssh-generator/ssh-proxy.c

@@ -268,7 +268,7 @@ static int fetch_machine(const char *machine, RuntimeScope scope, sd_json_varian
         assert(ret);
 
         _cleanup_free_ char *addr = NULL;
-        r = runtime_directory_generic(scope, "machine/io.systemd.Machine", &addr);
+        r = runtime_directory_generic(scope, "systemd/machine/io.systemd.Machine", &addr);
         if (r < 0)
                 return r;
 

src/test/test-capability-util.c

@@ -5,7 +5,6 @@
 #include <stdlib.h>
 #include <sys/prctl.h>
 #include <sys/socket.h>
-#include <sys/wait.h>
 #include <unistd.h>
 #include "pidref.h"
 
@@ -23,6 +22,7 @@
 
 static uid_t test_uid = -1;
 static gid_t test_gid = -1;
+static bool run_ambient = false;
 
 #if HAS_FEATURE_ADDRESS_SANITIZER
 /* Keep CAP_SYS_PTRACE when running under Address Sanitizer */
@@ -32,57 +32,6 @@ static const uint64_t test_flags = UINT64_C(1) << CAP_SYS_PTRACE;
 static const uint64_t test_flags = UINT64_C(1) << CAP_DAC_OVERRIDE;
 #endif
 
-/* verify cap_last_cap() against /proc/sys/kernel/cap_last_cap */
-static void test_last_cap_file(void) {
-        _cleanup_free_ char *content = NULL;
-        unsigned long val = 0;
-        int r;
-
-        r = read_one_line_file("/proc/sys/kernel/cap_last_cap", &content);
-        if (ERRNO_IS_NEG_PRIVILEGE(r))
-                return (void) log_tests_skipped_errno(r, "Failed to /proc/sys/kernel/cap_last_cap");
-        ASSERT_OK(r);
-
-        r = safe_atolu(content, &val);
-        ASSERT_OK(r);
-        assert_se(val != 0);
-        ASSERT_EQ(val, cap_last_cap());
-}
-
-/* verify cap_last_cap() against syscall probing */
-static void test_last_cap_probe(void) {
-        unsigned long p = (unsigned long)CAP_LAST_CAP;
-
-        if (prctl(PR_CAPBSET_READ, p) < 0) {
-                for (p--; p > 0; p--)
-                        if (prctl(PR_CAPBSET_READ, p) >= 0)
-                                break;
-        } else {
-                for (;; p++)
-                        if (prctl(PR_CAPBSET_READ, p+1) < 0)
-                                break;
-        }
-
-        assert_se(p != 0);
-        ASSERT_EQ(p, cap_last_cap());
-}
-
-static void fork_test(void (*test_func)(void)) {
-        pid_t pid = 0;
-
-        pid = fork();
-        assert_se(pid >= 0);
-        if (pid == 0) {
-                test_func();
-                exit(EXIT_SUCCESS);
-        } else if (pid > 0) {
-                int status;
-
-                assert_se(waitpid(pid, &status, 0) > 0);
-                assert_se(WIFEXITED(status) && WEXITSTATUS(status) == 0);
-        }
-}
-
 static void show_capabilities(void) {
         _cleanup_free_ char *e = NULL, *p = NULL, *i = NULL;
         CapabilityQuintet q;
@@ -95,35 +44,50 @@ static void show_capabilities(void) {
         log_info("Capabilities:e=%s p=%s, i=%s", e, p, i);
 }
 
-static int setup_tests(bool *run_ambient) {
-        struct passwd *nobody;
+/* verify cap_last_cap() against /proc/sys/kernel/cap_last_cap */
+TEST(last_cap_file) {
+        _cleanup_free_ char *content = NULL;
+        unsigned long val = 0;
         int r;
 
-        nobody = getpwnam(NOBODY_USER_NAME);
-        if (!nobody)
-                return log_warning_errno(SYNTHETIC_ERRNO(ENOENT), "Couldn't find 'nobody' user.");
+        r = read_one_line_file("/proc/sys/kernel/cap_last_cap", &content);
+        if (ERRNO_IS_NEG_PRIVILEGE(r))
+                return (void) log_tests_skipped_errno(r, "Failed to /proc/sys/kernel/cap_last_cap");
+        ASSERT_OK(r);
 
-        test_uid = nobody->pw_uid;
-        test_gid = nobody->pw_gid;
+        ASSERT_OK(safe_atolu(content, &val));
+        ASSERT_NE(val, 0UL);
+        ASSERT_EQ(val, cap_last_cap());
+}
 
-        r = prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_CLEAR_ALL, 0, 0, 0);
-        /* There's support for PR_CAP_AMBIENT if the prctl() call succeeded or error code was something else
-         * than EINVAL. The EINVAL check should be good enough to rule out false positives. */
-        *run_ambient = r >= 0 || errno != EINVAL;
+/* verify cap_last_cap() against syscall probing */
+TEST(last_cap_probe) {
+        unsigned long p = (unsigned long)CAP_LAST_CAP;
 
-        return 0;
+        if (prctl(PR_CAPBSET_READ, p) < 0) {
+                for (p--; p > 0; p--)
+                        if (prctl(PR_CAPBSET_READ, p) >= 0)
+                                break;
+        } else {
+                for (;; p++)
+                        if (prctl(PR_CAPBSET_READ, p+1) < 0)
+                                break;
+        }
+
+        ASSERT_NE(p, 0UL);
+        ASSERT_EQ(p, cap_last_cap());
 }
 
 static void test_drop_privileges_keep_net_raw(void) {
         int sock;
 
         sock = socket(AF_INET, SOCK_RAW, IPPROTO_UDP);
-        assert_se(sock >= 0);
+        ASSERT_OK_ERRNO(sock);
         safe_close(sock);
 
-        assert_se(drop_privileges(test_uid, test_gid, test_flags | (1ULL << CAP_NET_RAW)) >= 0);
-        assert_se(getuid() == test_uid);
-        assert_se(getgid() == test_gid);
+        ASSERT_OK(drop_privileges(test_uid, test_gid, test_flags | (1ULL << CAP_NET_RAW)));
+        ASSERT_EQ(getuid(), test_uid);
+        ASSERT_EQ(getgid(), test_gid);
         show_capabilities();
 
         sock = socket(AF_INET, SOCK_RAW, IPPROTO_UDP);
@@ -138,9 +102,9 @@ static void test_drop_privileges_dontkeep_net_raw(void) {
         ASSERT_OK(sock);
         safe_close(sock);
 
-        assert_se(drop_privileges(test_uid, test_gid, test_flags) >= 0);
-        assert_se(getuid() == test_uid);
-        assert_se(getgid() == test_gid);
+        ASSERT_OK(drop_privileges(test_uid, test_gid, test_flags));
+        ASSERT_EQ(getuid(), test_uid);
+        ASSERT_EQ(getgid(), test_gid);
         show_capabilities();
 
         sock = socket(AF_INET, SOCK_RAW, IPPROTO_UDP);
@@ -148,53 +112,103 @@ static void test_drop_privileges_dontkeep_net_raw(void) {
 }
 
 static void test_drop_privileges_fail(void) {
-        assert_se(drop_privileges(test_uid, test_gid, test_flags) >= 0);
-        assert_se(getuid() == test_uid);
-        assert_se(getgid() == test_gid);
+        ASSERT_OK(drop_privileges(test_uid, test_gid, test_flags));
+        ASSERT_EQ(getuid(), test_uid);
+        ASSERT_EQ(getgid(), test_gid);
 
-        ASSERT_LT(drop_privileges(test_uid, test_gid, test_flags), 0);
-        ASSERT_LT(drop_privileges(0, 0, test_flags), 0);
+        ASSERT_FAIL(drop_privileges(test_uid, test_gid, test_flags));
+        ASSERT_FAIL(drop_privileges(0, 0, test_flags));
 }
 
-static void test_drop_privileges(void) {
-        fork_test(test_drop_privileges_fail);
+TEST(drop_privileges) {
+        int r;
+
+        if (getuid() != 0)
+                return (void) log_tests_skipped("not running as root");
+        if (userns_has_single_user())
+                return (void) log_tests_skipped("running in single-user user namespace");
+
+        r = ASSERT_OK(pidref_safe_fork("test-cap", FORK_WAIT|FORK_DEATHSIG_SIGKILL|FORK_LOG, /* ret= */ NULL));
+        if (r == 0) {
+                test_drop_privileges_fail();
+                _exit(EXIT_SUCCESS);
+        }
 
         if (have_effective_cap(CAP_NET_RAW) <= 0) /* The remaining two tests only work if we have CAP_NET_RAW
                                                    * in the first place. If we are run in some restricted
                                                    * container environment we might not. */
                 return;
 
-        fork_test(test_drop_privileges_keep_net_raw);
-        fork_test(test_drop_privileges_dontkeep_net_raw);
+        r = ASSERT_OK(pidref_safe_fork("test-cap", FORK_WAIT|FORK_DEATHSIG_SIGKILL|FORK_LOG, /* ret= */ NULL));
+        if (r == 0) {
+                test_drop_privileges_keep_net_raw();
+                _exit(EXIT_SUCCESS);
+        }
+
+        r = ASSERT_OK(pidref_safe_fork("test-cap", FORK_WAIT|FORK_DEATHSIG_SIGKILL|FORK_LOG, /* ret= */ NULL));
+        if (r == 0) {
+                test_drop_privileges_dontkeep_net_raw();
+                _exit(EXIT_SUCCESS);
+        }
 }
 
-static void test_have_effective_cap(void) {
+static void test_have_effective_cap_impl(void) {
         ASSERT_GT(have_effective_cap(CAP_KILL), 0);
         ASSERT_GT(have_effective_cap(CAP_CHOWN), 0);
 
         ASSERT_OK(drop_privileges(test_uid, test_gid, test_flags | (1ULL << CAP_KILL)));
-        assert_se(getuid() == test_uid);
-        assert_se(getgid() == test_gid);
+        ASSERT_EQ(getuid(), test_uid);
+        ASSERT_EQ(getgid(), test_gid);
 
         ASSERT_GT(have_effective_cap(CAP_KILL), 0);
-        assert_se(have_effective_cap(CAP_CHOWN) == 0);
+        ASSERT_EQ(have_effective_cap(CAP_CHOWN), 0);
 }
 
-static void test_apply_ambient_caps(void) {
+TEST(have_effective_cap) {
+        int r;
+
+        if (getuid() != 0)
+                return (void) log_tests_skipped("not running as root");
+        if (userns_has_single_user())
+                return (void) log_tests_skipped("running in single-user user namespace");
+
+        r = ASSERT_OK(pidref_safe_fork("test-cap", FORK_WAIT|FORK_DEATHSIG_SIGKILL|FORK_LOG, /* ret= */ NULL));
+        if (r == 0) {
+                test_have_effective_cap_impl();
+                _exit(EXIT_SUCCESS);
+        }
+}
+
+static void test_apply_ambient_caps_impl(void) {
         ASSERT_OK_EQ_ERRNO(prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_IS_SET, CAP_CHOWN, 0, 0), 0);
 
-        ASSERT_OK(capability_ambient_set_apply(UINT64_C(1) << CAP_CHOWN, true));
+        ASSERT_OK(capability_ambient_set_apply(UINT64_C(1) << CAP_CHOWN, /* also_inherit= */ true));
         ASSERT_OK_POSITIVE(have_inheritable_cap(CAP_CHOWN));
 
         ASSERT_OK_EQ_ERRNO(prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_IS_SET, CAP_CHOWN, 0, 0), 1);
 
-        ASSERT_OK(capability_ambient_set_apply(0, true));
+        ASSERT_OK(capability_ambient_set_apply(0, /* also_inherit= */ true));
         ASSERT_OK_ZERO(have_inheritable_cap(CAP_CHOWN));
 
         ASSERT_OK_EQ_ERRNO(prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_IS_SET, CAP_CHOWN, 0, 0), 0);
 }
 
-static void test_ensure_cap_64_bit(void) {
+TEST(apply_ambient_caps) {
+        int r;
+
+        if (getuid() != 0)
+                return (void) log_tests_skipped("not running as root");
+        if (!run_ambient)
+                return (void) log_tests_skipped("ambient caps not supported");
+
+        r = ASSERT_OK(pidref_safe_fork("test-cap", FORK_WAIT|FORK_DEATHSIG_SIGKILL|FORK_LOG, /* ret= */ NULL));
+        if (r == 0) {
+                test_apply_ambient_caps_impl();
+                _exit(EXIT_SUCCESS);
+        }
+}
+
+TEST(ensure_cap_64_bit) {
         _cleanup_free_ char *content = NULL;
         unsigned long p = 0;
         int r;
@@ -208,13 +222,16 @@ static void test_ensure_cap_64_bit(void) {
 
         /* If caps don't fit into 64-bit anymore, we have a problem, fail the test. Moreover, we use
          * UINT64_MAX as unset, hence it must be smaller than or equals to 62 (CAP_LIMIT). */
-        assert_se(p <= CAP_LIMIT);
+        ASSERT_LE(p, (unsigned long) CAP_LIMIT);
 }
 
-static void test_capability_get_ambient(void) {
+TEST(capability_get_ambient) {
         uint64_t c;
         int r;
 
+        if (getuid() != 0)
+                return (void) log_tests_skipped("not running as root");
+
         ASSERT_OK(capability_get_ambient(&c));
 
         r = prctl(PR_CAPBSET_READ, CAP_MKNOD);
@@ -227,91 +244,82 @@ static void test_capability_get_ambient(void) {
         r = ASSERT_OK(pidref_safe_fork(
                         "(getambient)",
                         FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGTERM|FORK_WAIT|FORK_LOG,
-                        NULL));
+                        /* ret= */ NULL));
 
         if (r == 0) {
                 int x, y;
                 /* child */
-                assert_se(capability_get_ambient(&c) >= 0);
+                ASSERT_OK(capability_get_ambient(&c));
 
                 x = capability_ambient_set_apply(
                                 (UINT64_C(1) << CAP_MKNOD)|
                                 (UINT64_C(1) << CAP_LINUX_IMMUTABLE),
                                 /* also_inherit= */ true);
-                assert_se(x >= 0 || ERRNO_IS_PRIVILEGE(x));
+                ASSERT_TRUE(x >= 0 || ERRNO_IS_PRIVILEGE(x));
 
-                assert_se(capability_get_ambient(&c) >= 0);
-                assert_se(x < 0 || FLAGS_SET(c, UINT64_C(1) << CAP_MKNOD));
-                assert_se(x < 0 || FLAGS_SET(c, UINT64_C(1) << CAP_LINUX_IMMUTABLE));
-                assert_se(x < 0 || !FLAGS_SET(c, UINT64_C(1) << CAP_SETPCAP));
+                ASSERT_OK(capability_get_ambient(&c));
+                ASSERT_TRUE(x < 0 || FLAGS_SET(c, UINT64_C(1) << CAP_MKNOD));
+                ASSERT_TRUE(x < 0 || FLAGS_SET(c, UINT64_C(1) << CAP_LINUX_IMMUTABLE));
+                ASSERT_TRUE(x < 0 || !FLAGS_SET(c, UINT64_C(1) << CAP_SETPCAP));
 
                 y = capability_bounding_set_drop(
                                 ((UINT64_C(1) << CAP_LINUX_IMMUTABLE)|
                                  (UINT64_C(1) << CAP_SETPCAP)),
                                 /* right_now= */ true);
-                assert_se(y >= 0 || ERRNO_IS_PRIVILEGE(y));
+                ASSERT_TRUE(y >= 0 || ERRNO_IS_PRIVILEGE(y));
 
-                assert_se(capability_get_ambient(&c) >= 0);
-                assert_se(x < 0 || y < 0 || !FLAGS_SET(c, UINT64_C(1) << CAP_MKNOD));
-                assert_se(x < 0 || y < 0 || FLAGS_SET(c, UINT64_C(1) << CAP_LINUX_IMMUTABLE));
-                assert_se(x < 0 || y < 0 || !FLAGS_SET(c, UINT64_C(1) << CAP_SETPCAP));
+                ASSERT_OK(capability_get_ambient(&c));
+                ASSERT_TRUE(x < 0 || y < 0 || !FLAGS_SET(c, UINT64_C(1) << CAP_MKNOD));
+                ASSERT_TRUE(x < 0 || y < 0 || FLAGS_SET(c, UINT64_C(1) << CAP_LINUX_IMMUTABLE));
+                ASSERT_TRUE(x < 0 || y < 0 || !FLAGS_SET(c, UINT64_C(1) << CAP_SETPCAP));
 
                 y = capability_bounding_set_drop(
                                 (UINT64_C(1) << CAP_SETPCAP),
                                 /* right_now= */ true);
-                assert_se(y >= 0 || ERRNO_IS_PRIVILEGE(y));
+                ASSERT_TRUE(y >= 0 || ERRNO_IS_PRIVILEGE(y));
 
-                assert_se(capability_get_ambient(&c) >= 0);
-                assert_se(x < 0 || y < 0 || !FLAGS_SET(c, UINT64_C(1) << CAP_MKNOD));
-                assert_se(x < 0 || y < 0 || !FLAGS_SET(c, UINT64_C(1) << CAP_LINUX_IMMUTABLE));
-                assert_se(x < 0 || y < 0 || !FLAGS_SET(c, UINT64_C(1) << CAP_SETPCAP));
+                ASSERT_OK(capability_get_ambient(&c));
+                ASSERT_TRUE(x < 0 || y < 0 || !FLAGS_SET(c, UINT64_C(1) << CAP_MKNOD));
+                ASSERT_TRUE(x < 0 || y < 0 || !FLAGS_SET(c, UINT64_C(1) << CAP_LINUX_IMMUTABLE));
+                ASSERT_TRUE(x < 0 || y < 0 || !FLAGS_SET(c, UINT64_C(1) << CAP_SETPCAP));
 
                 _exit(EXIT_SUCCESS);
         }
 }
 
-static void test_pidref_get_capability(void) {
+TEST(pidref_get_capability) {
         CapabilityQuintet q = CAPABILITY_QUINTET_NULL;
 
-        assert_se(pidref_get_capability(&PIDREF_MAKE_FROM_PID(getpid_cached()), &q) >= 0);
-
-        assert_se(q.effective != CAP_MASK_UNSET);
-        assert_se(q.inheritable != CAP_MASK_UNSET);
-        assert_se(q.permitted != CAP_MASK_UNSET);
-        assert_se(q.effective != CAP_MASK_UNSET);
-        assert_se(q.ambient != CAP_MASK_UNSET);
-}
-
-int main(int argc, char *argv[]) {
-        bool run_ambient;
-
-        test_setup_logging(LOG_DEBUG);
-
-        test_ensure_cap_64_bit();
-
-        test_last_cap_file();
-        test_last_cap_probe();
-
         if (getuid() != 0)
-                return log_tests_skipped("not running as root");
+                return (void) log_tests_skipped("not running as root");
 
-        if (setup_tests(&run_ambient) < 0)
-                return log_tests_skipped("setup failed");
+        ASSERT_OK(pidref_get_capability(&PIDREF_MAKE_FROM_PID(getpid_cached()), &q));
 
-        show_capabilities();
-
-        if (!userns_has_single_user())
-                test_drop_privileges();
-
-        if (!userns_has_single_user())
-                fork_test(test_have_effective_cap);
-
-        if (run_ambient)
-                fork_test(test_apply_ambient_caps);
-
-        test_capability_get_ambient();
-
-        test_pidref_get_capability();
-
-        return 0;
+        ASSERT_NE(q.effective, CAP_MASK_UNSET);
+        ASSERT_NE(q.inheritable, CAP_MASK_UNSET);
+        ASSERT_NE(q.permitted, CAP_MASK_UNSET);
+        ASSERT_NE(q.effective, CAP_MASK_UNSET);
+        ASSERT_NE(q.ambient, CAP_MASK_UNSET);
 }
+
+static int intro(void) {
+        /* Try to set up nobody user/ambient caps for tests that need them.
+         * Not finding nobody is non-fatal — those tests will skip themselves. */
+        struct passwd *nobody = getpwnam(NOBODY_USER_NAME);
+        if (nobody) {
+                test_uid = nobody->pw_uid;
+                test_gid = nobody->pw_gid;
+        }
+
+        int r = prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_CLEAR_ALL, 0, 0, 0);
+        /* There's support for PR_CAP_AMBIENT if the prctl() call succeeded or error code was something else
+         * than EINVAL. The EINVAL check should be good enough to rule out false positives. */
+        run_ambient = r >= 0 || errno != EINVAL;
+
+        if (getuid() == 0)
+                show_capabilities();
+
+        return EXIT_SUCCESS;
+}
+
+DEFINE_TEST_MAIN_WITH_INTRO(LOG_DEBUG, intro);

src/udev/udevadm-info.c

@@ -346,7 +346,7 @@ static int print_record(sd_device *device, const char *prefix) {
          *
          * Coloring: let's be conservative with coloring. Let's use it to group related fields. Right now:
          *
-         *     • white for fields that give the device a name
+         *     • highlight fields that give the device a name
          *     • green for fields that categorize the device into subsystem/devtype and similar
          *     • cyan for fields about associated device nodes/symlinks/network interfaces and such
          *     • magenta for block device diskseq
@@ -354,16 +354,16 @@ static int print_record(sd_device *device, const char *prefix) {
          *     • no color for regular properties */
 
         assert_se(sd_device_get_devpath(device, &str) >= 0);
-        printf("%sP: %s%s%s\n", prefix, ansi_highlight_white(), str, ansi_normal());
+        printf("%sP: %s%s%s\n", prefix, ansi_highlight(), str, ansi_normal());
 
         if (sd_device_get_sysname(device, &str) >= 0)
-                printf("%sM: %s%s%s\n", prefix, ansi_highlight_white(), str, ansi_normal());
+                printf("%sM: %s%s%s\n", prefix, ansi_highlight(), str, ansi_normal());
 
         if (sd_device_get_sysnum(device, &str) >= 0)
-                printf("%sR: %s%s%s\n", prefix, ansi_highlight_white(), str, ansi_normal());
+                printf("%sR: %s%s%s\n", prefix, ansi_highlight(), str, ansi_normal());
 
         if (sd_device_get_device_id(device, &str) >= 0)
-                printf("%sJ: %s%s%s\n", prefix, ansi_highlight_white(), str, ansi_normal());
+                printf("%sJ: %s%s%s\n", prefix, ansi_highlight(), str, ansi_normal());
 
         if (sd_device_get_subsystem(device, &subsys) >= 0)
                 printf("%sU: %s%s%s\n", prefix, ansi_highlight_green(), subsys, ansi_normal());