From a7e885587949b6793ccf389505f3c436315fa653 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 19 Dec 2019 17:38:55 +0100 Subject: [PATCH] units: introduce blockdev@.target for properly ordering mounts/swaps against cryptsetup Let's hook it into both cryptsetup-generator and gpt-auto-generator with a shared implementation in generator.c Fixes: #8472 --- src/cryptsetup/cryptsetup-generator.c | 58 +++--------- src/fstab-generator/fstab-generator.c | 26 ++++-- src/gpt-auto-generator/gpt-auto-generator.c | 63 ++++++------- src/shared/generator.c | 97 +++++++++++++++++++++ src/shared/generator.h | 15 ++++ units/blockdev@.target | 13 +++ units/meson.build | 1 + 7 files changed, 188 insertions(+), 85 deletions(-) create mode 100644 units/blockdev@.target diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c index 02b2cae5c00..1deab765fbb 100644 --- a/src/cryptsetup/cryptsetup-generator.c +++ b/src/cryptsetup/cryptsetup-generator.c @@ -230,8 +230,8 @@ static int create_disk( const char *options) { _cleanup_free_ char *n = NULL, *d = NULL, *u = NULL, *e = NULL, - *keydev_mount = NULL, *keyfile_timeout_value = NULL, *password_escaped = NULL, - *filtered = NULL, *u_escaped = NULL, *filtered_escaped = NULL, *name_escaped = NULL, *header_path = NULL; + *keydev_mount = NULL, *keyfile_timeout_value = NULL, + *filtered = NULL, *u_escaped = NULL, *name_escaped = NULL, *header_path = NULL, *password_buffer = NULL; _cleanup_fclose_ FILE *f = NULL; const char *dmname; bool noauto, nofail, tmp, swap, netdev, attach_in_initrd; @@ -292,15 +292,9 @@ static int create_disk( if (r < 0) return r; - fprintf(f, - "[Unit]\n" - "Description=Cryptography Setup for %%I\n" - "Documentation=man:crypttab(5) man:systemd-cryptsetup-generator(8) man:systemd-cryptsetup@.service(8)\n" - "SourcePath=%s\n" - "DefaultDependencies=no\n" - "IgnoreOnIsolate=true\n" - "After=cryptsetup-pre.target\n", - arg_crypttab); + r = generator_write_cryptsetup_unit_section(f, arg_crypttab); + if (r < 0) + return r; if (netdev) fprintf(f, "After=remote-fs-pre.target\n"); @@ -309,24 +303,18 @@ static int create_disk( if (!attach_in_initrd) fprintf(f, "Conflicts=umount.target\n"); - if (password) { - password_escaped = specifier_escape(password); - if (!password_escaped) - return log_oom(); - } - if (keydev) { - _cleanup_free_ char *unit = NULL, *p = NULL; + _cleanup_free_ char *unit = NULL; r = generate_keydev_mount(name, keydev, keyfile_timeout_value, keyfile_can_timeout > 0, &unit, &keydev_mount); if (r < 0) return log_error_errno(r, "Failed to generate keydev mount unit: %m"); - p = path_join(keydev_mount, password_escaped); - if (!p) + password_buffer = path_join(keydev_mount, password); + if (!password_buffer) return log_oom(); - free_and_replace(password_escaped, p); + password = password_buffer; fprintf(f, "After=%s\n", unit); if (keyfile_can_timeout > 0) @@ -353,17 +341,13 @@ static int create_disk( return r; } - if (path_startswith(u, "/dev/")) { + if (path_startswith(u, "/dev/")) fprintf(f, "BindsTo=%s\n" "After=%s\n" "Before=umount.target\n", d, d); - - if (swap) - fputs("Before=dev-mapper-%i.swap\n", - f); - } else + 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) */ @@ -377,23 +361,9 @@ static int create_disk( if (r < 0) log_warning_errno(r, "Failed to write device timeout drop-in: %m"); - if (filtered) { - filtered_escaped = specifier_escape(filtered); - if (!filtered_escaped) - return log_oom(); - } - - fprintf(f, - "\n[Service]\n" - "Type=oneshot\n" - "RemainAfterExit=yes\n" - "TimeoutSec=0\n" /* the binary handles timeouts anyway */ - "KeyringMode=shared\n" /* make sure we can share cached keys among instances */ - "OOMScoreAdjust=500\n" /* unlocking can allocate a lot of memory if Argon2 is used */ - "ExecStart=" SYSTEMD_CRYPTSETUP_PATH " attach '%s' '%s' '%s' '%s'\n" - "ExecStop=" SYSTEMD_CRYPTSETUP_PATH " detach '%s'\n", - name_escaped, u_escaped, strempty(password_escaped), strempty(filtered_escaped), - name_escaped); + r = generator_write_cryptsetup_service_section(f, name, u, password, filtered); + if (r < 0) + return r; if (tmp) fprintf(f, diff --git a/src/fstab-generator/fstab-generator.c b/src/fstab-generator/fstab-generator.c index a75a74bb3c3..5a0a871759e 100644 --- a/src/fstab-generator/fstab-generator.c +++ b/src/fstab-generator/fstab-generator.c @@ -118,11 +118,18 @@ static int add_swap( fprintf(f, "[Unit]\n" - "SourcePath=%s\n" - "Documentation=man:fstab(5) man:systemd-fstab-generator(8)\n\n" - "[Swap]\n", + "Documentation=man:fstab(5) man:systemd-fstab-generator(8)\n" + "SourcePath=%s\n", fstab_path()); + r = generator_write_blockdev_dependency(f, what); + if (r < 0) + return r; + + fprintf(f, + "\n" + "[Swap]\n"); + r = write_what(f, what); if (r < 0) return r; @@ -374,8 +381,8 @@ static int add_mount( fprintf(f, "[Unit]\n" - "SourcePath=%s\n" - "Documentation=man:fstab(5) man:systemd-fstab-generator(8)\n", + "Documentation=man:fstab(5) man:systemd-fstab-generator(8)\n" + "SourcePath=%s\n", source); /* All mounts under /sysroot need to happen later, at initrd-fs.target time. IOW, it's not @@ -422,7 +429,14 @@ static int add_mount( return r; } - fprintf(f, "\n[Mount]\n"); + r = generator_write_blockdev_dependency(f, what); + if (r < 0) + return r; + + fprintf(f, + "\n" + "[Mount]\n"); + if (original_where) fprintf(f, "# Canonicalized from %s\n", original_where); diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c index e03cdbd5c02..2067eeaf89a 100644 --- a/src/gpt-auto-generator/gpt-auto-generator.c +++ b/src/gpt-auto-generator/gpt-auto-generator.c @@ -105,9 +105,8 @@ static int open_parent_block_device(dev_t devnum, int *ret_fd) { } static int add_cryptsetup(const char *id, const char *what, bool rw, bool require, char **device) { - _cleanup_free_ char *e = NULL, *n = NULL, *d = NULL, *id_escaped = NULL, *what_escaped = NULL; + _cleanup_free_ char *e = NULL, *n = NULL, *d = NULL; _cleanup_fclose_ FILE *f = NULL; - const char *p; int r; assert(id); @@ -125,44 +124,28 @@ static int add_cryptsetup(const char *id, const char *what, bool rw, bool requir if (r < 0) return log_error_errno(r, "Failed to generate unit name: %m"); - id_escaped = specifier_escape(id); - if (!id_escaped) - return log_oom(); + r = generator_open_unit_file(arg_dest, NULL, n, &f); + if (r < 0) + return r; - what_escaped = specifier_escape(what); - if (!what_escaped) - return log_oom(); - - p = prefix_roota(arg_dest, n); - f = fopen(p, "wxe"); - if (!f) - return log_error_errno(errno, "Failed to create unit file %s: %m", p); + r = generator_write_cryptsetup_unit_section(f, NULL); + if (r < 0) + return r; fprintf(f, - "# Automatically generated by systemd-gpt-auto-generator\n\n" - "[Unit]\n" - "Description=Cryptography Setup for %%I\n" - "Documentation=man:systemd-gpt-auto-generator(8) man:systemd-cryptsetup@.service(8)\n" - "DefaultDependencies=no\n" - "Conflicts=umount.target\n" - "BindsTo=dev-mapper-%%i.device %s\n" "Before=umount.target cryptsetup.target\n" - "After=%s\n" - "IgnoreOnIsolate=true\n" - "[Service]\n" - "Type=oneshot\n" - "RemainAfterExit=yes\n" - "TimeoutSec=0\n" /* the binary handles timeouts anyway */ - "KeyringMode=shared\n" /* make sure we can share cached keys among instances */ - "ExecStart=" SYSTEMD_CRYPTSETUP_PATH " attach '%s' '%s' '' '%s'\n" - "ExecStop=" SYSTEMD_CRYPTSETUP_PATH " detach '%s'\n", - d, d, - id_escaped, what_escaped, rw ? "" : "read-only", - id_escaped); + "Conflicts=umount.target\n" + "BindsTo=%s\n" + "After=%s\n", + d, d); + + r = generator_write_cryptsetup_service_section(f, id, what, NULL, rw ? NULL : "read-only"); + if (r < 0) + return r; r = fflush_and_check(f); if (r < 0) - return log_error_errno(r, "Failed to write file %s: %m", p); + return log_error_errno(r, "Failed to write file %s: %m", n); r = generator_add_symlink(arg_dest, d, "wants", n); if (r < 0) @@ -227,7 +210,6 @@ static int add_mount( log_debug("Adding %s: %s fstype=%s", where, what, fstype ?: "(any)"); if (streq_ptr(fstype, "crypto_LUKS")) { - r = add_cryptsetup(id, what, rw, true, &crypto_what); if (r < 0) return r; @@ -262,6 +244,10 @@ static int add_mount( if (r < 0) return r; + r = generator_write_blockdev_dependency(f, what); + if (r < 0) + return r; + fprintf(f, "\n" "[Mount]\n" @@ -370,7 +356,14 @@ static int add_swap(const char *path) { "# Automatically generated by systemd-gpt-auto-generator\n\n" "[Unit]\n" "Description=Swap Partition\n" - "Documentation=man:systemd-gpt-auto-generator(8)\n\n" + "Documentation=man:systemd-gpt-auto-generator(8)\n"); + + r = generator_write_blockdev_dependency(f, path); + if (r < 0) + return r; + + fprintf(f, + "\n" "[Swap]\n" "What=%s\n", path); diff --git a/src/shared/generator.c b/src/shared/generator.c index 06e1ab80312..48667c93a21 100644 --- a/src/shared/generator.c +++ b/src/shared/generator.c @@ -513,6 +513,103 @@ int generator_enable_remount_fs_service(const char *dir) { SYSTEM_DATA_UNIT_PATH "/" SPECIAL_REMOUNT_FS_SERVICE); } +int generator_write_blockdev_dependency( + FILE *f, + const char *what) { + + _cleanup_free_ char *escaped = NULL; + int r; + + assert(f); + assert(what); + + if (!path_startswith(what, "/dev/")) + return 0; + + r = unit_name_path_escape(what, &escaped); + if (r < 0) + return log_error_errno(r, "Failed to escape device node path %s: %m", what); + + fprintf(f, + "After=blockdev@%s.target\n", + escaped); + + return 0; +} + +int generator_write_cryptsetup_unit_section( + FILE *f, + const char *source) { + + assert(f); + + fprintf(f, + "[Unit]\n" + "Description=Cryptography Setup for %%I\n" + "Documentation=man:crypttab(5) man:systemd-cryptsetup-generator(8) man:systemd-cryptsetup@.service(8)\n"); + + if (source) + fprintf(f, "SourcePath=%s\n", source); + + fprintf(f, + "DefaultDependencies=no\n" + "IgnoreOnIsolate=true\n" + "After=cryptsetup-pre.target\n" + "Before=blockdev@dev-mapper-%%i.target\n" + "Wants=blockdev@dev-mapper-%%i.target\n"); + + return 0; +} + +int generator_write_cryptsetup_service_section( + FILE *f, + const char *name, + const char *what, + const char *password, + const char *options) { + + _cleanup_free_ char *name_escaped = NULL, *what_escaped = NULL, *password_escaped = NULL, *options_escaped = NULL; + + assert(f); + assert(name); + assert(what); + + name_escaped = specifier_escape(name); + if (!name_escaped) + return log_oom(); + + what_escaped = specifier_escape(what); + if (!what_escaped) + return log_oom(); + + if (password) { + password_escaped = specifier_escape(password); + if (!password_escaped) + return log_oom(); + } + + if (options) { + options_escaped = specifier_escape(options); + if (!options_escaped) + return log_oom(); + } + + fprintf(f, + "\n" + "[Service]\n" + "Type=oneshot\n" + "RemainAfterExit=yes\n" + "TimeoutSec=0\n" /* The binary handles timeouts on its own */ + "KeyringMode=shared\n" /* Make sure we can share cached keys among instances */ + "OOMScoreAdjust=500\n" /* Unlocking can allocate a lot of memory if Argon2 is used */ + "ExecStart=" SYSTEMD_CRYPTSETUP_PATH " attach '%s' '%s' '%s' '%s'\n" + "ExecStop=" SYSTEMD_CRYPTSETUP_PATH " detach '%s'\n", + name_escaped, what_escaped, strempty(password_escaped), strempty(options_escaped), + name_escaped); + + return 0; +} + void log_setup_generator(void) { log_set_prohibit_ipc(true); log_setup_service(); diff --git a/src/shared/generator.h b/src/shared/generator.h index fa002a91143..579e291fe8a 100644 --- a/src/shared/generator.h +++ b/src/shared/generator.h @@ -27,6 +27,21 @@ int generator_write_timeouts( const char *opts, char **filtered); +int generator_write_blockdev_dependency( + FILE *f, + const char *what); + +int generator_write_cryptsetup_unit_section( + FILE *f, + const char *source); + +int generator_write_cryptsetup_service_section( + FILE *f, + const char *name, + const char *what, + const char *password, + const char *options); + int generator_write_device_deps( const char *dir, const char *what, diff --git a/units/blockdev@.target b/units/blockdev@.target new file mode 100644 index 00000000000..22a9a5bb2bf --- /dev/null +++ b/units/blockdev@.target @@ -0,0 +1,13 @@ +# SPDX-License-Identifier: LGPL-2.1+ +# +# 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=Block Device Preparation for %f +Documentation=man:systemd.special(7) +StopWhenUnneeded=yes diff --git a/units/meson.build b/units/meson.build index f7653c920c9..27742044c5f 100644 --- a/units/meson.build +++ b/units/meson.build @@ -2,6 +2,7 @@ units = [ ['basic.target', ''], + ['blockdev@.target', ''], ['bluetooth.target', ''], ['boot-complete.target', ''], ['cryptsetup-pre.target', 'HAVE_LIBCRYPTSETUP'],