restore_dm_crypt_keys_to_thread_keyring() gets a reference to the user
keyring before restoring the saved dm-crypt keys.

The same keyring reference is then passed to add_key_to_keyring() for each
saved key, but add_key_to_keyring() drops that reference on every call.
This is only balanced when exactly one key is restored. With multiple
keys, the keyring reference is dropped too many times and may trigger a
refcount underflow or use-after-free.

The early error paths after lookup_user_key() also return without dropping
the keyring reference.

Keep ownership of the keyring reference in
restore_dm_crypt_keys_to_thread_keyring(), drop it once on all exit paths,
and make add_key_to_keyring() only use the reference without consuming it.

Fixes: 62f17d9df692 ("crash_dump: retrieve dm crypt keys in kdump kernel")
Signed-off-by: Guangshuo Li <[email protected]>
---
 kernel/crash_dump_dm_crypt.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index a20d4097744a..641c290f1270 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -80,7 +80,6 @@ static int add_key_to_keyring(struct dm_crypt_key *dm_key,
                kexec_dprintk("Error when adding key");
        }
 
-       key_ref_put(keyring_ref);
        return r;
 }
 
@@ -104,6 +103,7 @@ static int restore_dm_crypt_keys_to_thread_keyring(void)
        size_t keys_header_size;
        key_ref_t keyring_ref;
        u64 addr;
+       int ret = 0;
 
        /* find the target keyring (which must be writable) */
        keyring_ref =
@@ -117,7 +117,8 @@ static int restore_dm_crypt_keys_to_thread_keyring(void)
        dm_crypt_keys_read((char *)&key_count, sizeof(key_count), &addr);
        if (key_count < 0 || key_count > KEY_NUM_MAX) {
                kexec_dprintk("Failed to read the number of dm-crypt keys\n");
-               return -1;
+               ret = -1;
+               goto out;
        }
 
        kexec_dprintk("There are %u keys\n", key_count);
@@ -125,8 +126,10 @@ static int restore_dm_crypt_keys_to_thread_keyring(void)
 
        keys_header_size = get_keys_header_size(key_count);
        keys_header = kzalloc(keys_header_size, GFP_KERNEL);
-       if (!keys_header)
-               return -ENOMEM;
+       if (!keys_header) {
+               ret = -ENOMEM;
+               goto out;
+       }
 
        dm_crypt_keys_read((char *)keys_header, keys_header_size, &addr);
 
@@ -136,7 +139,9 @@ static int restore_dm_crypt_keys_to_thread_keyring(void)
                add_key_to_keyring(key, keyring_ref);
        }
 
-       return 0;
+out:
+       key_ref_put(keyring_ref);
+       return ret;
 }
 
 static int read_key_from_user_keying(struct dm_crypt_key *dm_key)
-- 
2.43.0


Reply via email to