If writing to the configfs group happens concurrently during
kexec_file_load syscall, it may lead to the following issues,
  - buffer overflow if dm-crypt keys are added after allocation
  - stale total_keys if dm-crypt keys are removed during iteration
  - keys_header will not be freed if config/crash_dm_crypt_key/reuse is
    set true

So hold config_keys_subsys.su_mutex for the entire sequence during the
kexec_file_load syscall to ensure a consistent snapshot.

Fixes: 479e58549b0f ("crash_dump: store dm crypt keys in kdump reserved memory")
Suggested-by: Sourabh Jain <[email protected]>
Signed-off-by: Coiby Xu <[email protected]>
---
 kernel/crash_dump_dm_crypt.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index 4335b6cb1fc4..d2e66c6fe6f3 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -293,6 +293,7 @@ static ssize_t config_keys_reuse_show(struct config_item 
*item, char *page)
 static ssize_t config_keys_reuse_store(struct config_item *item,
                                           const char *page, size_t count)
 {
+       struct mutex *lock;
        bool val;
        int r;
 
@@ -302,8 +303,12 @@ static ssize_t config_keys_reuse_store(struct config_item 
*item,
                return -EINVAL;
        }
 
+       lock = &to_config_group(item)->cg_subsys->su_mutex;
+       mutex_lock(lock);
+
+       r = -EINVAL;
        if (kstrtobool(page, &val) || !val)
-               return -EINVAL;
+               goto unlock;
 
        if (is_dm_key_reused) {
                pr_info("Already got dm-crypt keys, please continue with 
kexec_file_load syscall\n");
@@ -311,11 +316,15 @@ static ssize_t config_keys_reuse_store(struct config_item 
*item,
                r = get_keys_from_kdump_reserved_memory();
                if (r) {
                        pr_warn("Failed to get dm-crypt keys from reserved 
memory\n");
-                       return r;
+                       goto unlock;
                }
                is_dm_key_reused = true;
        }
 
+       r = count;
+
+unlock:
+       mutex_unlock(lock);
        return count;
 }
 
@@ -421,6 +430,8 @@ static int build_keys_header(void)
        return 0;
 }
 
+static bool mutex_acquired;
+
 int crash_load_dm_crypt_keys(struct kimage *image)
 {
        struct kexec_buf kbuf = {
@@ -432,6 +443,9 @@ int crash_load_dm_crypt_keys(struct kimage *image)
        };
        int r = 0;
 
+       mutex_lock(&config_keys_subsys.su_mutex);
+       mutex_acquired = true;
+
        if (key_count <= 0) {
                kexec_dprintk("No dm-crypt keys\n");
                return 0;
@@ -481,6 +495,11 @@ void kexec_file_post_load_cleanup_dm_crypt(struct kimage 
*image)
                kfree_sensitive(keys_header);
                keys_header = NULL;
        }
+
+       if (mutex_acquired) {
+               mutex_unlock(&config_keys_subsys.su_mutex);
+               mutex_acquired = false;
+       }
 }
 
 static int __init configfs_dmcrypt_keys_init(void)
-- 
2.55.0


Reply via email to