1st kernel will build up the kernel command parameter dmcryptkeys as
similar to elfcorehdr to pass the memory address of the stored info of
dm crypt key to kdump kernel.
Signed-off-by: Coiby Xu <coxu@redhat.com>
---
arch/x86/kernel/crash.c | 20 ++++++++++++++++++--
arch/x86/kernel/kexec-bzimage64.c | 7 +++++++
2 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 340af8155658..99d50c31db02 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -278,6 +278,7 @@ static int memmap_exclude_ranges(struct kimage *image, struct crash_mem *cmem,
unsigned long long mend)
{
unsigned long start, end;
+ int ret;
cmem->ranges[0].start = mstart;
cmem->ranges[0].end = mend;
@@ -286,22 +287,37 @@ static int memmap_exclude_ranges(struct kimage *image, struct crash_mem *cmem,
/* Exclude elf header region */
start = image->elf_load_addr;
end = start + image->elf_headers_sz - 1;
- return crash_exclude_mem_range(cmem, start, end);
+ ret = crash_exclude_mem_range(cmem, start, end);
+
+ if (ret)
+ return ret;
+
+ /* Exclude dm crypt keys region */
+ if (image->dm_crypt_keys_addr) {
+ start = image->dm_crypt_keys_addr;
+ end = start + image->dm_crypt_keys_sz - 1;
+ return crash_exclude_mem_range(cmem, start, end);
+ }
+
+ return ret;
}
/* Prepare memory map for crash dump kernel */
int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
{
+ unsigned int max_nr_ranges = 3;
int i, ret = 0;
unsigned long flags;
struct e820_entry ei;
struct crash_memmap_data cmd;
struct crash_mem *cmem;
- cmem = vzalloc(struct_size(cmem, ranges, 1));
+ cmem = vzalloc(struct_size(cmem, ranges, max_nr_ranges));
if (!cmem)
return -ENOMEM;
+ cmem->max_nr_ranges = max_nr_ranges;
+
memset(&cmd, 0, sizeof(struct crash_memmap_data));
cmd.params = params;
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 68530fad05f7..9c94428927bd 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -76,6 +76,10 @@ static int setup_cmdline(struct kimage *image, struct boot_params *params,
if (image->type == KEXEC_TYPE_CRASH) {
len = sprintf(cmdline_ptr,
"elfcorehdr=0x%lx ", image->elf_load_addr);
+
+ if (image->dm_crypt_keys_addr != 0)
+ len += sprintf(cmdline_ptr + len,
+ "dmcryptkeys=0x%lx ", image->dm_crypt_keys_addr);
}
memcpy(cmdline_ptr + len, cmdline, cmdline_len);
cmdline_len += len;
@@ -441,6 +445,9 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
ret = crash_load_segments(image);
if (ret)
return ERR_PTR(ret);
+ ret = crash_load_dm_crypt_keys(image);
+ if (ret)
+ pr_debug("Either no dm crypt key or error to retrieve the dm crypt key\n");
}
#endif
--
2.47.0On 10/29/24 at 01:52pm, Coiby Xu wrote:
> 1st kernel will build up the kernel command parameter dmcryptkeys as
> similar to elfcorehdr to pass the memory address of the stored info of
> dm crypt key to kdump kernel.
>
> Signed-off-by: Coiby Xu <coxu@redhat.com>
> ---
> arch/x86/kernel/crash.c | 20 ++++++++++++++++++--
> arch/x86/kernel/kexec-bzimage64.c | 7 +++++++
> 2 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index 340af8155658..99d50c31db02 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -278,6 +278,7 @@ static int memmap_exclude_ranges(struct kimage *image, struct crash_mem *cmem,
> unsigned long long mend)
> {
> unsigned long start, end;
> + int ret;
>
> cmem->ranges[0].start = mstart;
> cmem->ranges[0].end = mend;
> @@ -286,22 +287,37 @@ static int memmap_exclude_ranges(struct kimage *image, struct crash_mem *cmem,
> /* Exclude elf header region */
> start = image->elf_load_addr;
> end = start + image->elf_headers_sz - 1;
> - return crash_exclude_mem_range(cmem, start, end);
> + ret = crash_exclude_mem_range(cmem, start, end);
> +
> + if (ret)
> + return ret;
> +
> + /* Exclude dm crypt keys region */
> + if (image->dm_crypt_keys_addr) {
> + start = image->dm_crypt_keys_addr;
> + end = start + image->dm_crypt_keys_sz - 1;
> + return crash_exclude_mem_range(cmem, start, end);
> + }
> +
> + return ret;
> }
>
> /* Prepare memory map for crash dump kernel */
> int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
> {
> + unsigned int max_nr_ranges = 3;
Define a macro and add code comment to explain why this value is taken?
> int i, ret = 0;
> unsigned long flags;
> struct e820_entry ei;
> struct crash_memmap_data cmd;
> struct crash_mem *cmem;
>
> - cmem = vzalloc(struct_size(cmem, ranges, 1));
> + cmem = vzalloc(struct_size(cmem, ranges, max_nr_ranges));
> if (!cmem)
> return -ENOMEM;
>
> + cmem->max_nr_ranges = max_nr_ranges;
> +
> memset(&cmd, 0, sizeof(struct crash_memmap_data));
> cmd.params = params;
>
> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> index 68530fad05f7..9c94428927bd 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -76,6 +76,10 @@ static int setup_cmdline(struct kimage *image, struct boot_params *params,
> if (image->type == KEXEC_TYPE_CRASH) {
> len = sprintf(cmdline_ptr,
> "elfcorehdr=0x%lx ", image->elf_load_addr);
> +
> + if (image->dm_crypt_keys_addr != 0)
> + len += sprintf(cmdline_ptr + len,
> + "dmcryptkeys=0x%lx ", image->dm_crypt_keys_addr);
> }
> memcpy(cmdline_ptr + len, cmdline, cmdline_len);
> cmdline_len += len;
> @@ -441,6 +445,9 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> ret = crash_load_segments(image);
> if (ret)
> return ERR_PTR(ret);
> + ret = crash_load_dm_crypt_keys(image);
> + if (ret)
> + pr_debug("Either no dm crypt key or error to retrieve the dm crypt key\n");
If it's error, do we need change ti to pr_debug?
> }
> #endif
>
> --
> 2.47.0
>
On Wed, Dec 11, 2024 at 08:55:52PM +0800, Baoquan He wrote:
>On 10/29/24 at 01:52pm, Coiby Xu wrote:
>> 1st kernel will build up the kernel command parameter dmcryptkeys as
>> similar to elfcorehdr to pass the memory address of the stored info of
>> dm crypt key to kdump kernel.
>>
>> Signed-off-by: Coiby Xu <coxu@redhat.com>
>> ---
>> arch/x86/kernel/crash.c | 20 ++++++++++++++++++--
>> arch/x86/kernel/kexec-bzimage64.c | 7 +++++++
>> 2 files changed, 25 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
>> index 340af8155658..99d50c31db02 100644
>> --- a/arch/x86/kernel/crash.c
>> +++ b/arch/x86/kernel/crash.c
>> @@ -278,6 +278,7 @@ static int memmap_exclude_ranges(struct kimage *image, struct crash_mem *cmem,
>> unsigned long long mend)
>> {
>> unsigned long start, end;
>> + int ret;
>>
>> cmem->ranges[0].start = mstart;
>> cmem->ranges[0].end = mend;
>> @@ -286,22 +287,37 @@ static int memmap_exclude_ranges(struct kimage *image, struct crash_mem *cmem,
>> /* Exclude elf header region */
>> start = image->elf_load_addr;
>> end = start + image->elf_headers_sz - 1;
>> - return crash_exclude_mem_range(cmem, start, end);
>> + ret = crash_exclude_mem_range(cmem, start, end);
>> +
>> + if (ret)
>> + return ret;
>> +
>> + /* Exclude dm crypt keys region */
>> + if (image->dm_crypt_keys_addr) {
>> + start = image->dm_crypt_keys_addr;
>> + end = start + image->dm_crypt_keys_sz - 1;
>> + return crash_exclude_mem_range(cmem, start, end);
>> + }
>> +
>> + return ret;
>> }
>>
>> /* Prepare memory map for crash dump kernel */
>> int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
>> {
>> + unsigned int max_nr_ranges = 3;
>
>Define a macro and add code comment to explain why this value is taken?
Good suggestion! While drafting the reason for max_nr_ranges=3, I
realize max_nr_ranges=2 is sufficient. I also notice the fill_up_crash_elf_data
function in the same source file has similar comment. So next version
I'll add the following change,
int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
{
+ unsigned int nr_ranges = 0;
int i, ret = 0;
unsigned long flags;
struct e820_entry ei;
struct crash_memmap_data cmd;
struct crash_mem *cmem;
- cmem = vzalloc(struct_size(cmem, ranges, 1));
+ /*
+ * Using random kexec_buf for passing dm crypt keys may cause a range
+ * split. So use two slots here.
+ */
+ nr_ranges = 2;
+ cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
if (!cmem)
return -ENOMEM;
+ cmem->max_nr_ranges = nr_ranges;
+ cmem->nr_ranges = 0;
>
>> int i, ret = 0;
>> unsigned long flags;
>> struct e820_entry ei;
>> struct crash_memmap_data cmd;
>> struct crash_mem *cmem;
>>
>> - cmem = vzalloc(struct_size(cmem, ranges, 1));
>> + cmem = vzalloc(struct_size(cmem, ranges, max_nr_ranges));
>> if (!cmem)
>> return -ENOMEM;
>>
>> + cmem->max_nr_ranges = max_nr_ranges;
>> +
>> memset(&cmd, 0, sizeof(struct crash_memmap_data));
>> cmd.params = params;
>>
>> diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
>> index 68530fad05f7..9c94428927bd 100644
>> --- a/arch/x86/kernel/kexec-bzimage64.c
>> +++ b/arch/x86/kernel/kexec-bzimage64.c
>> @@ -76,6 +76,10 @@ static int setup_cmdline(struct kimage *image, struct boot_params *params,
>> if (image->type == KEXEC_TYPE_CRASH) {
>> len = sprintf(cmdline_ptr,
>> "elfcorehdr=0x%lx ", image->elf_load_addr);
>> +
>> + if (image->dm_crypt_keys_addr != 0)
>> + len += sprintf(cmdline_ptr + len,
>> + "dmcryptkeys=0x%lx ", image->dm_crypt_keys_addr);
>> }
>> memcpy(cmdline_ptr + len, cmdline, cmdline_len);
>> cmdline_len += len;
>> @@ -441,6 +445,9 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
>> ret = crash_load_segments(image);
>> if (ret)
>> return ERR_PTR(ret);
>> + ret = crash_load_dm_crypt_keys(image);
>> + if (ret)
>> + pr_debug("Either no dm crypt key or error to retrieve the dm crypt key\n");
>
>If it's error, do we need change ti to pr_debug?
Thanks for raising the concern! I think it's OK to let
crash_load_dm_crypt_keys fail since disk encryption may not be used for
kdump, thus pr_debug is sufficient. Or have I misunderstood your comment?
>> }
>> #endif
>>
>> --
>> 2.47.0
>>
>
--
Best regards,
Coiby
On 12/23/24 at 09:16am, Coiby Xu wrote:
> On Wed, Dec 11, 2024 at 08:55:52PM +0800, Baoquan He wrote:
> > On 10/29/24 at 01:52pm, Coiby Xu wrote:
......
> > > diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
> > > index 68530fad05f7..9c94428927bd 100644
> > > --- a/arch/x86/kernel/kexec-bzimage64.c
> > > +++ b/arch/x86/kernel/kexec-bzimage64.c
> > > @@ -76,6 +76,10 @@ static int setup_cmdline(struct kimage *image, struct boot_params *params,
> > > if (image->type == KEXEC_TYPE_CRASH) {
> > > len = sprintf(cmdline_ptr,
> > > "elfcorehdr=0x%lx ", image->elf_load_addr);
> > > +
> > > + if (image->dm_crypt_keys_addr != 0)
> > > + len += sprintf(cmdline_ptr + len,
> > > + "dmcryptkeys=0x%lx ", image->dm_crypt_keys_addr);
> > > }
> > > memcpy(cmdline_ptr + len, cmdline, cmdline_len);
> > > cmdline_len += len;
> > > @@ -441,6 +445,9 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
> > > ret = crash_load_segments(image);
> > > if (ret)
> > > return ERR_PTR(ret);
> > > + ret = crash_load_dm_crypt_keys(image);
> > > + if (ret)
> > > + pr_debug("Either no dm crypt key or error to retrieve the dm crypt key\n");
> >
> > If it's error, do we need change ti to pr_debug?
>
> Thanks for raising the concern! I think it's OK to let
> crash_load_dm_crypt_keys fail since disk encryption may not be used for
> kdump, thus pr_debug is sufficient. Or have I misunderstood your comment?
If crash_load_dm_crypt_keys() returned error, shouldn't we handle them
separately? If disk encryption is not used, we return a specific value
to indicate that, surely no need to pr_err(). However, if disk
encryption is used for kdump but error is caused and failed
crash_load_dm_crypt_keys(), why don't we error out for the case?
Maybe below change can be made to differentiate them?
diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
index f72a88b7d106..327f84ea57c5 100644
--- a/kernel/crash_dump_dm_crypt.c
+++ b/kernel/crash_dump_dm_crypt.c
@@ -400,7 +400,7 @@ int crash_load_dm_crypt_keys(struct kimage *image)
if (key_count <= 0) {
kexec_dprintk("No dm-crypt keys\n");
- return -EINVAL;
+ return -ENOENT;
}
On Thu, Dec 26, 2024 at 11:48:04AM +0800, Baoquan He wrote:
>On 12/23/24 at 09:16am, Coiby Xu wrote:
>> On Wed, Dec 11, 2024 at 08:55:52PM +0800, Baoquan He wrote:
>> > On 10/29/24 at 01:52pm, Coiby Xu wrote:
>......
>> > > diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
>> > > index 68530fad05f7..9c94428927bd 100644
>> > > --- a/arch/x86/kernel/kexec-bzimage64.c
>> > > +++ b/arch/x86/kernel/kexec-bzimage64.c
>> > > @@ -76,6 +76,10 @@ static int setup_cmdline(struct kimage *image, struct boot_params *params,
>> > > if (image->type == KEXEC_TYPE_CRASH) {
>> > > len = sprintf(cmdline_ptr,
>> > > "elfcorehdr=0x%lx ", image->elf_load_addr);
>> > > +
>> > > + if (image->dm_crypt_keys_addr != 0)
>> > > + len += sprintf(cmdline_ptr + len,
>> > > + "dmcryptkeys=0x%lx ", image->dm_crypt_keys_addr);
>> > > }
>> > > memcpy(cmdline_ptr + len, cmdline, cmdline_len);
>> > > cmdline_len += len;
>> > > @@ -441,6 +445,9 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
>> > > ret = crash_load_segments(image);
>> > > if (ret)
>> > > return ERR_PTR(ret);
>> > > + ret = crash_load_dm_crypt_keys(image);
>> > > + if (ret)
>> > > + pr_debug("Either no dm crypt key or error to retrieve the dm crypt key\n");
>> >
>> > If it's error, do we need change ti to pr_debug?
>>
>> Thanks for raising the concern! I think it's OK to let
>> crash_load_dm_crypt_keys fail since disk encryption may not be used for
>> kdump, thus pr_debug is sufficient. Or have I misunderstood your comment?
>
>If crash_load_dm_crypt_keys() returned error, shouldn't we handle them
>separately? If disk encryption is not used, we return a specific value
>to indicate that, surely no need to pr_err(). However, if disk
>encryption is used for kdump but error is caused and failed
>crash_load_dm_crypt_keys(), why don't we error out for the case?
>
>Maybe below change can be made to differentiate them?
>
>diff --git a/kernel/crash_dump_dm_crypt.c b/kernel/crash_dump_dm_crypt.c
>index f72a88b7d106..327f84ea57c5 100644
>--- a/kernel/crash_dump_dm_crypt.c
>+++ b/kernel/crash_dump_dm_crypt.c
>@@ -400,7 +400,7 @@ int crash_load_dm_crypt_keys(struct kimage *image)
>
> if (key_count <= 0) {
> kexec_dprintk("No dm-crypt keys\n");
>- return -EINVAL;
>+ return -ENOENT;
> }
Now I see your point! Thanks for the explanation and suggestion! Yes,
it's better to fail early if crash_load_dm_crypt_keys failed when
encrypted disk is used for kdump . I'll apply the change to next
version.
--
Best regards,
Coiby
© 2016 - 2026 Red Hat, Inc.