Re: [PATCH v2 8/9] crash_dump: Disallow configfs/crash_dm_crypt_key/reuse if CONFIG_CRASH_HOTPLUG enabled

On Wed, May 06, 2026 at 09:39:35PM +0530, Sourabh Jain wrote:
>
>
>On 02/05/26 05:13, Coiby Xu wrote:
>>If CONFIG_CRASH_HOTPLUG is enabled, dm-crypt keys saved to reserved
>>memory will be took care of automatically. Thus it doesn't make sense
>>to use configfs/crash_dm_crypt_key/reuse. Reserving
>>image->dm_crypt_keys_addr is also unnecessary. Currently x86_64 and
>>ppc64le have implemented CONFIG_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 <[email protected]>
>>---
>>  Documentation/admin-guide/kdump/kdump.rst |  9 ++++++---
>>  kernel/crash_dump_dm_crypt.c              | 14 +++++++++++---
>>  2 files changed, 17 insertions(+), 6 deletions(-)
>>
>>diff --git a/Documentation/admin-guide/kdump/kdump.rst b/Documentation/admin-guide/kdump/kdump.rst
>>index 7587caadbae1..73f2e9500c60 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
>>@@ -593,7 +594,9 @@ with /sys/kernel/config/crash_dm_crypt_keys for setup,
>>      # To support CPU/memory hot-plugging, reuse keys already saved to reserved
>>      # memory
>>-    echo true > /sys/kernel/config/crash_dm_crypt_key/reuse
>>+    # Note if CONFIG_CRASH_HOTPLUG is enabled, this API is totally unnecessary
>>+    # thus will be disabled.
>>+    echo true > /sys/kernel/config/crash_dm_crypt_keys/reuse
>>  2. Load the dump-capture kernel
>>diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
>>index 36e51807d94f..7a7cae17f578 100644
>>--- a/kernel/crash_dump_dm_crypt.c
>>+++ b/kernel/crash_dump_dm_crypt.c
>>@@ -304,6 +304,11 @@ static ssize_t config_keys_reuse_store(struct config_item *item,
>>  	bool val;
>>  	int r;
>>+	if (IS_ENABLED(CONFIG_CRASH_HOTPLUG)) {
>>+		pr_info("CONFIG_CRASH_HOTPLUG already enabled");
>>+		return -EINVAL;
>>+	}
>>+
>
>Deciding this solely at compile time can create issues. For example, 
>the kernel
>may be built with CONFIG_CRASH_HOTPLUG, but if kexec tool loads the kdump
>kernel using the kexec_load system call without hotplug support, it can
>cause problems. It is rare but possible.
>
>How about this:
>
>#ifdef CONFIG_CRASH_HOTPLUG
>    if (kexec_crash_image->hotplug_support) {
>pr_info("crash image is loaded with hotplug support\n");return -EINVAL;
>    }
>#endif
>
>This code should be placed after validating kexec_crash_image.

I didn't know CONFIG_CRASH_HOTPLUG isn't sufficient to tell hotplug
support is available. Thanks for pointing it out! Although
crash_load_dm_crypt_keys is only called during kexec_file_load syscall,
it will be good to apply your suggestion!

>
>
>- Sourabh Jain
>
>>  	if (!kexec_crash_image || !kexec_crash_image->dm_crypt_keys_addr) {
>>  		pr_info("dm-crypt keys haven't be saved to crash-reserved memory\n");
>>  		return -EINVAL;
>>@@ -486,15 +491,18 @@ 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);
>>  		keys_header = NULL;
>>  	}
>>+	if (IS_ENABLED(CONFIG_CRASH_HOTPLUG))
>>+		image->dm_crypt_keys_addr = 0;

I'll also adjust this part accordingly. Thanks!

Sorry I didn't notice this email and the email for [PATCH v2 2/9] had never
been sent out because of a msmtpqueue lock issue until last week.

-- 
Best regards,
Coiby