Merge ba692f2bce into c4d7a13c06

key-data might be NULL. Fixes crash:

0  0x0000559c62120530 in attach_luks_or_plain_or_bitlk (cd=0x559c6b192830, name=0x7ffd57981dc4 "root", token_type=TOKEN_FIDO2, key_file=0x0, key_data=0x0, passwords=0x0, flags=524296, until=0)
    at ../src/cryptsetup/cryptsetup.c:2234
        pass_volume_key = false
        r = 1469577760
        __func__ = '\000' <repeats 29 times>
1  0x0000559c6212279c in run (argc=6, argv=0x7ffd5797fe98) at ../src/cryptsetup/cryptsetup.c:2597
        discovered_key_data = {iov_base = 0x0, iov_len = 0}
        key_data = 0x0
        token_type = TOKEN_FIDO2
        destroy_key_file = 0x0
        flags = 524296
        until = 0
        passphrase_type = PASSPHRASE_NONE
        volume = 0x7ffd57981dc4 "root"
        source = 0x7ffd57981dc9 "/dev/disk/by-uuid/8372fb39-9ba4-461a-a618-07dcaae66280"
        status = CRYPT_INACTIVE
        tries = 0
        key_file = 0x0
        config = 0x7ffd57981e05 "luks,discard,fido2-device=auto,x-initrd.attach"
        use_cached_passphrase = true
        try_discover_key = true
        discovered_key_fn = 0x7ffd5797fa70 "root.key"
        passwords = 0x0
        cd = 0x559c6b192830
        verb = 0x7ffd57981dbd "attach"
        r = 0
        __func__ = "\000\000\000"
2  0x0000559c621231e6 in main (argc=6, argv=0x7ffd5797fe98) at ../src/cryptsetup/cryptsetup.c:2674
        r = 32553
        __func__ = "\000\000\000\000"

Follow-up for 53b6c99018

man/daemon.xml

@@ -684,6 +684,15 @@ fi</programlisting>
     <citerefentry><refentrytitle>file-hierarchy</refentrytitle><manvolnum>7</manvolnum></citerefentry>.</para>
   </refsect1>
 
+  <refsect1>
+    <title>Notes</title>
+
+    <para>
+       All example codes in this page are licensed under <literal>MIT No Attribution</literal>
+      (SPDX-License-Identifier: MIT-0).
+    </para>
+  </refsect1>
+
   <refsect1>
     <title>See Also</title>
     <para><simplelist type="inline">

src/cryptsetup/cryptsetup-pkcs11.c

@@ -16,6 +16,7 @@
 #include "fileio.h"
 #include "format-util.h"
 #include "hexdecoct.h"
+#include "iovec-util.h"
 #include "macro.h"
 #include "memory-util.h"
 #include "parse-util.h"
@@ -31,8 +32,7 @@ int decrypt_pkcs11_key(
                 const char *key_file,         /* We either expect key_file and associated parameters to be set (for file keys) … */
                 size_t key_file_size,
                 uint64_t key_file_offset,
-                const void *key_data,         /* … or key_data and key_data_size (for literal keys) */
-                size_t key_data_size,
+                const struct iovec *key_data, /* … or literal keys via key_data */
                 usec_t until,
                 AskPasswordFlags askpw_flags,
                 void **ret_decrypted_key,
@@ -47,15 +47,15 @@ int decrypt_pkcs11_key(
 
         assert(friendly_name);
         assert(pkcs11_uri);
-        assert(key_file || key_data);
+        assert(key_file || iovec_is_set(key_data));
         assert(ret_decrypted_key);
         assert(ret_decrypted_key_size);
 
         /* The functions called here log about all errors, except for EAGAIN which means "token not found right now" */
 
-        if (key_data) {
-                data.encrypted_key = (void*) key_data;
-                data.encrypted_key_size = key_data_size;
+        if (iovec_is_set(key_data)) {
+                data.encrypted_key = (void*) key_data->iov_base;
+                data.encrypted_key_size = key_data->iov_len;
 
                 data.free_encrypted_key = false;
         } else {

src/cryptsetup/cryptsetup-pkcs11.h

@@ -16,8 +16,7 @@ int decrypt_pkcs11_key(
                 const char *key_file,
                 size_t key_file_size,
                 uint64_t key_file_offset,
-                const void *key_data,
-                size_t key_data_size,
+                const struct iovec *key_data,
                 usec_t until,
                 AskPasswordFlags askpw_flags,
                 void **ret_decrypted_key,
@@ -39,8 +38,7 @@ static inline int decrypt_pkcs11_key(
                 const char *key_file,
                 size_t key_file_size,
                 uint64_t key_file_offset,
-                const void *key_data,
-                size_t key_data_size,
+                const struct iovec *key_data,
                 usec_t until,
                 AskPasswordFlags askpw_flags,
                 void **ret_decrypted_key,

src/cryptsetup/cryptsetup.c

@@ -1471,8 +1471,7 @@ static int attach_luks_or_plain_or_bitlk_by_fido2(
                 struct crypt_device *cd,
                 const char *name,
                 const char *key_file,
-                const void *key_data,
-                size_t key_data_size,
+                const struct iovec *key_data,
                 usec_t until,
                 uint32_t flags,
                 bool pass_volume_key) {
@@ -1489,7 +1488,7 @@ static int attach_luks_or_plain_or_bitlk_by_fido2(
         assert(name);
         assert(arg_fido2_device || arg_fido2_device_auto);
 
-        if (arg_fido2_cid && !key_file && !key_data)
+        if (arg_fido2_cid && !key_file && !iovec_is_set(key_data))
                 return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
                                         "FIDO2 mode with manual parameters selected, but no keyfile specified, refusing.");
 
@@ -1513,7 +1512,7 @@ static int attach_luks_or_plain_or_bitlk_by_fido2(
                                                 arg_fido2_rp_id,
                                                 arg_fido2_cid, arg_fido2_cid_size,
                                                 key_file, arg_keyfile_size, arg_keyfile_offset,
-                                                key_data, key_data_size,
+                                                key_data,
                                                 until,
                                                 arg_fido2_manual_flags,
                                                 "cryptsetup.fido2-pin",
@@ -1623,8 +1622,7 @@ static int attach_luks_or_plain_or_bitlk_by_pkcs11(
                 struct crypt_device *cd,
                 const char *name,
                 const char *key_file,
-                const void *key_data,
-                size_t key_data_size,
+                const struct iovec *key_data,
                 usec_t until,
                 uint32_t flags,
                 bool pass_volume_key) {
@@ -1635,6 +1633,7 @@ static int attach_luks_or_plain_or_bitlk_by_pkcs11(
         _cleanup_(erase_and_freep) void *decrypted_key = NULL;
         _cleanup_(sd_event_unrefp) sd_event *event = NULL;
         _cleanup_free_ void *discovered_key = NULL;
+        struct iovec discovered_key_data = {};
         int keyslot = arg_key_slot, r;
         const char *uri = NULL;
         bool use_libcryptsetup_plugin = use_token_plugins();
@@ -1653,13 +1652,13 @@ static int attach_luks_or_plain_or_bitlk_by_pkcs11(
                                 return r;
 
                         uri = discovered_uri;
-                        key_data = discovered_key;
-                        key_data_size = discovered_key_size;
+                        discovered_key_data = IOVEC_MAKE(discovered_key, discovered_key_size);
+                        key_data = &discovered_key_data;
                 }
         } else {
                 uri = arg_pkcs11_uri;
 
-                if (!key_file && !key_data)
+                if (!key_file && !iovec_is_set(key_data))
                         return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "PKCS#11 mode selected but no key file specified, refusing.");
         }
 
@@ -1682,7 +1681,7 @@ static int attach_luks_or_plain_or_bitlk_by_pkcs11(
                                         friendly,
                                         uri,
                                         key_file, arg_keyfile_size, arg_keyfile_offset,
-                                        key_data, key_data_size,
+                                        key_data,
                                         until,
                                         arg_ask_password_flags,
                                         &decrypted_key, &decrypted_key_size);
@@ -2231,9 +2230,9 @@ static int attach_luks_or_plain_or_bitlk(
         if (token_type == TOKEN_TPM2)
                 return attach_luks_or_plain_or_bitlk_by_tpm2(cd, name, key_file, key_data, until, flags, pass_volume_key);
         if (token_type == TOKEN_FIDO2)
-                return attach_luks_or_plain_or_bitlk_by_fido2(cd, name, key_file, key_data->iov_base, key_data->iov_len, until, flags, pass_volume_key);
+                return attach_luks_or_plain_or_bitlk_by_fido2(cd, name, key_file, key_data, until, flags, pass_volume_key);
         if (token_type == TOKEN_PKCS11)
-                return attach_luks_or_plain_or_bitlk_by_pkcs11(cd, name, key_file, key_data->iov_base, key_data->iov_len, until, flags, pass_volume_key);
+                return attach_luks_or_plain_or_bitlk_by_pkcs11(cd, name, key_file, key_data, until, flags, pass_volume_key);
         if (key_data)
                 return attach_luks_or_plain_or_bitlk_by_key_data(cd, name, key_data, flags, pass_volume_key);
         if (key_file)

src/shared/cryptsetup-fido2.c

@@ -24,8 +24,7 @@ int acquire_fido2_key(
                 const char *key_file,
                 size_t key_file_size,
                 uint64_t key_file_offset,
-                const void *key_data,
-                size_t key_data_size,
+                const struct iovec *key_data,
                 usec_t until,
                 Fido2EnrollFlags required,
                 const char *askpw_credential,
@@ -45,10 +44,10 @@ int acquire_fido2_key(
                                         "Local verification is required to unlock this volume, but the 'headless' parameter was set.");
 
         assert(cid);
-        assert(key_file || key_data);
+        assert(key_file || iovec_is_set(key_data));
 
-        if (key_data)
-                salt = IOVEC_MAKE(key_data, key_data_size);
+        if (iovec_is_set(key_data))
+                salt = *key_data;
         else {
                 if (key_file_size > 0)
                         log_debug("Ignoring 'keyfile-size=' option for a FIDO2 salt file.");
@@ -252,7 +251,7 @@ int acquire_fido2_key_auto(
                                 /* key_file= */ NULL, /* salt is read from LUKS header instead of key_file */
                                 /* key_file_size= */ 0,
                                 /* key_file_offset= */ 0,
-                                salt, salt_size,
+                                &IOVEC_MAKE(salt, salt_size),
                                 until,
                                 required,
                                 "cryptsetup.fido2-pin",

src/shared/cryptsetup-fido2.h

@@ -20,8 +20,7 @@ int acquire_fido2_key(
                 const char *key_file,
                 size_t key_file_size,
                 uint64_t key_file_offset,
-                const void *key_data,
-                size_t key_data_size,
+                const struct iovec *key_data,
                 usec_t until,
                 Fido2EnrollFlags required,
                 const char *askpw_credential,
@@ -52,8 +51,7 @@ static inline int acquire_fido2_key(
                 const char *key_file,
                 size_t key_file_size,
                 uint64_t key_file_offset,
-                const void *key_data,
-                size_t key_data_size,
+                const struct iovec *key_data,
                 usec_t until,
                 Fido2EnrollFlags required,
                 const char *askpw_credential,

src/sysupdate/updatectl.c

@@ -1414,7 +1414,7 @@ static int verb_enable(int argc, char **argv, void *userdata) {
                                        "SetFeatureEnabled",
                                        &error,
                                        /* reply= */ NULL,
-                                       "sbt",
+                                       "sit",
                                        *feature,
                                        (int) enable,
                                        UINT64_C(0));

src/udev/test-udev-spawn.c

@@ -10,17 +10,44 @@
 
 #define BUF_SIZE 1024
 
-static void test_event_spawn_core(bool with_pidfd, const char *cmd, char *result_buf, size_t buf_size) {
+static void test_event_spawn_core(bool with_pidfd, const char *cmd, char *result_buf, size_t buf_size, int expected) {
         _cleanup_(sd_device_unrefp) sd_device *dev = NULL;
         _cleanup_(udev_event_freep) UdevEvent *event = NULL;
 
-        assert_se(setenv("SYSTEMD_PIDFD", yes_no(with_pidfd), 1) >= 0);
+        ASSERT_OK_ERRNO(setenv("SYSTEMD_PIDFD", yes_no(with_pidfd), 1));
 
-        assert_se(sd_device_new_from_syspath(&dev, "/sys/class/net/lo") >= 0);
-        assert_se(event = udev_event_new(dev, NULL, EVENT_TEST_SPAWN));
-        assert_se(udev_event_spawn(event, false, cmd, result_buf, buf_size, NULL) == 0);
+        ASSERT_OK(sd_device_new_from_syspath(&dev, "/sys/class/net/lo"));
+        ASSERT_NOT_NULL(event = udev_event_new(dev, NULL, EVENT_TEST_SPAWN));
+        ASSERT_EQ(udev_event_spawn(event, false, cmd, result_buf, buf_size, NULL), expected);
 
-        assert_se(unsetenv("SYSTEMD_PIDFD") >= 0);
+        ASSERT_OK_ERRNO(unsetenv("SYSTEMD_PIDFD"));
+}
+
+static void test_event_spawn_sleep_child(bool with_pidfd) {
+        _cleanup_free_ char *cmd = NULL;
+
+        log_debug("/* %s(%s) */", __func__, yes_no(with_pidfd));
+
+        ASSERT_OK(find_executable("sleep", &cmd));
+        ASSERT_NOT_NULL(strextend_with_separator(&cmd, " ", "1h"));
+
+        test_event_spawn_core(with_pidfd, cmd, NULL, 0, -EIO);
+}
+
+static void test_event_spawn_sleep(bool with_pidfd) {
+        PidRef pidref;
+        int r;
+
+        r = pidref_safe_fork("(test-worker)", FORK_LOG, &pidref);
+        ASSERT_OK(r);
+        if (r == 0) {
+                test_event_spawn_sleep_child(with_pidfd);
+                _exit(EXIT_SUCCESS);
+        }
+
+        ASSERT_OK(usleep_safe(USEC_PER_SEC));
+        ASSERT_OK(pidref_kill(&pidref, SIGTERM));
+        ASSERT_OK(wait_for_terminate_with_timeout(pidref.pid, 10 * USEC_PER_SEC));
 }
 
 static void test_event_spawn_cat(bool with_pidfd, size_t buf_size) {
@@ -30,18 +57,18 @@ static void test_event_spawn_cat(bool with_pidfd, size_t buf_size) {
 
         log_debug("/* %s(%s) */", __func__, yes_no(with_pidfd));
 
-        assert_se(find_executable("cat", &cmd) >= 0);
-        assert_se(strextend_with_separator(&cmd, " ", "/sys/class/net/lo/uevent"));
+        ASSERT_OK(find_executable("cat", &cmd));
+        ASSERT_NOT_NULL(strextend_with_separator(&cmd, " ", "/sys/class/net/lo/uevent"));
 
         test_event_spawn_core(with_pidfd, cmd, result_buf,
-                              buf_size >= BUF_SIZE ? BUF_SIZE : buf_size);
+                              buf_size >= BUF_SIZE ? BUF_SIZE : buf_size, 0);
 
-        assert_se(lines = strv_split_newlines(result_buf));
+        ASSERT_NOT_NULL(lines = strv_split_newlines(result_buf));
         strv_print(lines);
 
         if (buf_size >= BUF_SIZE) {
-                assert_se(strv_contains(lines, "INTERFACE=lo"));
-                assert_se(strv_contains(lines, "IFINDEX=1"));
+                ASSERT_TRUE(strv_contains(lines, "INTERFACE=lo"));
+                ASSERT_TRUE(strv_contains(lines, "IFINDEX=1"));
         }
 }
 
@@ -53,15 +80,15 @@ static void test_event_spawn_self(const char *self, const char *arg, bool with_p
         log_debug("/* %s(%s, %s) */", __func__, arg, yes_no(with_pidfd));
 
         /* 'self' may contain spaces, hence needs to be quoted. */
-        assert_se(cmd = strjoin("'", self, "' ", arg));
+        ASSERT_NOT_NULL(cmd = strjoin("'", self, "' ", arg));
 
-        test_event_spawn_core(with_pidfd, cmd, result_buf, BUF_SIZE);
+        test_event_spawn_core(with_pidfd, cmd, result_buf, BUF_SIZE, 0);
 
-        assert_se(lines = strv_split_newlines(result_buf));
+        ASSERT_NOT_NULL(lines = strv_split_newlines(result_buf));
         strv_print(lines);
 
-        assert_se(strv_contains(lines, "aaa"));
-        assert_se(strv_contains(lines, "bbb"));
+        ASSERT_TRUE(strv_contains(lines, "aaa"));
+        ASSERT_TRUE(strv_contains(lines, "bbb"));
 }
 
 static void test1(void) {
@@ -114,5 +141,7 @@ int main(int argc, char *argv[]) {
         test_event_spawn_self(self, "test2", true);
         test_event_spawn_self(self, "test2", false);
 
+        test_event_spawn_sleep(true);
+        test_event_spawn_sleep(false);
         return 0;
 }

src/udev/udev-spawn.c

@@ -16,6 +16,7 @@
 #include "udev-trace.h"
 
 typedef struct Spawn {
+        UdevWorker *worker;
         sd_device *device;
         const char *cmd;
         pid_t pid;
@@ -151,11 +152,27 @@ static int on_spawn_sigchld(sd_event_source *s, const siginfo_t *si, void *userd
         return 1;
 }
 
+static int on_spawn_sigterm(sd_event_source *s, const struct signalfd_siginfo *si, void *userdata) {
+        Spawn *spawn = ASSERT_PTR(userdata);
+
+        DEVICE_TRACE_POINT(spawn_sigterm, spawn->device, spawn->cmd);
+
+        log_device_error(spawn->device, "Worker process received SIGTERM, killing spawned process '%s' ["PID_FMT"].",
+                         spawn->cmd, spawn->pid);
+
+        (void) kill_and_sigcont(spawn->pid, SIGTERM);
+
+        /* terminate the main event source of the worker. Note, the spawn->worker may be NULL in tests. */
+        if (spawn->worker)
+                (void) sd_event_exit(spawn->worker->event, 0);
+
+        return 1;
+}
+
 static int spawn_wait(Spawn *spawn) {
         _cleanup_(sd_event_unrefp) sd_event *e = NULL;
-        _cleanup_(sd_event_source_disable_unrefp) sd_event_source *sigchld_source = NULL;
-        _cleanup_(sd_event_source_disable_unrefp) sd_event_source *stdout_source = NULL;
-        _cleanup_(sd_event_source_disable_unrefp) sd_event_source *stderr_source = NULL;
+        _cleanup_(sd_event_source_disable_unrefp) sd_event_source
+                *sigchld_source = NULL, *stdout_source = NULL, *stderr_source = NULL, *sigterm_source = NULL;
         int r;
 
         assert(spawn);
@@ -201,11 +218,21 @@ static int spawn_wait(Spawn *spawn) {
         r = sd_event_add_child(e, &sigchld_source, spawn->pid, WEXITED, on_spawn_sigchld, spawn);
         if (r < 0)
                 return log_device_debug_errno(spawn->device, r, "Failed to create sigchild event source: %m");
+
         /* SIGCHLD should be processed after IO is complete */
         r = sd_event_source_set_priority(sigchld_source, SD_EVENT_PRIORITY_NORMAL + 1);
         if (r < 0)
                 return log_device_debug_errno(spawn->device, r, "Failed to set priority to sigchild event source: %m");
 
+        r = sd_event_add_signal(e, &sigterm_source, SIGTERM | SD_EVENT_SIGNAL_PROCMASK, on_spawn_sigterm, spawn);
+        if (r < 0)
+                return log_device_debug_errno(spawn->device, r, "Failed to set SIGTERM event: %m");
+
+        /* SIGTERM should be processed with higher priorities with the others. */
+        r = sd_event_source_set_priority(sigterm_source, SD_EVENT_PRIORITY_NORMAL - 1);
+        if (r < 0)
+                return log_device_debug_errno(spawn->device, r, "Failed to set priority to sigchild event source: %m");
+
         return sd_event_loop(e);
 }
 
@@ -239,6 +266,10 @@ int udev_event_spawn(
                 return 0;
         }
 
+        if (event->worker && sd_event_get_exit_code(event->worker->event, NULL) >= 0)
+                return log_device_debug_errno(event->dev, SYNTHETIC_ERRNO(EIO),
+                                              "The main event loop of the worker process already terminating, skipping execution of '%s'.", cmd);
+
         int timeout_signal = event->worker ? event->worker->timeout_signal : SIGKILL;
         usec_t timeout_usec = event->worker ? event->worker->timeout_usec : DEFAULT_WORKER_TIMEOUT_USEC;
         usec_t now_usec = now(CLOCK_MONOTONIC);
@@ -304,6 +335,7 @@ int udev_event_spawn(
         errpipe[WRITE_END] = safe_close(errpipe[WRITE_END]);
 
         spawn = (Spawn) {
+                .worker = event->worker,
                 .device = event->dev,
                 .cmd = cmd,
                 .pid = pid,

test/units/TEST-17-UDEV.device_is_processing.sh

@@ -15,10 +15,8 @@ at_exit() {
     systemctl stop testsleep.service
     rm -f /run/udev/udev.conf.d/timeout.conf
     rm -f /run/udev/rules.d/99-testsuite.rules
-    # Forcibly kills sleep command invoked by the udev rule before restarting,
-    # otherwise systemctl restart below will takes longer.
-    killall -KILL sleep
-    systemctl restart systemd-udevd.service
+    # Check if udevd can be restarted within a reasonably short time.
+    timeout 10 systemctl restart systemd-udevd.service
     ip link del "$IFNAME"
 }