From 882f5f429ee14c7c39196f1b40bbbe133eaf9b2f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michal=20Sekleta=CC=81r?= <msekleta@redhat.com>
Date: Wed, 9 Sep 2020 14:00:42 +0200
Subject: [PATCH] cryptsetup: generate the unit to umount keydev filesystem

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.
---
 src/cryptsetup/cryptsetup-generator.c | 61 ++++++++++++++++++++++++---
 1 file changed, 55 insertions(+), 6 deletions(-)

diff --git a/src/cryptsetup/cryptsetup-generator.c b/src/cryptsetup/cryptsetup-generator.c
index e8550d2f75..dec4d20181 100644
--- a/src/cryptsetup/cryptsetup-generator.c
+++ b/src/cryptsetup/cryptsetup-generator.c
@@ -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;
 
@@ -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();
@@ -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)
@@ -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);