From d4ea4ebc393d27aaa150b659f063f09ba75d5ffa Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 1 Jun 2023 14:48:55 +0200 Subject: [PATCH 01/12] fstab-util: drop redundant check fstab_test_option() checks this as first thing anyway, hence let's drop the redundant check. --- src/shared/fstab-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/shared/fstab-util.c b/src/shared/fstab-util.c index 164e71a150..d22cff2d82 100644 --- a/src/shared/fstab-util.c +++ b/src/shared/fstab-util.c @@ -54,7 +54,7 @@ bool fstab_is_extrinsic(const char *mount, const char *opts) { /* If this is an initrd mount, and we are not in the initrd, then leave * this around forever, too. */ - if (opts && fstab_test_option(opts, "x-initrd.mount\0") && !in_initrd()) + if (fstab_test_option(opts, "x-initrd.mount\0") && !in_initrd()) return true; return false; From 6747d9a13fb9554e720cf352dd407133fb1a79cf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 1 Jun 2023 18:49:16 +0200 Subject: [PATCH 02/12] cryptsetup: fix whitespace issue --- src/cryptsetup/cryptsetup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cryptsetup/cryptsetup.c b/src/cryptsetup/cryptsetup.c index 3dadf83d62..ad86801844 100644 --- a/src/cryptsetup/cryptsetup.c +++ b/src/cryptsetup/cryptsetup.c @@ -986,7 +986,7 @@ static int attach_tcrypt( if (arg_tcrypt_veracrypt) params.flags |= CRYPT_TCRYPT_VERA_MODES; - + if (arg_tcrypt_veracrypt && arg_tcrypt_veracrypt_pim != 0) params.veracrypt_pim = arg_tcrypt_veracrypt_pim; From 953006dcb0a37a57aed0e762ff6289700e8689e8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 1 Jun 2023 17:20:59 +0200 Subject: [PATCH 03/12] veritysetup-generator: use generic veritysetup writers at one more place Let's use the common generator_write_veritysetup_unit_section(), ggenerator_write_veritysetup_service_section(), generator_add_symlink() implementation we already have at one more place. This mostly generates the same unit, but for the first time hooks up blockdev@dev-mapper-*.device for the device, which means things like growfs on usr+root volumes will actually work now. (I mean, growfs won#t, because verity devices are immutable after all, but things *like* it that want to run between the device popping up and being mounted.) --- src/veritysetup/veritysetup-generator.c | 67 ++++++++----------------- 1 file changed, 21 insertions(+), 46 deletions(-) diff --git a/src/veritysetup/veritysetup-generator.c b/src/veritysetup/veritysetup-generator.c index 3481362e5f..9ce95608c8 100644 --- a/src/veritysetup/veritysetup-generator.c +++ b/src/veritysetup/veritysetup-generator.c @@ -50,36 +50,34 @@ STATIC_DESTRUCTOR_REGISTER(arg_usr_options, freep); static int create_device( const char *name, const char *service, - const char *hash, + const char *roothash, const char *data_what, const char *hash_what, const char *options) { - _cleanup_free_ char *u = NULL, *v = NULL, *d = NULL, *e = NULL, *u_escaped = NULL, *v_escaped = NULL, - *hash_escaped = NULL, *options_escaped = NULL; + _cleanup_free_ char *u = NULL, *v = NULL, *d = NULL, *e = NULL; _cleanup_fclose_ FILE *f = NULL; - const char *to, *from; int r; assert(name); assert(service); /* If all three pieces of information are missing, then verity is turned off */ - if (!hash && !data_what && !hash_what) + if (!roothash && !data_what && !hash_what) return 0; /* if one of them is missing however, the data is simply incomplete and this is an error */ - if (!hash) - log_error("Verity information for %s incomplete, hash unspecified.", name); + if (!roothash) + log_error("Verity information for %s incomplete, root hash unspecified.", name); if (!data_what) log_error("Verity information for %s incomplete, data device unspecified.", name); if (!hash_what) log_error("Verity information for %s incomplete, hash device unspecified.", name); - if (!hash || !data_what || !hash_what) + if (!roothash || !data_what || !hash_what) return -EINVAL; - log_debug("Using %s verity data device %s, hash device %s, options %s, and hash %s.", name, data_what, hash_what, options, hash); + log_debug("Using %s verity data device %s, hash device %s, options %s, and hash %s.", name, data_what, hash_what, options, roothash); u = fstab_node_to_udev_node(data_what); if (!u) @@ -88,13 +86,6 @@ static int create_device( if (!v) return log_oom(); - u_escaped = specifier_escape(u); - if (!u_escaped) - return log_oom(); - v_escaped = specifier_escape(v); - if (!v_escaped) - return log_oom(); - r = unit_name_from_path(u, ".device", &d); if (r < 0) return log_error_errno(r, "Failed to generate unit name: %m"); @@ -102,49 +93,33 @@ static int create_device( if (r < 0) return log_error_errno(r, "Failed to generate unit name: %m"); - options_escaped = specifier_escape(strempty(options)); - if (!options_escaped) - return log_oom(); - - hash_escaped = specifier_escape(hash); - if (!hash_escaped) - return log_oom(); - r = generator_open_unit_file(arg_dest, NULL, service, &f); if (r < 0) return r; + r = generator_write_veritysetup_unit_section(f, "/proc/cmdline"); + if (r < 0) + return r; + fprintf(f, - "[Unit]\n" - "Description=Verity Protection Setup for %%I\n" - "Documentation=man:systemd-veritysetup-generator(8) man:systemd-veritysetup@.service(8)\n" - "SourcePath=/proc/cmdline\n" - "DefaultDependencies=no\n" "Conflicts=umount.target\n" "BindsTo=%s %s\n" - "IgnoreOnIsolate=true\n" - "After=veritysetup-pre.target systemd-udevd-kernel.socket %s %s\n" - "Before=veritysetup.target umount.target\n" - "\n[Service]\n" - "Type=oneshot\n" - "RemainAfterExit=yes\n" - "ExecStart=" ROOTLIBEXECDIR "/systemd-veritysetup attach '%s' '%s' '%s' '%s' '%s'\n" - "ExecStop=" ROOTLIBEXECDIR "/systemd-veritysetup detach '%s' \n", + "After=%s %s\n" + "Before=veritysetup.target umount.target\n", d, e, - d, e, - name, u_escaped, v_escaped, hash_escaped, options_escaped, - name); + d, e); + + r = generator_write_veritysetup_service_section(f, name, u, v, roothash, options); + if (r < 0) + return r; r = fflush_and_check(f); if (r < 0) return log_error_errno(r, "Failed to write file unit %s: %m", service); - to = strjoina(arg_dest, "/veritysetup.target.requires/", service); - from = strjoina("../", service); - - (void) mkdir_parents(to, 0755); - if (symlink(from, to) < 0) - return log_error_errno(errno, "Failed to create symlink %s: %m", to); + r = generator_add_symlink(arg_dest, "veritysetup.target", "requires", service); + if (r < 0) + return r; return 0; } From 7a2f3194ff0e2967a9a62d6fc8790e361407eec3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 1 Jun 2023 18:28:08 +0200 Subject: [PATCH 04/12] units: set DefaultDependencies=no for veritysetup slice This mimics what we already have for cryptsetup services: the slice they are placed in (they have their own slice since that's what we do by default for instantiated services) shouldn't conflict with shutdown.target, so that veritysetup services can stay around until the very end (which is what we want for the root and usr verity volumes). It's literally just a copy of the same unit we already have for cryptsetup, just with an updated description string. --- units/meson.build | 1 + "units/system-systemd\\x2dcryptsetup.slice" | 2 +- "units/system-systemd\\x2dveritysetup.slice" | 13 +++++++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 "units/system-systemd\\x2dveritysetup.slice" diff --git a/units/meson.build b/units/meson.build index e6eb300661..fa76946ddb 100644 --- a/units/meson.build +++ b/units/meson.build @@ -96,6 +96,7 @@ units = [ ['sysinit.target', ''], ['syslog.socket', ''], ['system-systemd\\x2dcryptsetup.slice', 'HAVE_LIBCRYPTSETUP'], + ['system-systemd\\x2dveritysetup.slice','HAVE_LIBCRYPTSETUP'], ['system-update.target', ''], ['system-update-pre.target', ''], ['system-update-cleanup.service', ''], diff --git "a/units/system-systemd\\x2dcryptsetup.slice" "b/units/system-systemd\\x2dcryptsetup.slice" index 98206a9d06..e81925d587 100644 --- "a/units/system-systemd\\x2dcryptsetup.slice" +++ "b/units/system-systemd\\x2dcryptsetup.slice" @@ -8,6 +8,6 @@ # (at your option) any later version. [Unit] -Description=Cryptsetup Units Slice +Description=Encrypted Volume Units Service Slice Documentation=man:systemd-cryptsetup@.service(8) DefaultDependencies=no diff --git "a/units/system-systemd\\x2dveritysetup.slice" "b/units/system-systemd\\x2dveritysetup.slice" new file mode 100644 index 0000000000..8ccaac90e6 --- /dev/null +++ "b/units/system-systemd\\x2dveritysetup.slice" @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# This file is part of systemd. +# +# systemd is free software; you can redistribute it and/or modify it +# under the terms of the GNU Lesser General Public License as published by +# the Free Software Foundation; either version 2.1 of the License, or +# (at your option) any later version. + +[Unit] +Description=Verity Protection Service Slice +Documentation=man:systemd-cryptsetup@.service(8) +DefaultDependencies=no From d120ce478dc0043c89899799b5c1aaf62901bea9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 1 Jun 2023 18:31:17 +0200 Subject: [PATCH 05/12] units: don't stop blockdev@.target unit at shutdown We want that cryptsetup/veritysetup devices can stick around until the very end, as well as the users of them which might depend on blockdev@.target for the devices. Hence leave the targets around till the very end. Note that their runtime is managed via StopWhenUnneeded= anyway, hence unless their are volumes that actually survive still the very end they target units will still be stopped. --- units/blockdev@.target | 1 + 1 file changed, 1 insertion(+) diff --git a/units/blockdev@.target b/units/blockdev@.target index ddc85bf890..1c86506351 100644 --- a/units/blockdev@.target +++ b/units/blockdev@.target @@ -10,4 +10,5 @@ [Unit] Description=Block Device Preparation for %f Documentation=man:systemd.special(7) +DefaultDependencies=no StopWhenUnneeded=yes From 8ce02b87cece09797c1030c778db4180e1e2ce2e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 1 Jun 2023 14:40:38 +0200 Subject: [PATCH 06/12] cryptsetup-generator: imply x-initrd.attach for "usr" and "root" volumes Let's imply "x-initrd.attach" for "usr" and "root" volumes, so that we do not attempt to umount them anymore during shutdown. The names of these volumes have been mandated by the Discoverable Partition Spec: https://uapi-group.org/specifications/specs/discoverable_partitions_specification/#suggested-mode-of-operation Hence it appears reasonably safe to special case these volume names. Note that a similar logic is implemented in fstab-generator and in fact PID 1 to treat the root mount and /usr/ mount specially too, to avoid trying to umount it at shutdown. (This is what fstab_is_extrinsic() checks). This should ensure that if /usr/ or / is for some reason a LUKS medium we won't try to detach it during runtime, which likely fails, since we run off it. Note this also moves an ordering dep towards umount.target under the x-initrd.attach check, becasue that's where the crucial conflicts dep is placed too. --- src/cryptsetup/cryptsetup-generator.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c index 083878e42a..e00d4d4126 100644 --- a/src/cryptsetup/cryptsetup-generator.c +++ b/src/cryptsetup/cryptsetup-generator.c @@ -282,6 +282,18 @@ static int print_dependencies(FILE *f, const char* device_path, const char* time return 0; } +static bool attach_in_initrd(const char *name, const char *options) { + assert(name); + + /* Imply x-initrd.attach in case the volume name is among those defined in the Discoverable Partition + * Specification for partitions that we require to be mounted during the initrd → host transition, + * i.e. for the root fs itself, and /usr/. This mirrors similar behaviour in + * systemd-fstab-generator. */ + + return fstab_test_option(options, "x-initrd.attach\0") || + STR_IN_SET(name, "root", "usr"); +} + static int create_disk( const char *name, const char *device, @@ -297,7 +309,7 @@ static int create_disk( *tmp_fstype = NULL, *filtered_header = NULL, *headerdev_mount = NULL; _cleanup_fclose_ FILE *f = NULL; const char *dmname; - bool noauto, nofail, swap, netdev, attach_in_initrd; + bool noauto, nofail, swap, netdev; int r, detached_header, keyfile_can_timeout, tmp; assert(name); @@ -307,7 +319,6 @@ static int create_disk( nofail = fstab_test_yes_no_option(options, "nofail\0" "fail\0"); swap = fstab_test_option(options, "swap\0"); netdev = fstab_test_option(options, "_netdev\0"); - attach_in_initrd = fstab_test_option(options, "x-initrd.attach\0"); keyfile_can_timeout = fstab_filter_options(options, "keyfile-timeout\0", @@ -374,8 +385,10 @@ static int create_disk( fprintf(f, "After=remote-fs-pre.target\n"); /* If initrd takes care of attaching the disk then it should also detach it during shutdown. */ - if (!attach_in_initrd) - fprintf(f, "Conflicts=umount.target\n"); + if (!attach_in_initrd(name, options)) + fprintf(f, + "Conflicts=umount.target\n" + "Before=umount.target\n"); if (keydev) { _cleanup_free_ char *unit = NULL, *umount_unit = NULL; @@ -490,8 +503,7 @@ static int create_disk( if (path_startswith(u, "/dev/")) fprintf(f, "BindsTo=%s\n" - "After=%s\n" - "Before=umount.target\n", + "After=%s\n", d, d); else /* For loopback devices, add systemd-tmpfiles-setup-dev.service From bf1484c70a24cf04c145a9509c8124ffd7fb0879 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 1 Jun 2023 14:50:15 +0200 Subject: [PATCH 07/12] veritysetup-generator: imply x-initrd.attach for "usr" and "root" volumes Similar to the previous commit, just vor Verity rather than LUKS. --- src/veritysetup/veritysetup-generator.c | 32 ++++++++++++++++--------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/src/veritysetup/veritysetup-generator.c b/src/veritysetup/veritysetup-generator.c index 9ce95608c8..c1a9d51af3 100644 --- a/src/veritysetup/veritysetup-generator.c +++ b/src/veritysetup/veritysetup-generator.c @@ -102,10 +102,9 @@ static int create_device( return r; fprintf(f, - "Conflicts=umount.target\n" + "Before=veritysetup.target\n" "BindsTo=%s %s\n" - "After=%s %s\n" - "Before=veritysetup.target umount.target\n", + "After=%s %s\n", d, e, d, e); @@ -288,6 +287,18 @@ static int determine_devices(void) { return determine_device("usr", arg_usr_hash, &arg_usr_data_what, &arg_usr_hash_what); } +static bool attach_in_initrd(const char *name, const char *options) { + assert(name); + + /* Imply x-initrd.attach in case the volume name is among those defined in the Discoverable Partition + * Specification for partitions that we require to be mounted during the initrd → host transition, + * i.e. for the root fs itself, and /usr/. This mirrors similar behaviour in + * systemd-fstab-generator. */ + + return fstab_test_option(options, "x-initrd.attach\0") || + STR_IN_SET(name, "root", "usr"); +} + static int create_disk( const char *name, const char *data_device, @@ -300,7 +311,7 @@ static int create_disk( *du_escaped = NULL, *hu_escaped = NULL, *name_escaped = NULL; _cleanup_fclose_ FILE *f = NULL; const char *dmname; - bool noauto, nofail, netdev, attach_in_initrd; + bool noauto, nofail, netdev; int r; assert(name); @@ -311,7 +322,6 @@ static int create_disk( noauto = fstab_test_yes_no_option(options, "noauto\0" "auto\0"); nofail = fstab_test_yes_no_option(options, "nofail\0" "fail\0"); netdev = fstab_test_option(options, "_netdev\0"); - attach_in_initrd = fstab_test_option(options, "x-initrd.attach\0"); name_escaped = specifier_escape(name); if (!name_escaped) @@ -361,8 +371,10 @@ static int create_disk( fprintf(f, "After=remote-fs-pre.target\n"); /* If initrd takes care of attaching the disk then it should also detach it during shutdown. */ - if (!attach_in_initrd) - fprintf(f, "Conflicts=umount.target\n"); + if (!attach_in_initrd(name, options)) + fprintf(f, + "Conflicts=umount.target\n" + "Before=umount.target\n"); if (!nofail) fprintf(f, @@ -372,8 +384,7 @@ static int create_disk( if (path_startswith(du, "/dev/")) fprintf(f, "BindsTo=%s\n" - "After=%s\n" - "Before=umount.target\n", + "After=%s\n", dd, dd); else /* For loopback devices, add systemd-tmpfiles-setup-dev.service @@ -388,8 +399,7 @@ static int create_disk( if (path_startswith(hu, "/dev/")) fprintf(f, "BindsTo=%s\n" - "After=%s\n" - "Before=umount.target\n", + "After=%s\n", hd, hd); else /* For loopback devices, add systemd-tmpfiles-setup-dev.service From 5dd02147079cc88a1c264d0a3e0fc2aa19eafae4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 1 Jun 2023 14:55:26 +0200 Subject: [PATCH 08/12] veritysetup-generator: only generate one set of deps on systemd-tmpfiles-setup-dev.service If both the data and the hash device are a regular file we might create two sets of deps on s-t-s-d.s, which is of course redundant. Shorten the code to only generate this once. No change in behaviour. --- src/veritysetup/veritysetup-generator.c | 31 +++++++++++-------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/veritysetup/veritysetup-generator.c b/src/veritysetup/veritysetup-generator.c index c1a9d51af3..68ef5d458f 100644 --- a/src/veritysetup/veritysetup-generator.c +++ b/src/veritysetup/veritysetup-generator.c @@ -311,7 +311,7 @@ static int create_disk( *du_escaped = NULL, *hu_escaped = NULL, *name_escaped = NULL; _cleanup_fclose_ FILE *f = NULL; const char *dmname; - bool noauto, nofail, netdev; + bool noauto, nofail, netdev, need_loop = false; int r; assert(name); @@ -386,30 +386,27 @@ static int create_disk( "BindsTo=%s\n" "After=%s\n", dd, dd); - else - /* For loopback devices, add systemd-tmpfiles-setup-dev.service - dependency to ensure that loopback support is available in - the kernel (/dev/loop-control needs to exist) */ - fprintf(f, - "RequiresMountsFor=%s\n" - "Requires=systemd-tmpfiles-setup-dev.service\n" - "After=systemd-tmpfiles-setup-dev.service\n", - du_escaped); + else { + fprintf(f, "RequiresMountsFor=%s\n", du_escaped); + need_loop = true; + } if (path_startswith(hu, "/dev/")) fprintf(f, "BindsTo=%s\n" "After=%s\n", hd, hd); - else - /* For loopback devices, add systemd-tmpfiles-setup-dev.service - dependency to ensure that loopback support is available in - the kernel (/dev/loop-control needs to exist) */ + else { + fprintf(f, "RequiresMountsFor=%s\n", hu_escaped); + need_loop = true; + } + + if (need_loop) + /* For loopback devices, add systemd-tmpfiles-setup-dev.service dependency to ensure that + * loopback support is available in the kernel (/dev/loop-control needs to exist) */ fprintf(f, - "RequiresMountsFor=%s\n" "Requires=systemd-tmpfiles-setup-dev.service\n" - "After=systemd-tmpfiles-setup-dev.service\n", - hu_escaped); + "After=systemd-tmpfiles-setup-dev.service\n"); r = generator_write_veritysetup_service_section(f, name, du_escaped, hu_escaped, roothash, options); if (r < 0) From ce49a479d296648033611a4ce5fde929efb49354 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 1 Jun 2023 15:00:27 +0200 Subject: [PATCH 09/12] {crypt|verity}setup: replace dep on systemd-tmpfiles-setup-dev.service by modprobe@loop.service Both should have the same effect: the /dev/loop-control devices should become available. systemd-tmpfiles-setup-dev.service creates the device node "dry" based on modalias data, while modprobe@loop.service creates it fully, because the module backing it is loaded properly. This should shorten the deps chain a bit, simplify things and allows us to focus on the stuff we actually need (i.e. the loopback infra) instead of all entrypoints anyone might possibly need (i.e. the device nodes) --- src/cryptsetup/cryptsetup-generator.c | 10 +++++----- src/veritysetup/veritysetup-generator.c | 9 +++++---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c index e00d4d4126..a22d932342 100644 --- a/src/cryptsetup/cryptsetup-generator.c +++ b/src/cryptsetup/cryptsetup-generator.c @@ -506,13 +506,13 @@ static int create_disk( "After=%s\n", d, d); else - /* For loopback devices, add systemd-tmpfiles-setup-dev.service - dependency to ensure that loopback support is available in - the kernel (/dev/loop-control needs to exist) */ + /* For loopback devices make sure to explicitly load loop.ko, as this code might run very + * early where device nodes created via systemd-tmpfiles-setup-dev.service might not be + * around yet. Hence let's sync on the module itself. */ fprintf(f, "RequiresMountsFor=%s\n" - "Requires=systemd-tmpfiles-setup-dev.service\n" - "After=systemd-tmpfiles-setup-dev.service\n", + "Wants=modprobe@loop.service\n" + "After=modprobe@loop.service\n", u_escaped); r = generator_write_timeouts(arg_dest, device, name, options, &filtered); diff --git a/src/veritysetup/veritysetup-generator.c b/src/veritysetup/veritysetup-generator.c index 68ef5d458f..93aa265ba1 100644 --- a/src/veritysetup/veritysetup-generator.c +++ b/src/veritysetup/veritysetup-generator.c @@ -402,11 +402,12 @@ static int create_disk( } if (need_loop) - /* For loopback devices, add systemd-tmpfiles-setup-dev.service dependency to ensure that - * loopback support is available in the kernel (/dev/loop-control needs to exist) */ + /* For loopback devices make sure to explicitly load loop.ko, as this code might run very + * early where device nodes created via systemd-tmpfiles-setup-dev.service might not be + * around yet. Hence let's sync on the module itself. */ fprintf(f, - "Requires=systemd-tmpfiles-setup-dev.service\n" - "After=systemd-tmpfiles-setup-dev.service\n"); + "Wants=modprobe@loop.service\n" + "After=modprobe@loop.service\n"); r = generator_write_veritysetup_service_section(f, name, du_escaped, hu_escaped, roothash, options); if (r < 0) From ffe0da297d7ec4723b583ed19f31a2d605cb9142 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 1 Jun 2023 15:31:25 +0200 Subject: [PATCH 10/12] {crypt|verity}setup: mention volume name in some error messages --- src/cryptsetup/cryptsetup.c | 4 ++-- src/veritysetup/veritysetup.c | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cryptsetup/cryptsetup.c b/src/cryptsetup/cryptsetup.c index ad86801844..d58defcc42 100644 --- a/src/cryptsetup/cryptsetup.c +++ b/src/cryptsetup/cryptsetup.c @@ -2324,13 +2324,13 @@ static int run(int argc, char *argv[]) { return 0; } if (r < 0) - return log_error_errno(r, "crypt_init_by_name() failed: %m"); + return log_error_errno(r, "crypt_init_by_name() for volume '%s' failed: %m", volume); cryptsetup_enable_logging(cd); r = crypt_deactivate(cd, volume); if (r < 0) - return log_error_errno(r, "Failed to deactivate: %m"); + return log_error_errno(r, "Failed to deactivate '%s': %m", volume); } else return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Unknown verb %s.", verb); diff --git a/src/veritysetup/veritysetup.c b/src/veritysetup/veritysetup.c index be95c1f206..d73c2d39fb 100644 --- a/src/veritysetup/veritysetup.c +++ b/src/veritysetup/veritysetup.c @@ -398,7 +398,7 @@ static int run(int argc, char *argv[]) { } else r = crypt_activate_by_volume_key(cd, volume, m, l, arg_activate_flags); if (r < 0) - return log_error_errno(r, "Failed to set up verity device: %m"); + return log_error_errno(r, "Failed to set up verity device '%s': %m", volume); } else if (streq(verb, "detach")) { const char *volume; @@ -410,17 +410,17 @@ static int run(int argc, char *argv[]) { r = crypt_init_by_name(&cd, volume); if (r == -ENODEV) { - log_info("Volume %s already inactive.", volume); + log_info("Volume %s 'already' inactive.", volume); return 0; } if (r < 0) - return log_error_errno(r, "crypt_init_by_name() failed: %m"); + return log_error_errno(r, "crypt_init_by_name() for volume '%s' failed: %m", volume); cryptsetup_enable_logging(cd); r = crypt_deactivate(cd, volume); if (r < 0) - return log_error_errno(r, "Failed to deactivate: %m"); + return log_error_errno(r, "Failed to deactivate volume '%s': %m", volume); } else return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Unknown verb %s.", verb); From deb60ef92e47e35d8124ae8e03deaa914701b9b4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 1 Jun 2023 18:54:33 +0200 Subject: [PATCH 11/12] veritysetup: minor renaming of functions create_device() and create_disk() so far did very similar things, but the name didn't give a hint what the difference was. Hence let's rename them to create_special_device() and create_veritytab_device() to make this more understandabe, as one creates /proc/cmdline specified roothash=/usrhash= devices, and the other one devices for items listed in /etc/veritytab. No code changes besides renaming. --- src/veritysetup/veritysetup-generator.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/veritysetup/veritysetup-generator.c b/src/veritysetup/veritysetup-generator.c index 93aa265ba1..ca2e6401ac 100644 --- a/src/veritysetup/veritysetup-generator.c +++ b/src/veritysetup/veritysetup-generator.c @@ -47,7 +47,7 @@ STATIC_DESTRUCTOR_REGISTER(arg_usr_data_what, freep); STATIC_DESTRUCTOR_REGISTER(arg_usr_hash_what, freep); STATIC_DESTRUCTOR_REGISTER(arg_usr_options, freep); -static int create_device( +static int create_special_device( const char *name, const char *service, const char *roothash, @@ -59,6 +59,8 @@ static int create_device( _cleanup_fclose_ FILE *f = NULL; int r; + /* Creates a systemd-veritysetup@.service instance for the special kernel cmdline specified root + usr devices. */ + assert(name); assert(service); @@ -124,11 +126,11 @@ static int create_device( } static int create_root_device(void) { - return create_device("root", SYSTEMD_VERITYSETUP_SERVICE_ROOT, arg_root_hash, arg_root_data_what, arg_root_hash_what, arg_root_options); + return create_special_device("root", SYSTEMD_VERITYSETUP_SERVICE_ROOT, arg_root_hash, arg_root_data_what, arg_root_hash_what, arg_root_options); } static int create_usr_device(void) { - return create_device("usr", SYSTEMD_VERITYSETUP_SERVICE_USR, arg_usr_hash, arg_usr_data_what, arg_usr_hash_what, arg_usr_options); + return create_special_device("usr", SYSTEMD_VERITYSETUP_SERVICE_USR, arg_usr_hash, arg_usr_data_what, arg_usr_hash_what, arg_usr_options); } static int parse_proc_cmdline_item(const char *key, const char *value, void *data) { @@ -299,7 +301,7 @@ static bool attach_in_initrd(const char *name, const char *options) { STR_IN_SET(name, "root", "usr"); } -static int create_disk( +static int create_veritytab_device( const char *name, const char *data_device, const char *hash_device, @@ -314,6 +316,8 @@ static int create_disk( bool noauto, nofail, netdev, need_loop = false; int r; + /* Creates a systemd-veritysetup@.service instance for volumes specified in /etc/veritytab. */ + assert(name); assert(data_device); assert(hash_device); @@ -475,7 +479,8 @@ static int add_veritytab_devices(void) { if (!hash_uuid) hash_uuid = path_startswith(hash_device, "/dev/disk/by-uuid/"); - r = create_disk(name, + r = create_veritytab_device( + name, data_device, hash_device, roothash, From 45e3406e23eb003b56eaf19a0979dec48aa97651 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 1 Jun 2023 19:15:10 +0200 Subject: [PATCH 12/12] veritysetup: remove double escaping of data device + hash device spec generator_write_veritysetup_service_section() already escapes the parameters internally, doing so in the caller means double escaping, which is a bug. Fix it. --- src/veritysetup/veritysetup-generator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/veritysetup/veritysetup-generator.c b/src/veritysetup/veritysetup-generator.c index ca2e6401ac..303e0ced6d 100644 --- a/src/veritysetup/veritysetup-generator.c +++ b/src/veritysetup/veritysetup-generator.c @@ -413,7 +413,7 @@ static int create_veritytab_device( "Wants=modprobe@loop.service\n" "After=modprobe@loop.service\n"); - r = generator_write_veritysetup_service_section(f, name, du_escaped, hu_escaped, roothash, options); + r = generator_write_veritysetup_service_section(f, name, du, hu, roothash, options); if (r < 0) return r;