[PATCH v2] crash_dump: release keyring reference at the correct time

[PATCH v2] crash_dump: release keyring reference at the correct time

[Guangshuo Li]

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.

When more than five keys are restored, a refcount underflow/use-after-free
warning can be triggered.

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 <lgs201920130244@gmail.com>
---
v2:
  - Mention the refcount underflow/use-after warning seen when restoring
    more than five keys, as suggested by Coiby Xu.
  - Order local variables in reverse Christmas tree style.

 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 cb875ddb6ba6..c685497cd470 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -81,7 +81,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)
 	struct dm_crypt_key *key;
 	size_t keys_header_size;
 	key_ref_t keyring_ref;
+	int ret = 0;
 	u64 addr;
 
 	/* find the target keyring (which must be writable) */
@@ -118,7 +118,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 > 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);
@@ -126,8 +127,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);
 
@@ -137,7 +140,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_keyring(struct dm_crypt_key *dm_key)
-- 
2.43.0

Re: [PATCH v2] crash_dump: release keyring reference at the correct time

[Coiby Xu]

On Sat, Jul 04, 2026 at 07:25:09PM +0800, Guangshuo Li wrote:
>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.
>
>When more than five keys are restored, a refcount underflow/use-after-free
>warning can be triggered.
>
>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 <lgs201920130244@gmail.com>
>---
>v2:
>  - Mention the refcount underflow/use-after warning seen when restoring
>    more than five keys, as suggested by Coiby Xu.
>  - Order local variables in reverse Christmas tree style.
>
> 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 cb875ddb6ba6..c685497cd470 100644
>--- a/kernel/crash_dump_dm_crypt.c
>+++ b/kernel/crash_dump_dm_crypt.c
>@@ -81,7 +81,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)
> 	struct dm_crypt_key *key;
> 	size_t keys_header_size;
> 	key_ref_t keyring_ref;
>+	int ret = 0;
> 	u64 addr;
>
> 	/* find the target keyring (which must be writable) */
>@@ -118,7 +118,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 > 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);
>@@ -126,8 +127,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);
>
>@@ -137,7 +140,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_keyring(struct dm_crypt_key *dm_key)
>-- 
>2.43.0
>
>

Hi Guangshuo,

Thanks for addressing my feedback!

Reviewed-and-tested-by: Coiby Xu <Coiby.Xu@gmail.com>

-- 
Best regards,
Coiby

Re: [PATCH v2] crash_dump: release keyring reference at the correct time

[Baoquan He]

On 07/04/26 at 07:25pm, Guangshuo Li wrote:
> 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.
> 
> When more than five keys are restored, a refcount underflow/use-after-free
> warning can be triggered.
> 
> 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 <lgs201920130244@gmail.com>
> ---
> v2:
>   - Mention the refcount underflow/use-after warning seen when restoring
>     more than five keys, as suggested by Coiby Xu.
>   - Order local variables in reverse Christmas tree style.
> 
>  kernel/crash_dump_dm_crypt.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)

LGTM,

Acked-by: Baoquan He <baoquan.he@linux.dev>

> 
> diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
> index cb875ddb6ba6..c685497cd470 100644
> --- a/kernel/crash_dump_dm_crypt.c
> +++ b/kernel/crash_dump_dm_crypt.c
> @@ -81,7 +81,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)
>  	struct dm_crypt_key *key;
>  	size_t keys_header_size;
>  	key_ref_t keyring_ref;
> +	int ret = 0;
>  	u64 addr;
>  
>  	/* find the target keyring (which must be writable) */
> @@ -118,7 +118,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 > 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);
> @@ -126,8 +127,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);
>  
> @@ -137,7 +140,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_keyring(struct dm_crypt_key *dm_key)
> -- 
> 2.43.0
> 

Re: [PATCH v2] crash_dump: release keyring reference at the correct time

[Bradley Morgan]

Nice! :)

Reviewed-by: Bradley Morgan <include@grrlz.net>

Thanks!

Re: [PATCH v2] crash_dump: release keyring reference at the correct time

[Mike Rapoport]

On Sat, 04 Jul 2026 19:25:09 +0800, Guangshuo Li wrote:
> crash_dump: release keyring reference at the correct time

Applied to kexec-fixes branch of liveupdate/linux.git tree, thanks!

[1/1] crash_dump: release keyring reference at the correct time
      commit: ada2e5a44e99113e08ad9b7b71396c6c572204da

tree: https://git.kernel.org/pub/scm/linux/kernel/git/liveupdate/linux
branch: kexec-fixes

--
Sincerely yours,
Mike.