Skip to content

Commit

Permalink
cryptsetup: generate the unit to umount keydev filesystem
Browse files Browse the repository at this point in the history
Previously we would call umount from ExecStartPost= of
systemd-cryptsetup instance in order to get rid of the keydev
mount (i.e. filesystem containing keyfile). Let's generate unit to
handle umount. Making this symmetrical (both mount and umount of keydev
are handled by units) fixes the problem with lingering keydev mounts.

Motivation for the change is the issue where keydev mount would stay
around even if device was successfully unlocked and mount is no longer
needed. That could happen previously because when generator options are
not prefixed with "rd." we run generators twice (e.g. rd.luks.key=...).

In such case disk is unlocked in initramfs phase of boot (assuming the
initrd image contains the generator and is able to handle unlocking of
LUKS devices). After switchroot we however enqueue start job for
systemd-cryptsetup instance (because units are regenerated second time)
and that pulls in its dependencies into transaction. Later the main
systemd-cryptsetup unit not actually started since it is already active
and has RemainaAfterExit=yes. Nevertheless, dependencies get activated
and keydev mount is attached again. Because previously we called umount
from ExecStartPost= of systemd-cryptsetup instance the umount is not
called second time and keydev filesystem stays lingering.
  • Loading branch information
msekletar authored and keszybz committed Sep 11, 2020
1 parent 5501da1 commit 882f5f4
Showing 1 changed file with 55 additions and 6 deletions.
61 changes: 55 additions & 6 deletions src/cryptsetup/cryptsetup-generator.c
Expand Up @@ -181,6 +181,47 @@ static int generate_keydev_mount(
return 0;
}

static int generate_keydev_umount(const char *name,
const char *keydev_mount,
char **ret_umount_unit) {
_cleanup_fclose_ FILE *f = NULL;
_cleanup_free_ char *u = NULL, *name_escaped = NULL, *mount = NULL;
int r;

assert(name);
assert(ret_umount_unit);

name_escaped = cescape(name);
if (!name_escaped)
return -ENOMEM;

u = strjoin("keydev-", name_escaped, "-umount.service");
if (!u)
return -ENOMEM;

r = unit_name_from_path(keydev_mount, ".mount", &mount);
if (r < 0)
return r;

r = generator_open_unit_file(arg_dest, NULL, u, &f);
if (r < 0)
return r;

fprintf(f,
"[Unit]\n"
"DefaultDependencies=no\n"
"After=%s\n\n"
"[Service]\n"
"ExecStart=-" UMOUNT_PATH " %s\n\n", mount, keydev_mount);

r = fflush_and_check(f);
if (r < 0)
return r;

*ret_umount_unit = TAKE_PTR(u);
return 0;
}

static int print_dependencies(FILE *f, const char* device_path) {
int r;

Expand Down Expand Up @@ -314,12 +355,16 @@ static int create_disk(
fprintf(f, "Conflicts=umount.target\n");

if (keydev) {
_cleanup_free_ char *unit = NULL;
_cleanup_free_ char *unit = NULL, *umount_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");

r = generate_keydev_umount(name, keydev_mount, &umount_unit);
if (r < 0)
return log_error_errno(r, "Failed to generate keydev umount unit: %m");

password_buffer = path_join(keydev_mount, password);
if (!password_buffer)
return log_oom();
Expand All @@ -331,6 +376,15 @@ static int create_disk(
fprintf(f, "Wants=%s\n", unit);
else
fprintf(f, "Requires=%s\n", unit);

if (umount_unit) {
fprintf(f,
"Wants=%s\n"
"Before=%s\n",
umount_unit,
umount_unit
);
}
}

if (!nofail)
Expand Down Expand Up @@ -394,11 +448,6 @@ static int create_disk(
"ExecStartPost=" ROOTLIBEXECDIR "/systemd-makefs swap '/dev/mapper/%s'\n",
name_escaped);

if (keydev)
fprintf(f,
"ExecStartPost=-" UMOUNT_PATH " %s\n\n",
keydev_mount);

r = fflush_and_check(f);
if (r < 0)
return log_error_errno(r, "Failed to write unit file %s: %m", n);
Expand Down

0 comments on commit 882f5f4

Please sign in to comment.