Re: [PATCH v4 8/9] crash_dump: Disallow configfs/crash_dm_crypt_key/reuse if crash hotplug supported

[Coiby Xu <coiby.xu@gmail.com>]

On Sun, Aug 30, 2026 at 01:28:59PM +0530, Sourabh Jain wrote:
>
>
>On 28/08/26 14:18, Coiby Xu wrote:
>>If crash hotplug is supported, dm-crypt keys saved to reserved memory
>>will be taken care of automatically. Thus it doesn't make sense to use
>>configfs/crash_dm_crypt_key/reuse. Not reserving
>>image->dm_crypt_keys_addr makes it implicitly to disallow using this
>>API. Currently x86_64 and ppc64le have implemented crash hotplug
>>feature.
>>
>>Also update the doc accordingly. Note two doc issues are fixed as well.
>>
>>Fixes: 9ebfa8dcaea7 ("crash_dump: reuse saved dm crypt keys for CPU/memory hot-plugging")
>>Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
>>---
>>  Documentation/admin-guide/kdump/kdump.rst | 16 ++++++++++------
>>  kernel/crash_dump_dm_crypt.c              | 13 +++++++++----
>>  2 files changed, 19 insertions(+), 10 deletions(-)
>>
>>diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst
>>index 7587caadbae1..0bf2eb100a05 100644
>>--- a/Documentation/admin-guide/kdump/kdump.rst
>>+++ b/Documentation/admin-guide/kdump/kdump.rst
>>@@ -577,9 +577,10 @@ with /sys/kernel/config/crash_dm_crypt_keys for setup,
>>  1. Tell the first kernel what logon keys are needed to unlock the disk volumes,
>>      # Add key #1
>>-    mkdir /sys/kernel/config/crash_dm_crypt_keys/7d26b7b4-e342-4d2d-b660-7426b0996720
>>+    VOL1_UUID=7d26b7b4-e342-4d2d-b660-7426b0996720
>>+    mkdir /sys/kernel/config/crash_dm_crypt_keys/$VOL1_UUID
>>      # Add key #1's description
>>-    echo cryptsetup:7d26b7b4-e342-4d2d-b660-7426b0996720 > /sys/kernel/config/crash_dm_crypt_keys/description
>>+    echo cryptsetup:$VOL1_UUID > /sys/kernel/config/crash_dm_crypt_keys/$VOL1_UUID/description
>>      # how many keys do we have now?
>>      cat /sys/kernel/config/crash_dm_crypt_keys/count
>>@@ -591,15 +592,18 @@ with /sys/kernel/config/crash_dm_crypt_keys for setup,
>>      cat /sys/kernel/config/crash_dm_crypt_keys/count
>>      2
>>-    # To support CPU/memory hot-plugging, reuse keys already saved to reserved
>>-    # memory
>>-    echo true > /sys/kernel/config/crash_dm_crypt_key/reuse
>>-
>>  2. Load the dump-capture kernel
>>  3. After the dump-capture kerne get booted, restore the keys to user keyring
>>     echo yes > /sys/kernel/crash_dm_crypt_keys/restore
>>+For CPU/memory hot-plugging, you can reuse keys already saved to reserved
>>+memory before reloading the kdump image,
>>+    echo true > /sys/kernel/config/crash_dm_crypt_keys/reuse
>>+
>>+Note if crash hotplug is supported, this API is totally unnecessary thus will
>>+be disabled automatically.
>>+
>>  Contact
>>  =======
>>diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
>>index b7629174e4db..3aaa0f9c3117 100644
>>--- a/kernel/crash_dump_dm_crypt.c
>>+++ b/kernel/crash_dump_dm_crypt.c
>>@@ -324,7 +324,7 @@ static ssize_t config_keys_reuse_store(struct config_item *item,
>>  	r = -EINVAL;
>>  	if (!kexec_crash_image || !kexec_crash_image->dm_crypt_keys_addr) {
>>-		pr_debug("dm-crypt keys haven't be saved to crash-reserved memory\n");
>>+		pr_debug("dm-crypt keys haven't be saved to crash-reserved memory or crash hotplug supported\n");
>
>Nit: the above line is more than 100 characters. There is no
>complaint from checkpatch, but I feel we should reduce it to
>below 100 characters.

Thanks for the reminder! Documentation/process/coding-style.rst
explicitly states,
     However, never break user-visible strings such as printk messages because
     that breaks the ability to grep for them.

In fact, scripts/checkpatch.pl will issue a SPLIT_STRING warning if we
break the user-visible strings. So I think this is accepted unless
kexec/kdump subsystem has a hard column limit.

>
>Also, I checked that we don't have pr_fmt defined for this file,
>but we do have pr_debug and other pr_* calls. See if defining
>pr_fmt helps reduce the character count in the above line.

If I'm not mistaken, pr_fmt only adds a prefix. So I don't think it will
help reduce the character count in this particular case.

>
>But the overall idea of handling crash hotplug support with
>crash dm-crypt looks good to me. So feel free to add:
>
>Reviewed-by: Sourabh Jain sourabhjain@linux.ibm.com

Thanks for reviewing and ack'ing the patch!

>
>Thanks,
>Sourabh Jain
>
>
>>  		goto unlock;
>>  	}
>>@@ -519,9 +519,9 @@ int crash_load_dm_crypt_keys(struct kimage *image)
>>  void kexec_file_post_load_cleanup_dm_crypt(struct kimage *image)
>>  {
>>  	/*
>>-	 * For CPU/memory hot-plugging, the kdump image will be reloaded. Prevent
>>-	 * keys_header from being cleaned up during unloading when
>>-	 * is_dm_key_reused=true
>>+	 * For CPU/memory hot-plugging without CONFIG_CRASH_HOTPLUG, the whole kdump
>>+	 * image will be reloaded. Prevent keys_header from being cleaned up during
>>+	 * unloading when is_dm_key_reused=true
>>  	 */
>>  	if (!is_dm_key_reused) {
>>  		kfree_sensitive(keys_header);
>>@@ -532,6 +532,11 @@ void kexec_file_post_load_cleanup_dm_crypt(struct kimage *image)
>>  		mutex_unlock(&config_keys_subsys.su_mutex);
>>  		mutex_acquired = false;
>>  	}
>>+
>>+#ifdef CONFIG_CRASH_HOTPLUG
>>+	if (image->hotplug_support)
>>+		image->dm_crypt_keys_addr = 0;
>>+#endif
>>  }
>>  static int __init configfs_dmcrypt_keys_init(void)
>

-- 
Best regards,
Coiby