[PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[fuqiang wang]

In memmap_exclude_ranges(), elfheader will be excluded from crashk_res.
In the current x86 architecture code, the elfheader is always allocated
at crashk_res.start. It seems that there won't be a new split range.
But it depends on the allocation position of elfheader in crashk_res. To
avoid potential out of bounds in future, add a extra slot.

The similar issue also exists in fill_up_crash_elf_data(). The range to
be excluded is [0, 1M], start (0) is special and will not appear in the
middle of existing cmem->ranges[]. But in cast the low 1M could be
changed in the future, add a extra slot too.

Previously discussed link:
[1] https://lore.kernel.org/kexec/ZXk2oBf%2FT1Ul6o0c@MiWiFi-R3L-srv/
[2] https://lore.kernel.org/kexec/273284e8-7680-4f5f-8065-c5d780987e59@easystack.cn/
[3] https://lore.kernel.org/kexec/ZYQ6O%2F57sHAPxTHm@MiWiFi-R3L-srv/

Signed-off-by: fuqiang wang <fuqiang.wang@easystack.cn>
---
 arch/x86/kernel/crash.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index b6b044356f1b..d21592ad8952 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -149,8 +149,18 @@ static struct crash_mem *fill_up_crash_elf_data(void)
 	/*
 	 * Exclusion of crash region and/or crashk_low_res may cause
 	 * another range split. So add extra two slots here.
+	 *
+	 * Exclusion of low 1M may not cause another range split, because the
+	 * range of exclude is [0, 1M] and the condition for splitting a new
+	 * region is that the start, end parameters are both in a certain
+	 * existing region in cmem and cannot be equal to existing region's
+	 * start or end. Obviously, the start of [0, 1M] cannot meet this
+	 * condition.
+	 *
+	 * But in order to lest the low 1M could be changed in the future,
+	 * (e.g. [stare, 1M]), add a extra slot.
 	 */
-	nr_ranges += 2;
+	nr_ranges += 3;
 	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
 	if (!cmem)
 		return NULL;
@@ -282,9 +292,16 @@ int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
 	struct crash_memmap_data cmd;
 	struct crash_mem *cmem;
 
-	cmem = vzalloc(struct_size(cmem, ranges, 1));
+	/*
+	 * In the current x86 architecture code, the elfheader is always
+	 * allocated at crashk_res.start. But it depends on the allocation
+	 * position of elfheader in crashk_res. To avoid potential out of
+	 * bounds in future, add a extra slot.
+	 */
+	cmem = vzalloc(struct_size(cmem, ranges, 2));
 	if (!cmem)
 		return -ENOMEM;
+	cmem->max_nr_ranges = 2;
 
 	memset(&cmd, 0, sizeof(struct crash_memmap_data));
 	cmd.params = params;
-- 
2.42.0

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Baoquan He]

On 01/08/24 at 09:06pm, fuqiang wang wrote:
> In memmap_exclude_ranges(), elfheader will be excluded from crashk_res.
> In the current x86 architecture code, the elfheader is always allocated
> at crashk_res.start. It seems that there won't be a new split range.
> But it depends on the allocation position of elfheader in crashk_res. To
> avoid potential out of bounds in future, add a extra slot.
> 
> The similar issue also exists in fill_up_crash_elf_data(). The range to
> be excluded is [0, 1M], start (0) is special and will not appear in the
> middle of existing cmem->ranges[]. But in cast the low 1M could be
> changed in the future, add a extra slot too.
> 
> Previously discussed link:
> [1] https://lore.kernel.org/kexec/ZXk2oBf%2FT1Ul6o0c@MiWiFi-R3L-srv/
> [2] https://lore.kernel.org/kexec/273284e8-7680-4f5f-8065-c5d780987e59@easystack.cn/
> [3] https://lore.kernel.org/kexec/ZYQ6O%2F57sHAPxTHm@MiWiFi-R3L-srv/
> 
> Signed-off-by: fuqiang wang <fuqiang.wang@easystack.cn>
> ---
>  arch/x86/kernel/crash.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index b6b044356f1b..d21592ad8952 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -149,8 +149,18 @@ static struct crash_mem *fill_up_crash_elf_data(void)
>  	/*
>  	 * Exclusion of crash region and/or crashk_low_res may cause
>  	 * another range split. So add extra two slots here.
> +	 *
> +	 * Exclusion of low 1M may not cause another range split, because the
> +	 * range of exclude is [0, 1M] and the condition for splitting a new
> +	 * region is that the start, end parameters are both in a certain
> +	 * existing region in cmem and cannot be equal to existing region's
> +	 * start or end. Obviously, the start of [0, 1M] cannot meet this
> +	 * condition.
> +	 *
> +	 * But in order to lest the low 1M could be changed in the future,
> +	 * (e.g. [stare, 1M]), add a extra slot.
>  	 */
> -	nr_ranges += 2;
> +	nr_ranges += 3;
>  	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
>  	if (!cmem)
>  		return NULL;
> @@ -282,9 +292,16 @@ int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
>  	struct crash_memmap_data cmd;
>  	struct crash_mem *cmem;
>  
> -	cmem = vzalloc(struct_size(cmem, ranges, 1));
> +	/*
> +	 * In the current x86 architecture code, the elfheader is always
> +	 * allocated at crashk_res.start. But it depends on the allocation
> +	 * position of elfheader in crashk_res. To avoid potential out of
> +	 * bounds in future, add a extra slot.
> +	 */
> +	cmem = vzalloc(struct_size(cmem, ranges, 2));
>  	if (!cmem)
>  		return -ENOMEM;
> +	cmem->max_nr_ranges = 2;

LGTM, thx

Acked-by: Baoquan He <bhe@redhat.com>

>  
>  	memset(&cmd, 0, sizeof(struct crash_memmap_data));
>  	cmd.params = params;
> -- 
> 2.42.0
> 

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Coiby Xu]

On Tue, Jan 09, 2024 at 11:46:15AM +0800, Baoquan He wrote:
>On 01/08/24 at 09:06pm, fuqiang wang wrote:
>> In memmap_exclude_ranges(), elfheader will be excluded from crashk_res.
>> In the current x86 architecture code, the elfheader is always allocated
>> at crashk_res.start. It seems that there won't be a new split range.
>> But it depends on the allocation position of elfheader in crashk_res. To
>> avoid potential out of bounds in future, add a extra slot.
>>
>> The similar issue also exists in fill_up_crash_elf_data(). The range to
>> be excluded is [0, 1M], start (0) is special and will not appear in the
>> middle of existing cmem->ranges[]. But in cast the low 1M could be
>> changed in the future, add a extra slot too.
>>
>> Previously discussed link:
>> [1] https://lore.kernel.org/kexec/ZXk2oBf%2FT1Ul6o0c@MiWiFi-R3L-srv/
>> [2] https://lore.kernel.org/kexec/273284e8-7680-4f5f-8065-c5d780987e59@easystack.cn/
>> [3] https://lore.kernel.org/kexec/ZYQ6O%2F57sHAPxTHm@MiWiFi-R3L-srv/
>>
>> Signed-off-by: fuqiang wang <fuqiang.wang@easystack.cn>
>> ---
>>  arch/x86/kernel/crash.c | 21 +++++++++++++++++++--
>>  1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
>> index b6b044356f1b..d21592ad8952 100644
>> --- a/arch/x86/kernel/crash.c
>> +++ b/arch/x86/kernel/crash.c
>> @@ -149,8 +149,18 @@ static struct crash_mem *fill_up_crash_elf_data(void)
>>  	/*
>>  	 * Exclusion of crash region and/or crashk_low_res may cause
>>  	 * another range split. So add extra two slots here.
>> +	 *
>> +	 * Exclusion of low 1M may not cause another range split, because the
>> +	 * range of exclude is [0, 1M] and the condition for splitting a new
>> +	 * region is that the start, end parameters are both in a certain
>> +	 * existing region in cmem and cannot be equal to existing region's
>> +	 * start or end. Obviously, the start of [0, 1M] cannot meet this
>> +	 * condition.
>> +	 *
>> +	 * But in order to lest the low 1M could be changed in the future,
>> +	 * (e.g. [stare, 1M]), add a extra slot.
>>  	 */
>> -	nr_ranges += 2;
>> +	nr_ranges += 3;
>>  	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
>>  	if (!cmem)
>>  		return NULL;
>> @@ -282,9 +292,16 @@ int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
>>  	struct crash_memmap_data cmd;
>>  	struct crash_mem *cmem;
>>
>> -	cmem = vzalloc(struct_size(cmem, ranges, 1));
>> +	/*
>> +	 * In the current x86 architecture code, the elfheader is always
>> +	 * allocated at crashk_res.start. But it depends on the allocation
>> +	 * position of elfheader in crashk_res. To avoid potential out of
>> +	 * bounds in future, add a extra slot.
>> +	 */
>> +	cmem = vzalloc(struct_size(cmem, ranges, 2));
>>  	if (!cmem)
>>  		return -ENOMEM;
>> +	cmem->max_nr_ranges = 2;
>
>LGTM, thx
>
>Acked-by: Baoquan He <bhe@redhat.com>

Hi Andrew,

It seems this patch was missed. Will you pick it up? Without this patch,
kdump kernel will fail to be loaded by the kexec_file_load,

  [  139.736948] UBSAN: array-index-out-of-bounds in arch/x86/kernel/crash.c:350:25
  [  139.742360] index 0 is out of range for type 'range [*]'
  [  139.745695] CPU: 0 UID: 0 PID: 5778 Comm: kexec Not tainted 6.15.0-0.rc3.20250425git02ddfb981de8.32.fc43.x86_64 #1 PREEMPT(lazy) 
  [  139.745698] Hardware name: Amazon EC2 c5.large/, BIOS 1.0 10/16/2017
  [  139.745699] Call Trace:
  [  139.745700]  <TASK>
  [  139.745701]  dump_stack_lvl+0x5d/0x80
  [  139.745706]  ubsan_epilogue+0x5/0x2b
  [  139.745709]  __ubsan_handle_out_of_bounds.cold+0x54/0x59
  [  139.745711]  crash_setup_memmap_entries+0x2d9/0x330
  [  139.745716]  setup_boot_parameters+0xf8/0x6a0
  [  139.745720]  bzImage64_load+0x41b/0x4e0
  [  139.745722]  ? find_next_iomem_res+0x109/0x140
  [  139.745727]  ? locate_mem_hole_callback+0x109/0x170
  [  139.745737]  kimage_file_alloc_init+0x1ef/0x3e0
  [  139.745740]  __do_sys_kexec_file_load+0x180/0x2f0
  [  139.745742]  do_syscall_64+0x7b/0x160
  [  139.745745]  ? do_user_addr_fault+0x21a/0x690
  [  139.745747]  ? exc_page_fault+0x7e/0x1a0
  [  139.745749]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
  [  139.745751] RIP: 0033:0x7f7712c84e4d

Thanks!

-- 
Best regards,
Coiby

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Andrew Morton]

On Thu, 8 May 2025 12:25:15 +0800 Coiby Xu <coxu@redhat.com> wrote:

> >
> >Acked-by: Baoquan He <bhe@redhat.com>
> 
> Hi Andrew,
> 
> It seems this patch was missed.

January 2024.  Yes, it's fair to assume that it was missed ;)

> Will you pick it up?

Sure.

> Without this patch,
> kdump kernel will fail to be loaded by the kexec_file_load,
> 
>   [  139.736948] UBSAN: array-index-out-of-bounds in arch/x86/kernel/crash.c:350:25
>   [  139.742360] index 0 is out of range for type 'range [*]'
>   [  139.745695] CPU: 0 UID: 0 PID: 5778 Comm: kexec Not tainted 6.15.0-0.rc3.20250425git02ddfb981de8.32.fc43.x86_64 #1 PREEMPT(lazy) 
>   [  139.745698] Hardware name: Amazon EC2 c5.large/, BIOS 1.0 10/16/2017
>   [  139.745699] Call Trace:
>   [  139.745700]  <TASK>
>   [  139.745701]  dump_stack_lvl+0x5d/0x80
>   [  139.745706]  ubsan_epilogue+0x5/0x2b
>   [  139.745709]  __ubsan_handle_out_of_bounds.cold+0x54/0x59
>   [  139.745711]  crash_setup_memmap_entries+0x2d9/0x330
>   [  139.745716]  setup_boot_parameters+0xf8/0x6a0
>   [  139.745720]  bzImage64_load+0x41b/0x4e0
>   [  139.745722]  ? find_next_iomem_res+0x109/0x140
>   [  139.745727]  ? locate_mem_hole_callback+0x109/0x170
>   [  139.745737]  kimage_file_alloc_init+0x1ef/0x3e0
>   [  139.745740]  __do_sys_kexec_file_load+0x180/0x2f0
>   [  139.745742]  do_syscall_64+0x7b/0x160
>   [  139.745745]  ? do_user_addr_fault+0x21a/0x690
>   [  139.745747]  ? exc_page_fault+0x7e/0x1a0
>   [  139.745749]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
>   [  139.745751] RIP: 0033:0x7f7712c84e4d
> 

Do we know why this has appeared at such a late date?  The reporter
must be doing something rare.

Baoquan, please re-review this?

A -stable backport is clearly required.  A Fixes: would be nice, but I
assume this goes back a long time so it isn't worth spending a lot of
time working out when this was introduced.

The patch needed a bit of work to apply to current code.  I did the
below.  It compiles.

--- a/arch/x86/kernel/crash.c~x86-kexec-fix-potential-cmem-ranges-out-of-bounds
+++ a/arch/x86/kernel/crash.c
@@ -165,8 +165,18 @@ static struct crash_mem *fill_up_crash_e
 	/*
 	 * Exclusion of crash region and/or crashk_low_res may cause
 	 * another range split. So add extra two slots here.
+	 *
+	 * Exclusion of low 1M may not cause another range split, because the
+	 * range of exclude is [0, 1M] and the condition for splitting a new
+	 * region is that the start, end parameters are both in a certain
+	 * existing region in cmem and cannot be equal to existing region's
+	 * start or end. Obviously, the start of [0, 1M] cannot meet this
+	 * condition.
+	 *
+	 * But in order to lest the low 1M could be changed in the future,
+	 * (e.g. [stare, 1M]), add a extra slot.
 	 */
-	nr_ranges += 2;
+	nr_ranges += 3;
 	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
 	if (!cmem)
 		return NULL;
@@ -317,9 +327,16 @@ int crash_setup_memmap_entries(struct ki
 	 * split. So use two slots here.
 	 */
 	nr_ranges = 2;
-	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
+	/*
+	 * In the current x86 architecture code, the elfheader is always
+	 * allocated at crashk_res.start. But it depends on the allocation
+	 * position of elfheader in crashk_res. To avoid potential out of
+	 * bounds in future, add a extra slot.
+	 */
+	cmem = vzalloc(struct_size(cmem, ranges, 2));
 	if (!cmem)
 		return -ENOMEM;
+	cmem->max_nr_ranges = 2;
 
 	cmem->max_nr_ranges = nr_ranges;
 	cmem->nr_ranges = 0;
_

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Baoquan He]

On 05/07/25 at 10:59pm, Andrew Morton wrote:
> On Thu, 8 May 2025 12:25:15 +0800 Coiby Xu <coxu@redhat.com> wrote:
> 
> > >
> > >Acked-by: Baoquan He <bhe@redhat.com>
> > 
> > Hi Andrew,
> > 
> > It seems this patch was missed.
> 
> January 2024.  Yes, it's fair to assume that it was missed ;)
> 
> > Will you pick it up?
> 
> Sure.
> 
> > Without this patch,
> > kdump kernel will fail to be loaded by the kexec_file_load,
> > 
> >   [  139.736948] UBSAN: array-index-out-of-bounds in arch/x86/kernel/crash.c:350:25
> >   [  139.742360] index 0 is out of range for type 'range [*]'
> >   [  139.745695] CPU: 0 UID: 0 PID: 5778 Comm: kexec Not tainted 6.15.0-0.rc3.20250425git02ddfb981de8.32.fc43.x86_64 #1 PREEMPT(lazy) 
> >   [  139.745698] Hardware name: Amazon EC2 c5.large/, BIOS 1.0 10/16/2017
> >   [  139.745699] Call Trace:
> >   [  139.745700]  <TASK>
> >   [  139.745701]  dump_stack_lvl+0x5d/0x80
> >   [  139.745706]  ubsan_epilogue+0x5/0x2b
> >   [  139.745709]  __ubsan_handle_out_of_bounds.cold+0x54/0x59
> >   [  139.745711]  crash_setup_memmap_entries+0x2d9/0x330
> >   [  139.745716]  setup_boot_parameters+0xf8/0x6a0
> >   [  139.745720]  bzImage64_load+0x41b/0x4e0
> >   [  139.745722]  ? find_next_iomem_res+0x109/0x140
> >   [  139.745727]  ? locate_mem_hole_callback+0x109/0x170
> >   [  139.745737]  kimage_file_alloc_init+0x1ef/0x3e0
> >   [  139.745740]  __do_sys_kexec_file_load+0x180/0x2f0
> >   [  139.745742]  do_syscall_64+0x7b/0x160
> >   [  139.745745]  ? do_user_addr_fault+0x21a/0x690
> >   [  139.745747]  ? exc_page_fault+0x7e/0x1a0
> >   [  139.745749]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> >   [  139.745751] RIP: 0033:0x7f7712c84e4d
> > 
> 
> Do we know why this has appeared at such a late date?  The reporter
> must be doing something rare.
> 
> Baoquan, please re-review this?
> 
> A -stable backport is clearly required.  A Fixes: would be nice, but I
> assume this goes back a long time so it isn't worth spending a lot of
> time working out when this was introduced.

No need for stable kernel. The UBSAN only warns a potential risk, it
won't happen in reality. I am talking to Coiby, he got it wrong about
the testing result. He saw the UBSAN warning when UBSAN is enabled,
while vmcore is till saved successfully.

> 
> The patch needed a bit of work to apply to current code.  I did the
> below.  It compiles.
> 
> --- a/arch/x86/kernel/crash.c~x86-kexec-fix-potential-cmem-ranges-out-of-bounds
> +++ a/arch/x86/kernel/crash.c
> @@ -165,8 +165,18 @@ static struct crash_mem *fill_up_crash_e
>  	/*
>  	 * Exclusion of crash region and/or crashk_low_res may cause
>  	 * another range split. So add extra two slots here.
> +	 *
> +	 * Exclusion of low 1M may not cause another range split, because the
> +	 * range of exclude is [0, 1M] and the condition for splitting a new
> +	 * region is that the start, end parameters are both in a certain
> +	 * existing region in cmem and cannot be equal to existing region's
> +	 * start or end. Obviously, the start of [0, 1M] cannot meet this
> +	 * condition.
> +	 *
> +	 * But in order to lest the low 1M could be changed in the future,
> +	 * (e.g. [stare, 1M]), add a extra slot.
>  	 */
> -	nr_ranges += 2;
> +	nr_ranges += 3;
>  	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
>  	if (!cmem)
>  		return NULL;
> @@ -317,9 +327,16 @@ int crash_setup_memmap_entries(struct ki
>  	 * split. So use two slots here.
>  	 */
>  	nr_ranges = 2;
> -	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
> +	/*
> +	 * In the current x86 architecture code, the elfheader is always
> +	 * allocated at crashk_res.start. But it depends on the allocation
> +	 * position of elfheader in crashk_res. To avoid potential out of
> +	 * bounds in future, add a extra slot.
> +	 */
> +	cmem = vzalloc(struct_size(cmem, ranges, 2));
>  	if (!cmem)
>  		return -ENOMEM;
> +	cmem->max_nr_ranges = 2;
>  
>  	cmem->max_nr_ranges = nr_ranges;
>  	cmem->nr_ranges = 0;
> _
> 

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Baoquan He]

On 05/07/25 at 10:59pm, Andrew Morton wrote:
> On Thu, 8 May 2025 12:25:15 +0800 Coiby Xu <coxu@redhat.com> wrote:
> 
> > >
> > >Acked-by: Baoquan He <bhe@redhat.com>
> > 
> > Hi Andrew,
> > 
> > It seems this patch was missed.
> 
> January 2024.  Yes, it's fair to assume that it was missed ;)
> 
> > Will you pick it up?
> 
> Sure.
> 
> > Without this patch,
> > kdump kernel will fail to be loaded by the kexec_file_load,
> > 
> >   [  139.736948] UBSAN: array-index-out-of-bounds in arch/x86/kernel/crash.c:350:25
> >   [  139.742360] index 0 is out of range for type 'range [*]'
> >   [  139.745695] CPU: 0 UID: 0 PID: 5778 Comm: kexec Not tainted 6.15.0-0.rc3.20250425git02ddfb981de8.32.fc43.x86_64 #1 PREEMPT(lazy) 
> >   [  139.745698] Hardware name: Amazon EC2 c5.large/, BIOS 1.0 10/16/2017
> >   [  139.745699] Call Trace:
> >   [  139.745700]  <TASK>
> >   [  139.745701]  dump_stack_lvl+0x5d/0x80
> >   [  139.745706]  ubsan_epilogue+0x5/0x2b
> >   [  139.745709]  __ubsan_handle_out_of_bounds.cold+0x54/0x59
> >   [  139.745711]  crash_setup_memmap_entries+0x2d9/0x330
> >   [  139.745716]  setup_boot_parameters+0xf8/0x6a0
> >   [  139.745720]  bzImage64_load+0x41b/0x4e0
> >   [  139.745722]  ? find_next_iomem_res+0x109/0x140
> >   [  139.745727]  ? locate_mem_hole_callback+0x109/0x170
> >   [  139.745737]  kimage_file_alloc_init+0x1ef/0x3e0
> >   [  139.745740]  __do_sys_kexec_file_load+0x180/0x2f0
> >   [  139.745742]  do_syscall_64+0x7b/0x160
> >   [  139.745745]  ? do_user_addr_fault+0x21a/0x690
> >   [  139.745747]  ? exc_page_fault+0x7e/0x1a0
> >   [  139.745749]  entry_SYSCALL_64_after_hwframe+0x76/0x7e
> >   [  139.745751] RIP: 0033:0x7f7712c84e4d
> > 
> 
> Do we know why this has appeared at such a late date?  The reporter
> must be doing something rare.
> 
> Baoquan, please re-review this?
> 
> A -stable backport is clearly required.  A Fixes: would be nice, but I
> assume this goes back a long time so it isn't worth spending a lot of
> time working out when this was introduced.
> 
> The patch needed a bit of work to apply to current code.  I did the
> below.  It compiles.

The 2nd hunk is not so good, it discard one slot adding. I made a new
version and will reply to Fuqiang's patch, please help check.

> 
> --- a/arch/x86/kernel/crash.c~x86-kexec-fix-potential-cmem-ranges-out-of-bounds
> +++ a/arch/x86/kernel/crash.c
> @@ -165,8 +165,18 @@ static struct crash_mem *fill_up_crash_e
>  	/*
>  	 * Exclusion of crash region and/or crashk_low_res may cause
>  	 * another range split. So add extra two slots here.
> +	 *
> +	 * Exclusion of low 1M may not cause another range split, because the
> +	 * range of exclude is [0, 1M] and the condition for splitting a new
> +	 * region is that the start, end parameters are both in a certain
> +	 * existing region in cmem and cannot be equal to existing region's
> +	 * start or end. Obviously, the start of [0, 1M] cannot meet this
> +	 * condition.
> +	 *
> +	 * But in order to lest the low 1M could be changed in the future,
> +	 * (e.g. [stare, 1M]), add a extra slot.
>  	 */
> -	nr_ranges += 2;
> +	nr_ranges += 3;
>  	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
>  	if (!cmem)
>  		return NULL;
> @@ -317,9 +327,16 @@ int crash_setup_memmap_entries(struct ki
>  	 * split. So use two slots here.
>  	 */
>  	nr_ranges = 2;
> -	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
> +	/*
> +	 * In the current x86 architecture code, the elfheader is always
> +	 * allocated at crashk_res.start. But it depends on the allocation
> +	 * position of elfheader in crashk_res. To avoid potential out of
> +	 * bounds in future, add a extra slot.
> +	 */
> +	cmem = vzalloc(struct_size(cmem, ranges, 2));
>  	if (!cmem)
>  		return -ENOMEM;
> +	cmem->max_nr_ranges = 2;
>  
>  	cmem->max_nr_ranges = nr_ranges;
>  	cmem->nr_ranges = 0;
> _
> 

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Coiby Xu]

On Wed, May 07, 2025 at 10:59:59PM -0700, Andrew Morton wrote:
>On Thu, 8 May 2025 12:25:15 +0800 Coiby Xu <coxu@redhat.com> wrote:
>
>> >
>> >Acked-by: Baoquan He <bhe@redhat.com>
>>
>> Hi Andrew,
>>
>> It seems this patch was missed.
>
>January 2024.  Yes, it's fair to assume that it was missed ;)
>
>> Will you pick it up?
>
>Sure.

Thanks for quickly processing this patch! Sorry I didn't reply yesterday
as I was trying to reproduce the UBSAN warning and truly understand the
it.

>
>> Without this patch,
>> kdump kernel will fail to be loaded by the kexec_file_load,

As already pointed out by Baoquan, a manual test shows kexec_file_load
actually works despite the UBSAN warning. Sorry I misinterpreted the
UBSAN warning and the automated test result failure (somehow sysrq
wasn't be triggered and vmcore wasn't saved either).


>>
>>   [  139.736948] UBSAN: array-index-out-of-bounds in arch/x86/kernel/crash.c:350:25
>>   [  139.742360] index 0 is out of range for type 'range [*]'
[...]
>>
>
>Do we know why this has appeared at such a late date?  The reporter
>must be doing something rare.

The UBSAN warning happens because flexible array members annotated with
__counted_by are accessed without assigning an array element count i.e.
crash_mem->ranges[0] is accessed without setting max_nr_ranges after
vzalloc,

     // include/linux/crash_core.h
     struct crash_mem {
     	unsigned int max_nr_ranges;
     	unsigned int nr_ranges;
     	struct range ranges[] __counted_by(max_nr_ranges);
     };

The bad commit was introduced in 2021 but only recent gcc-15 supports
__counted_by. That's why we don't see this UBSAN warning until this
year. And although this UBSAN warning is scary enough, fortunately it
doesn't cause a real problem. 

>
>Baoquan, please re-review this?
>
>A -stable backport is clearly required.  A Fixes: would be nice, but I
>assume this goes back a long time so it isn't worth spending a lot of
>time working out when this was introduced.

So I believe the correct fix should be as follows,

--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -301,6 +301,7 @@ int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
         cmem = vzalloc(struct_size(cmem, ranges, 1));
         if (!cmem)
                 return -ENOMEM;
+       cmem->max_nr_ranges = 1;
  
         memset(&cmd, 0, sizeof(struct crash_memmap_data));
         cmd.params = params;


And a Fixes tag should be dedicated to commit
5849cdf8c120 ("x86/crash: Fix crash_setup_memmap_entries() out-of-bounds access")
which forgot to set cmem->max_nr_ranges=1.

>
>The patch needed a bit of work to apply to current code.  I did the
>below.  It compiles.
>
>--- a/arch/x86/kernel/crash.c~x86-kexec-fix-potential-cmem-ranges-out-of-bounds
>+++ a/arch/x86/kernel/crash.c
>@@ -165,8 +165,18 @@ static struct crash_mem *fill_up_crash_e
> 	/*
> 	 * Exclusion of crash region and/or crashk_low_res may cause
> 	 * another range split. So add extra two slots here.
>+	 *
>+	 * Exclusion of low 1M may not cause another range split, because the
>+	 * range of exclude is [0, 1M] and the condition for splitting a new
>+	 * region is that the start, end parameters are both in a certain
>+	 * existing region in cmem and cannot be equal to existing region's
>+	 * start or end. Obviously, the start of [0, 1M] cannot meet this
>+	 * condition.
>+	 *
>+	 * But in order to lest the low 1M could be changed in the future,
>+	 * (e.g. [stare, 1M]), add a extra slot.
> 	 */
>-	nr_ranges += 2;
>+	nr_ranges += 3;
> 	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
> 	if (!cmem)
> 		return NULL;
>@@ -317,9 +327,16 @@ int crash_setup_memmap_entries(struct ki
> 	 * split. So use two slots here.
> 	 */
> 	nr_ranges = 2;
>-	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
>+	/*
>+	 * In the current x86 architecture code, the elfheader is always
>+	 * allocated at crashk_res.start. But it depends on the allocation
>+	 * position of elfheader in crashk_res. To avoid potential out of
>+	 * bounds in future, add a extra slot.
>+	 */
>+	cmem = vzalloc(struct_size(cmem, ranges, 2));
> 	if (!cmem)
> 		return -ENOMEM;
>+	cmem->max_nr_ranges = 2;

Thanks for coming up with the above patch! I think the goal of this
patch is addressing a different issue but it also fixes the UBSAN
warning because cmem->max_nr_ranges is now set.

>
> 	cmem->max_nr_ranges = nr_ranges;
> 	cmem->nr_ranges = 0;
>_


-- 
Best regards,
Coiby

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Baoquan He]

On 05/09/25 at 12:04pm, Coiby Xu wrote:
> On Wed, May 07, 2025 at 10:59:59PM -0700, Andrew Morton wrote:
> > On Thu, 8 May 2025 12:25:15 +0800 Coiby Xu <coxu@redhat.com> wrote:
> > 
> > > >
> > > >Acked-by: Baoquan He <bhe@redhat.com>
> > > 
> > > Hi Andrew,
> > > 
> > > It seems this patch was missed.
> > 
> > January 2024.  Yes, it's fair to assume that it was missed ;)
> > 
> > > Will you pick it up?
> > 
> > Sure.
> 
> Thanks for quickly processing this patch! Sorry I didn't reply yesterday
> as I was trying to reproduce the UBSAN warning and truly understand the
> it.
> 
> > 
> > > Without this patch,
> > > kdump kernel will fail to be loaded by the kexec_file_load,
> 
> As already pointed out by Baoquan, a manual test shows kexec_file_load
> actually works despite the UBSAN warning. Sorry I misinterpreted the
> UBSAN warning and the automated test result failure (somehow sysrq
> wasn't be triggered and vmcore wasn't saved either).
> 
> 
> > > 
> > >   [  139.736948] UBSAN: array-index-out-of-bounds in arch/x86/kernel/crash.c:350:25
> > >   [  139.742360] index 0 is out of range for type 'range [*]'
> [...]
> > > 
> > 
> > Do we know why this has appeared at such a late date?  The reporter
> > must be doing something rare.
> 
> The UBSAN warning happens because flexible array members annotated with
> __counted_by are accessed without assigning an array element count i.e.
> crash_mem->ranges[0] is accessed without setting max_nr_ranges after
> vzalloc,
> 
>     // include/linux/crash_core.h
>     struct crash_mem {
>     	unsigned int max_nr_ranges;
>     	unsigned int nr_ranges;
>     	struct range ranges[] __counted_by(max_nr_ranges);
>     };
> 
> The bad commit was introduced in 2021 but only recent gcc-15 supports
> __counted_by. That's why we don't see this UBSAN warning until this
> year. And although this UBSAN warning is scary enough, fortunately it
> doesn't cause a real problem.
> 
> > 
> > Baoquan, please re-review this?
> > 
> > A -stable backport is clearly required.  A Fixes: would be nice, but I
> > assume this goes back a long time so it isn't worth spending a lot of
> > time working out when this was introduced.
> 
> So I believe the correct fix should be as follows,

Thanks for testing and investigation into these. Could you arrange this
into formal patches based on your testing and analysis? 

It would be great if you can include Fuqiang's patch since it has
conflict with your LUKS patch. This can facilitate patch merging for
Andrew. Thanks in advance.

> 
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -301,6 +301,7 @@ int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
>         cmem = vzalloc(struct_size(cmem, ranges, 1));
>         if (!cmem)
>                 return -ENOMEM;
> +       cmem->max_nr_ranges = 1;
>         memset(&cmd, 0, sizeof(struct crash_memmap_data));
>         cmd.params = params;
> 
> 
> And a Fixes tag should be dedicated to commit
> 5849cdf8c120 ("x86/crash: Fix crash_setup_memmap_entries() out-of-bounds access")
> which forgot to set cmem->max_nr_ranges=1.
> 
> > 
> > The patch needed a bit of work to apply to current code.  I did the
> > below.  It compiles.
> > 
> > --- a/arch/x86/kernel/crash.c~x86-kexec-fix-potential-cmem-ranges-out-of-bounds
> > +++ a/arch/x86/kernel/crash.c
> > @@ -165,8 +165,18 @@ static struct crash_mem *fill_up_crash_e
> > 	/*
> > 	 * Exclusion of crash region and/or crashk_low_res may cause
> > 	 * another range split. So add extra two slots here.
> > +	 *
> > +	 * Exclusion of low 1M may not cause another range split, because the
> > +	 * range of exclude is [0, 1M] and the condition for splitting a new
> > +	 * region is that the start, end parameters are both in a certain
> > +	 * existing region in cmem and cannot be equal to existing region's
> > +	 * start or end. Obviously, the start of [0, 1M] cannot meet this
> > +	 * condition.
> > +	 *
> > +	 * But in order to lest the low 1M could be changed in the future,
> > +	 * (e.g. [stare, 1M]), add a extra slot.
> > 	 */
> > -	nr_ranges += 2;
> > +	nr_ranges += 3;
> > 	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
> > 	if (!cmem)
> > 		return NULL;
> > @@ -317,9 +327,16 @@ int crash_setup_memmap_entries(struct ki
> > 	 * split. So use two slots here.
> > 	 */
> > 	nr_ranges = 2;
> > -	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
> > +	/*
> > +	 * In the current x86 architecture code, the elfheader is always
> > +	 * allocated at crashk_res.start. But it depends on the allocation
> > +	 * position of elfheader in crashk_res. To avoid potential out of
> > +	 * bounds in future, add a extra slot.
> > +	 */
> > +	cmem = vzalloc(struct_size(cmem, ranges, 2));
> > 	if (!cmem)
> > 		return -ENOMEM;
> > +	cmem->max_nr_ranges = 2;
> 
> Thanks for coming up with the above patch! I think the goal of this
> patch is addressing a different issue but it also fixes the UBSAN
> warning because cmem->max_nr_ranges is now set.
> 
> > 
> > 	cmem->max_nr_ranges = nr_ranges;
> > 	cmem->nr_ranges = 0;
> > _
> 
> 
> -- 
> Best regards,
> Coiby
> 

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Andrew Morton]

On Fri, 9 May 2025 17:58:01 +0800 Baoquan He <bhe@redhat.com> wrote:

> > The bad commit was introduced in 2021 but only recent gcc-15 supports
> > __counted_by. That's why we don't see this UBSAN warning until this
> > year. And although this UBSAN warning is scary enough, fortunately it
> > doesn't cause a real problem.
> > 
> > > 
> > > Baoquan, please re-review this?
> > > 
> > > A -stable backport is clearly required.  A Fixes: would be nice, but I
> > > assume this goes back a long time so it isn't worth spending a lot of
> > > time working out when this was introduced.
> > 
> > So I believe the correct fix should be as follows,
> 
> Thanks for testing and investigation into these. Could you arrange this
> into formal patches based on your testing and analysis? 
> 
> It would be great if you can include Fuqiang's patch since it has
> conflict with your LUKS patch. This can facilitate patch merging for
> Andrew. Thanks in advance.

Yes please, I'm a bit lost here. 
x86-kexec-fix-potential-cmem-ranges-out-of-bounds.patch is not
presently in mm.git and I'd appreciate clarity on how to resolve the
conflicts which a new version of
x86-kexec-fix-potential-cmem-ranges-out-of-bounds.patch will produce.

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Coiby Xu]

On Fri, May 09, 2025 at 06:35:18PM -0700, Andrew Morton wrote:
>On Fri, 9 May 2025 17:58:01 +0800 Baoquan He <bhe@redhat.com> wrote:
>
>> > The bad commit was introduced in 2021 but only recent gcc-15 supports
>> > __counted_by. That's why we don't see this UBSAN warning until this
>> > year. And although this UBSAN warning is scary enough, fortunately it
>> > doesn't cause a real problem.
>> >
>> > >
>> > > Baoquan, please re-review this?
>> > >
>> > > A -stable backport is clearly required.  A Fixes: would be nice, but I
>> > > assume this goes back a long time so it isn't worth spending a lot of
>> > > time working out when this was introduced.
>> >
>> > So I believe the correct fix should be as follows,
>>
>> Thanks for testing and investigation into these. Could you arrange this
>> into formal patches based on your testing and analysis?
>>
>> It would be great if you can include Fuqiang's patch since it has
>> conflict with your LUKS patch. This can facilitate patch merging for
>> Andrew. Thanks in advance.
>
>Yes please, I'm a bit lost here.
>x86-kexec-fix-potential-cmem-ranges-out-of-bounds.patch is not
>presently in mm.git and I'd appreciate clarity on how to resolve the
>conflicts which a new version of
>x86-kexec-fix-potential-cmem-ranges-out-of-bounds.patch will produce.

I'll resolve any conflict between these patches. Before that, I'm not 
sure if a separate patch to fix the UBSAN warnings alone is needed to Cc
stable@vger.kernel.org because 
1) the UBSAN warnings don't mean there is a real problem;
2) both Fuqiang's patch and my kdump LUKS support patches fix the UBSAN
warnings as a by-product.

It seems the answer largely depends on if the stable tree or longterm
trees need it. Currently, only longterm tree 6.12.28 and the stable tree
6.14.6 have the UBSAN warnings if they are compiled with gcc-15 or
clang-18. Any advice will be appreciated! Thanks!


-- 
Best regards,
Coiby

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Baoquan He]

On 05/11/25 at 10:19am, Coiby Xu wrote:
> On Fri, May 09, 2025 at 06:35:18PM -0700, Andrew Morton wrote:
> > On Fri, 9 May 2025 17:58:01 +0800 Baoquan He <bhe@redhat.com> wrote:
> > 
> > > > The bad commit was introduced in 2021 but only recent gcc-15 supports
> > > > __counted_by. That's why we don't see this UBSAN warning until this
> > > > year. And although this UBSAN warning is scary enough, fortunately it
> > > > doesn't cause a real problem.
> > > >
> > > > >
> > > > > Baoquan, please re-review this?
> > > > >
> > > > > A -stable backport is clearly required.  A Fixes: would be nice, but I
> > > > > assume this goes back a long time so it isn't worth spending a lot of
> > > > > time working out when this was introduced.
> > > >
> > > > So I believe the correct fix should be as follows,
> > > 
> > > Thanks for testing and investigation into these. Could you arrange this
> > > into formal patches based on your testing and analysis?
> > > 
> > > It would be great if you can include Fuqiang's patch since it has
> > > conflict with your LUKS patch. This can facilitate patch merging for
> > > Andrew. Thanks in advance.
> > 
> > Yes please, I'm a bit lost here.
> > x86-kexec-fix-potential-cmem-ranges-out-of-bounds.patch is not
> > presently in mm.git and I'd appreciate clarity on how to resolve the
> > conflicts which a new version of
> > x86-kexec-fix-potential-cmem-ranges-out-of-bounds.patch will produce.
> 
> I'll resolve any conflict between these patches. Before that, I'm not sure
> if a separate patch to fix the UBSAN warnings alone is needed to Cc
> stable@vger.kernel.org because 1) the UBSAN warnings don't mean there is a
> real problem;
> 2) both Fuqiang's patch and my kdump LUKS support patches fix the UBSAN
> warnings as a by-product.
> 
> It seems the answer largely depends on if the stable tree or longterm
> trees need it. Currently, only longterm tree 6.12.28 and the stable tree
> 6.14.6 have the UBSAN warnings if they are compiled with gcc-15 or
> clang-18. Any advice will be appreciated! Thanks!

I personally think UBSAN warning fix is not necessary for stable kernel.

Hi Kees, Andrew,

Could you help answer Coiby's question about whether we need post a
standalone patch to fix the UBSAN warning fix so that it can be back
ported to stable kernel?

In the case exposed during reviewing this patch, the code UBSAN warned
is not risky.

Thanks
Baoquan

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Kees Cook]

On Fri, May 16, 2025 at 11:35:12AM +0800, Baoquan He wrote:
> On 05/11/25 at 10:19am, Coiby Xu wrote:
> > On Fri, May 09, 2025 at 06:35:18PM -0700, Andrew Morton wrote:
> > > On Fri, 9 May 2025 17:58:01 +0800 Baoquan He <bhe@redhat.com> wrote:
> > > 
> > > > > The bad commit was introduced in 2021 but only recent gcc-15 supports
> > > > > __counted_by. That's why we don't see this UBSAN warning until this
> > > > > year. And although this UBSAN warning is scary enough, fortunately it
> > > > > doesn't cause a real problem.
> > > > >
> > > > > >
> > > > > > Baoquan, please re-review this?
> > > > > >
> > > > > > A -stable backport is clearly required.  A Fixes: would be nice, but I
> > > > > > assume this goes back a long time so it isn't worth spending a lot of
> > > > > > time working out when this was introduced.
> > > > >
> > > > > So I believe the correct fix should be as follows,
> > > > 
> > > > Thanks for testing and investigation into these. Could you arrange this
> > > > into formal patches based on your testing and analysis?
> > > > 
> > > > It would be great if you can include Fuqiang's patch since it has
> > > > conflict with your LUKS patch. This can facilitate patch merging for
> > > > Andrew. Thanks in advance.
> > > 
> > > Yes please, I'm a bit lost here.
> > > x86-kexec-fix-potential-cmem-ranges-out-of-bounds.patch is not
> > > presently in mm.git and I'd appreciate clarity on how to resolve the
> > > conflicts which a new version of
> > > x86-kexec-fix-potential-cmem-ranges-out-of-bounds.patch will produce.
> > 
> > I'll resolve any conflict between these patches. Before that, I'm not sure
> > if a separate patch to fix the UBSAN warnings alone is needed to Cc
> > stable@vger.kernel.org because 1) the UBSAN warnings don't mean there is a
> > real problem;
> > 2) both Fuqiang's patch and my kdump LUKS support patches fix the UBSAN
> > warnings as a by-product.
> > 
> > It seems the answer largely depends on if the stable tree or longterm
> > trees need it. Currently, only longterm tree 6.12.28 and the stable tree
> > 6.14.6 have the UBSAN warnings if they are compiled with gcc-15 or
> > clang-18. Any advice will be appreciated! Thanks!
> 
> I personally think UBSAN warning fix is not necessary for stable kernel.
> 
> Hi Kees, Andrew,
> 
> Could you help answer Coiby's question about whether we need post a
> standalone patch to fix the UBSAN warning fix so that it can be back
> ported to stable kernel?

I went back through the thread and the referenced threads and I can't
find any details on the USBAN splat. Can that please get reproduced in a
commit log? That would help understand if it's a false positive or not.

Also, referencing the commit would be good. I assume this is discussing
commit 15fcedd43a08 ("kexec: Annotate struct crash_mem with __counted_by")?

> In the case exposed during reviewing this patch, the code UBSAN warned
> is not risky.

Given that this makes things work correctly with newer compilers, I
would say it should be backported to whatever -stable kernels have the
"counted_by" annotation. (Hence the request to add a "Fixes" line so
that it will happen automatically.)

-Kees

-- 
Kees Cook

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Baoquan He]

On 05/16/25 at 04:20pm, Kees Cook wrote:
> On Fri, May 16, 2025 at 11:35:12AM +0800, Baoquan He wrote:
> > On 05/11/25 at 10:19am, Coiby Xu wrote:
> > > On Fri, May 09, 2025 at 06:35:18PM -0700, Andrew Morton wrote:
> > > > On Fri, 9 May 2025 17:58:01 +0800 Baoquan He <bhe@redhat.com> wrote:
> > > > 
> > > > > > The bad commit was introduced in 2021 but only recent gcc-15 supports
> > > > > > __counted_by. That's why we don't see this UBSAN warning until this
> > > > > > year. And although this UBSAN warning is scary enough, fortunately it
> > > > > > doesn't cause a real problem.
> > > > > >
> > > > > > >
> > > > > > > Baoquan, please re-review this?
> > > > > > >
> > > > > > > A -stable backport is clearly required.  A Fixes: would be nice, but I
> > > > > > > assume this goes back a long time so it isn't worth spending a lot of
> > > > > > > time working out when this was introduced.
> > > > > >
> > > > > > So I believe the correct fix should be as follows,
> > > > > 
> > > > > Thanks for testing and investigation into these. Could you arrange this
> > > > > into formal patches based on your testing and analysis?
> > > > > 
> > > > > It would be great if you can include Fuqiang's patch since it has
> > > > > conflict with your LUKS patch. This can facilitate patch merging for
> > > > > Andrew. Thanks in advance.
> > > > 
> > > > Yes please, I'm a bit lost here.
> > > > x86-kexec-fix-potential-cmem-ranges-out-of-bounds.patch is not
> > > > presently in mm.git and I'd appreciate clarity on how to resolve the
> > > > conflicts which a new version of
> > > > x86-kexec-fix-potential-cmem-ranges-out-of-bounds.patch will produce.
> > > 
> > > I'll resolve any conflict between these patches. Before that, I'm not sure
> > > if a separate patch to fix the UBSAN warnings alone is needed to Cc
> > > stable@vger.kernel.org because 1) the UBSAN warnings don't mean there is a
> > > real problem;
> > > 2) both Fuqiang's patch and my kdump LUKS support patches fix the UBSAN
> > > warnings as a by-product.
> > > 
> > > It seems the answer largely depends on if the stable tree or longterm
> > > trees need it. Currently, only longterm tree 6.12.28 and the stable tree
> > > 6.14.6 have the UBSAN warnings if they are compiled with gcc-15 or
> > > clang-18. Any advice will be appreciated! Thanks!
> > 
> > I personally think UBSAN warning fix is not necessary for stable kernel.
> > 
> > Hi Kees, Andrew,
> > 
> > Could you help answer Coiby's question about whether we need post a
> > standalone patch to fix the UBSAN warning fix so that it can be back
> > ported to stable kernel?
> 
> I went back through the thread and the referenced threads and I can't
> find any details on the USBAN splat. Can that please get reproduced in a
> commit log? That would help understand if it's a false positive or not.


The original patch is trying to fix a potential issue in which a memory
range is split, while the sub-range split out is always on top of the
entire memory range, hence no risk.

Later, we encountered a UBSAN warning around the above memory range
splitting code several times. We found this patch can mute the warning.

Please see below UBSAN splat trace report from Coiby:
https://lore.kernel.org/all/4de3c2onosr7negqnfhekm4cpbklzmsimgdfv33c52dktqpza5@z5pb34ghz4at/T/#u

Later, Coiby got the root cause from investigation, please see:
https://lore.kernel.org/all/2754f4evjfumjqome63bc3inqb7ozepemejn2lcl57ryio2t6k@35l3tnn73gei/T/#u

> 
> Also, referencing the commit would be good. I assume this is discussing
> commit 15fcedd43a08 ("kexec: Annotate struct crash_mem with __counted_by")?

Right.

> 
> > In the case exposed during reviewing this patch, the code UBSAN warned
> > is not risky.
> 
> Given that this makes things work correctly with newer compilers, I
> would say it should be backported to whatever -stable kernels have the
> "counted_by" annotation. (Hence the request to add a "Fixes" line so
> that it will happen automatically.)

Got it, then Coiby can post a standalone patch to fix commit 15fcedd43a08
("kexec: Annotate struct crash_mem with __counted_by") and CC stable, then
post a new version of this patch on top.

Thanks a lot for confirming.

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Kees Cook]

On Mon, May 19, 2025 at 09:22:30AM +0800, Baoquan He wrote:
> On 05/16/25 at 04:20pm, Kees Cook wrote:
> > On Fri, May 16, 2025 at 11:35:12AM +0800, Baoquan He wrote:
> > > On 05/11/25 at 10:19am, Coiby Xu wrote:
> > > > On Fri, May 09, 2025 at 06:35:18PM -0700, Andrew Morton wrote:
> > > > > On Fri, 9 May 2025 17:58:01 +0800 Baoquan He <bhe@redhat.com> wrote:
> > > > > 
> > > > > > > The bad commit was introduced in 2021 but only recent gcc-15 supports
> > > > > > > __counted_by. That's why we don't see this UBSAN warning until this
> > > > > > > year. And although this UBSAN warning is scary enough, fortunately it
> > > > > > > doesn't cause a real problem.
> > > > > > >
> > > > > > > >
> > > > > > > > Baoquan, please re-review this?
> > > > > > > >
> > > > > > > > A -stable backport is clearly required.  A Fixes: would be nice, but I
> > > > > > > > assume this goes back a long time so it isn't worth spending a lot of
> > > > > > > > time working out when this was introduced.
> > > > > > >
> > > > > > > So I believe the correct fix should be as follows,
> > > > > > 
> > > > > > Thanks for testing and investigation into these. Could you arrange this
> > > > > > into formal patches based on your testing and analysis?
> > > > > > 
> > > > > > It would be great if you can include Fuqiang's patch since it has
> > > > > > conflict with your LUKS patch. This can facilitate patch merging for
> > > > > > Andrew. Thanks in advance.
> > > > > 
> > > > > Yes please, I'm a bit lost here.
> > > > > x86-kexec-fix-potential-cmem-ranges-out-of-bounds.patch is not
> > > > > presently in mm.git and I'd appreciate clarity on how to resolve the
> > > > > conflicts which a new version of
> > > > > x86-kexec-fix-potential-cmem-ranges-out-of-bounds.patch will produce.
> > > > 
> > > > I'll resolve any conflict between these patches. Before that, I'm not sure
> > > > if a separate patch to fix the UBSAN warnings alone is needed to Cc
> > > > stable@vger.kernel.org because 1) the UBSAN warnings don't mean there is a
> > > > real problem;
> > > > 2) both Fuqiang's patch and my kdump LUKS support patches fix the UBSAN
> > > > warnings as a by-product.
> > > > 
> > > > It seems the answer largely depends on if the stable tree or longterm
> > > > trees need it. Currently, only longterm tree 6.12.28 and the stable tree
> > > > 6.14.6 have the UBSAN warnings if they are compiled with gcc-15 or
> > > > clang-18. Any advice will be appreciated! Thanks!
> > > 
> > > I personally think UBSAN warning fix is not necessary for stable kernel.
> > > 
> > > Hi Kees, Andrew,
> > > 
> > > Could you help answer Coiby's question about whether we need post a
> > > standalone patch to fix the UBSAN warning fix so that it can be back
> > > ported to stable kernel?
> > 
> > I went back through the thread and the referenced threads and I can't
> > find any details on the USBAN splat. Can that please get reproduced in a
> > commit log? That would help understand if it's a false positive or not.
> 
> 
> The original patch is trying to fix a potential issue in which a memory
> range is split, while the sub-range split out is always on top of the
> entire memory range, hence no risk.
> 
> Later, we encountered a UBSAN warning around the above memory range
> splitting code several times. We found this patch can mute the warning.
> 
> Please see below UBSAN splat trace report from Coiby:
> https://lore.kernel.org/all/4de3c2onosr7negqnfhekm4cpbklzmsimgdfv33c52dktqpza5@z5pb34ghz4at/T/#u

Ah-ha! Thanks for the link.

> Later, Coiby got the root cause from investigation, please see:
> https://lore.kernel.org/all/2754f4evjfumjqome63bc3inqb7ozepemejn2lcl57ryio2t6k@35l3tnn73gei/T/#u

Looking at https://lore.kernel.org/all/aBxfflkkQXTetmbq@MiWiFi-R3L-srv/
it seems like this actually turned out to be a legitimate overflow
detection? I.e. the fix isn't silencing a false positive, but rather
allocating enough space?

-- 
Kees Cook

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Baoquan He]

On 05/19/25 at 07:19am, Kees Cook wrote:
> On Mon, May 19, 2025 at 09:22:30AM +0800, Baoquan He wrote:
> > On 05/16/25 at 04:20pm, Kees Cook wrote:
> > > On Fri, May 16, 2025 at 11:35:12AM +0800, Baoquan He wrote:
> > > > On 05/11/25 at 10:19am, Coiby Xu wrote:
> > > > > On Fri, May 09, 2025 at 06:35:18PM -0700, Andrew Morton wrote:
> > > > > > On Fri, 9 May 2025 17:58:01 +0800 Baoquan He <bhe@redhat.com> wrote:
> > > > > > 
> > > > > > > > The bad commit was introduced in 2021 but only recent gcc-15 supports
> > > > > > > > __counted_by. That's why we don't see this UBSAN warning until this
> > > > > > > > year. And although this UBSAN warning is scary enough, fortunately it
> > > > > > > > doesn't cause a real problem.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > Baoquan, please re-review this?
> > > > > > > > >
> > > > > > > > > A -stable backport is clearly required.  A Fixes: would be nice, but I
> > > > > > > > > assume this goes back a long time so it isn't worth spending a lot of
> > > > > > > > > time working out when this was introduced.
> > > > > > > >
> > > > > > > > So I believe the correct fix should be as follows,
> > > > > > > 
> > > > > > > Thanks for testing and investigation into these. Could you arrange this
> > > > > > > into formal patches based on your testing and analysis?
> > > > > > > 
> > > > > > > It would be great if you can include Fuqiang's patch since it has
> > > > > > > conflict with your LUKS patch. This can facilitate patch merging for
> > > > > > > Andrew. Thanks in advance.
> > > > > > 
> > > > > > Yes please, I'm a bit lost here.
> > > > > > x86-kexec-fix-potential-cmem-ranges-out-of-bounds.patch is not
> > > > > > presently in mm.git and I'd appreciate clarity on how to resolve the
> > > > > > conflicts which a new version of
> > > > > > x86-kexec-fix-potential-cmem-ranges-out-of-bounds.patch will produce.
> > > > > 
> > > > > I'll resolve any conflict between these patches. Before that, I'm not sure
> > > > > if a separate patch to fix the UBSAN warnings alone is needed to Cc
> > > > > stable@vger.kernel.org because 1) the UBSAN warnings don't mean there is a
> > > > > real problem;
> > > > > 2) both Fuqiang's patch and my kdump LUKS support patches fix the UBSAN
> > > > > warnings as a by-product.
> > > > > 
> > > > > It seems the answer largely depends on if the stable tree or longterm
> > > > > trees need it. Currently, only longterm tree 6.12.28 and the stable tree
> > > > > 6.14.6 have the UBSAN warnings if they are compiled with gcc-15 or
> > > > > clang-18. Any advice will be appreciated! Thanks!
> > > > 
> > > > I personally think UBSAN warning fix is not necessary for stable kernel.
> > > > 
> > > > Hi Kees, Andrew,
> > > > 
> > > > Could you help answer Coiby's question about whether we need post a
> > > > standalone patch to fix the UBSAN warning fix so that it can be back
> > > > ported to stable kernel?
> > > 
> > > I went back through the thread and the referenced threads and I can't
> > > find any details on the USBAN splat. Can that please get reproduced in a
> > > commit log? That would help understand if it's a false positive or not.
> > 
> > 
> > The original patch is trying to fix a potential issue in which a memory
> > range is split, while the sub-range split out is always on top of the
> > entire memory range, hence no risk.
> > 
> > Later, we encountered a UBSAN warning around the above memory range
> > splitting code several times. We found this patch can mute the warning.
> > 
> > Please see below UBSAN splat trace report from Coiby:
> > https://lore.kernel.org/all/4de3c2onosr7negqnfhekm4cpbklzmsimgdfv33c52dktqpza5@z5pb34ghz4at/T/#u
> 
> Ah-ha! Thanks for the link.
> 
> > Later, Coiby got the root cause from investigation, please see:
> > https://lore.kernel.org/all/2754f4evjfumjqome63bc3inqb7ozepemejn2lcl57ryio2t6k@35l3tnn73gei/T/#u
> 
> Looking at https://lore.kernel.org/all/aBxfflkkQXTetmbq@MiWiFi-R3L-srv/
> it seems like this actually turned out to be a legitimate overflow
> detection? I.e. the fix isn't silencing a false positive, but rather
> allocating enough space?

This v5 is on top of below patch which Andrew has picked to his mm tree.
In there, it happened to get the ubsan warning fixed. But the hunk
doesn't reflect it in the v5 patch.

[PATCH v9 7/8] x86/crash: pass dm crypt keys to kdump kernel
https://lore.kernel.org/all/20250502011246.99238-8-coxu@redhat.com/T/#u

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Coiby Xu]

On Mon, May 19, 2025 at 10:34:39PM +0800, Baoquan He wrote:
>On 05/19/25 at 07:19am, Kees Cook wrote:
>> On Mon, May 19, 2025 at 09:22:30AM +0800, Baoquan He wrote:
[...]
>> > > I went back through the thread and the referenced threads and I can't
>> > > find any details on the USBAN splat. Can that please get reproduced in a
>> > > commit log? That would help understand if it's a false positive or not.
>> >
>> >
>> > The original patch is trying to fix a potential issue in which a memory
>> > range is split, while the sub-range split out is always on top of the
>> > entire memory range, hence no risk.
>> >
>> > Later, we encountered a UBSAN warning around the above memory range
>> > splitting code several times. We found this patch can mute the warning.
>> >
>> > Please see below UBSAN splat trace report from Coiby:
>> > https://lore.kernel.org/all/4de3c2onosr7negqnfhekm4cpbklzmsimgdfv33c52dktqpza5@z5pb34ghz4at/T/#u
>>
>> Ah-ha! Thanks for the link.
>>
>> > Later, Coiby got the root cause from investigation, please see:
>> > https://lore.kernel.org/all/2754f4evjfumjqome63bc3inqb7ozepemejn2lcl57ryio2t6k@35l3tnn73gei/T/#u
>>
>> Looking at https://lore.kernel.org/all/aBxfflkkQXTetmbq@MiWiFi-R3L-srv/
>> it seems like this actually turned out to be a legitimate overflow
>> detection? I.e. the fix isn't silencing a false positive, but rather
>> allocating enough space?

The words "out of bounds" in the patch subject are kind of misleading
because the patch is outdated. A later merged commit 6dff31597264
("crash_core: fix and simplify the logic of crash_exclude_mem_range()")
has actually fixed out-of-bound access issue as illustrated in
https://lore.kernel.org/kexec/ZXrY7QbXAlxydsSC@MiWiFi-R3L-srv/ 
Current crash_exclude_mem_range simply returns -ENOMEM when there is no
enough space to hold split ranges (I'll post a patch to prove the
correctness of crash_exclude_mem_range by reasoning about the code and
including a thorough unit tests). So I'll change the subject to "fix
potential cmem->ranges out of memory" in the upcoming patch.

>
>This v5 is on top of below patch which Andrew has picked to his mm tree.
>In there, it happened to get the ubsan warning fixed. But the hunk
>doesn't reflect it in the v5 patch.
>
>[PATCH v9 7/8] x86/crash: pass dm crypt keys to kdump kernel
>https://lore.kernel.org/all/20250502011246.99238-8-coxu@redhat.com/T/#u
>

-- 
Best regards,
Coiby

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Coiby Xu]

On Tue, May 20, 2025 at 05:13:28PM +0800, Coiby Xu wrote:
>On Mon, May 19, 2025 at 10:34:39PM +0800, Baoquan He wrote:
>>On 05/19/25 at 07:19am, Kees Cook wrote:
>>>On Mon, May 19, 2025 at 09:22:30AM +0800, Baoquan He wrote:
>[...]
>>>> > I went back through the thread and the referenced threads and I can't
>>>> > find any details on the USBAN splat. Can that please get reproduced in a
>>>> > commit log? That would help understand if it's a false positive or not.
>>>>
>>>>
>>>> The original patch is trying to fix a potential issue in which a memory
>>>> range is split, while the sub-range split out is always on top of the
>>>> entire memory range, hence no risk.
>>>>
>>>> Later, we encountered a UBSAN warning around the above memory range
>>>> splitting code several times. We found this patch can mute the warning.
>>>>
>>>> Please see below UBSAN splat trace report from Coiby:
>>>> https://lore.kernel.org/all/4de3c2onosr7negqnfhekm4cpbklzmsimgdfv33c52dktqpza5@z5pb34ghz4at/T/#u
>>>
>>>Ah-ha! Thanks for the link.
>>>
>>>> Later, Coiby got the root cause from investigation, please see:
>>>> https://lore.kernel.org/all/2754f4evjfumjqome63bc3inqb7ozepemejn2lcl57ryio2t6k@35l3tnn73gei/T/#u
>>>
>>>Looking at https://lore.kernel.org/all/aBxfflkkQXTetmbq@MiWiFi-R3L-srv/
>>>it seems like this actually turned out to be a legitimate overflow
>>>detection? I.e. the fix isn't silencing a false positive, but rather
>>>allocating enough space?
>
>The words "out of bounds" in the patch subject are kind of misleading
>because the patch is outdated. A later merged commit 6dff31597264
>("crash_core: fix and simplify the logic of crash_exclude_mem_range()")
>has actually fixed out-of-bound access issue as illustrated in
>https://lore.kernel.org/kexec/ZXrY7QbXAlxydsSC@MiWiFi-R3L-srv/ Current 
>crash_exclude_mem_range simply returns -ENOMEM when there is no
>enough space to hold split ranges (I'll post a patch to prove the
>correctness of crash_exclude_mem_range by reasoning about the code and
>including a thorough unit tests). So I'll change the subject to "fix
>potential cmem->ranges out of memory" in the upcoming patch.

The kdump LUKS support patches which fix the UBSAN warnings as a
byproduct are now in the Andrew's mm-nonmm-stable tree. Can I assume
it's not appropriate to re-send a new version of kdump LUKS support
patches which includes a separate patch to fix the UBSAN warnings alone?
If that's the case, I can send a single patch to the stable tree if the
stable tree requires an upstream commit already in Linus's tree.

-- 
Best regards,
Coiby

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Coiby Xu]

On Mon, May 19, 2025 at 09:22:30AM +0800, Baoquan He wrote:
>On 05/16/25 at 04:20pm, Kees Cook wrote:
[...]
>> > > I'll resolve any conflict between these patches. Before that, I'm not sure
>> > > if a separate patch to fix the UBSAN warnings alone is needed to Cc
>> > > stable@vger.kernel.org because 1) the UBSAN warnings don't mean there is a
>> > > real problem;
>> > > 2) both Fuqiang's patch and my kdump LUKS support patches fix the UBSAN
>> > > warnings as a by-product.
>> > >
>> > > It seems the answer largely depends on if the stable tree or longterm
>> > > trees need it. Currently, only longterm tree 6.12.28 and the stable tree
>> > > 6.14.6 have the UBSAN warnings if they are compiled with gcc-15 or
>> > > clang-18. Any advice will be appreciated! Thanks!
>> >
>> > I personally think UBSAN warning fix is not necessary for stable kernel.
>> >
>> > Hi Kees, Andrew,
>> >
>> > Could you help answer Coiby's question about whether we need post a
>> > standalone patch to fix the UBSAN warning fix so that it can be back
>> > ported to stable kernel?
>>
>> I went back through the thread and the referenced threads and I can't
>> find any details on the USBAN splat. Can that please get reproduced in a
>> commit log? That would help understand if it's a false positive or not.
>
>
>The original patch is trying to fix a potential issue in which a memory
>range is split, while the sub-range split out is always on top of the
>entire memory range, hence no risk.
>
>Later, we encountered a UBSAN warning around the above memory range
>splitting code several times. We found this patch can mute the warning.
>
>Please see below UBSAN splat trace report from Coiby:
>https://lore.kernel.org/all/4de3c2onosr7negqnfhekm4cpbklzmsimgdfv33c52dktqpza5@z5pb34ghz4at/T/#u
>
>Later, Coiby got the root cause from investigation, please see:
>https://lore.kernel.org/all/2754f4evjfumjqome63bc3inqb7ozepemejn2lcl57ryio2t6k@35l3tnn73gei/T/#u
>
>>
>> Also, referencing the commit would be good. I assume this is discussing
>> commit 15fcedd43a08 ("kexec: Annotate struct crash_mem with __counted_by")?
>
>Right.
>
>>
>> > In the case exposed during reviewing this patch, the code UBSAN warned
>> > is not risky.
>>
>> Given that this makes things work correctly with newer compilers, I
>> would say it should be backported to whatever -stable kernels have the
>> "counted_by" annotation. (Hence the request to add a "Fixes" line so
>> that it will happen automatically.)
>
>Got it, then Coiby can post a standalone patch to fix commit 15fcedd43a08
>("kexec: Annotate struct crash_mem with __counted_by") and CC stable, then
>post a new version of this patch on top.
>
>Thanks a lot for confirming.

Yes, thank Kees for the confirmation and thank Baoquan for providing the
context and links! I'll send a standalone patch referencing
15fcedd43a08. But I don't think commit 15fcedd43a08 itself introduced
any bug so I shouldn't assign a Fixes tag to it. It's commit
5849cdf8c120 ("x86/crash: Fix crash_setup_memmap_entries() out-of-bounds
access") which forgot to set crash_mem->max_nr_ranges.
crash_mem->max_nr_ranges should always be set to make sure
crash_exclude_mem_range will work as expected. If
crash_mem->max_nr_ranges=0, crash_exclude_mem_range will return -ENOMEM
if there a range split. So if there is no objection, I will include 
   Fixes: 5849cdf8c120 ("x86/crash: Fix crash_setup_memmap_entries() out-of-bounds access")

A preview of to-be-sent patch is available via 
https://github.com/torvalds/linux/commit/43c5a68f3d01b2e065cbb8686279224710cba682

-- 
Best regards,
Coiby

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Coiby Xu]

On Fri, May 09, 2025 at 05:58:01PM +0800, Baoquan He wrote:
>On 05/09/25 at 12:04pm, Coiby Xu wrote:
>> On Wed, May 07, 2025 at 10:59:59PM -0700, Andrew Morton wrote:
[...]
>> >
>> > Do we know why this has appeared at such a late date?  The reporter
>> > must be doing something rare.
>>
>> The UBSAN warning happens because flexible array members annotated with
>> __counted_by are accessed without assigning an array element count i.e.
>> crash_mem->ranges[0] is accessed without setting max_nr_ranges after
>> vzalloc,
>>
>>     // include/linux/crash_core.h
>>     struct crash_mem {
>>     	unsigned int max_nr_ranges;
>>     	unsigned int nr_ranges;
>>     	struct range ranges[] __counted_by(max_nr_ranges);
>>     };
>>
>> The bad commit was introduced in 2021 but only recent gcc-15 supports
>> __counted_by. That's why we don't see this UBSAN warning until this
>> year. And although this UBSAN warning is scary enough, fortunately it
>> doesn't cause a real problem.
>>
>> >
>> > Baoquan, please re-review this?
>> >
>> > A -stable backport is clearly required.  A Fixes: would be nice, but I
>> > assume this goes back a long time so it isn't worth spending a lot of
>> > time working out when this was introduced.
>>
>> So I believe the correct fix should be as follows,
>
>Thanks for testing and investigation into these. Could you arrange this
>into formal patches based on your testing and analysis?
>
>It would be great if you can include Fuqiang's patch since it has
>conflict with your LUKS patch. This can facilitate patch merging for
>Andrew. Thanks in advance.

Sure, I'll ask Andrew if a separate patch is needed because both my
Fuqiang's patch and the kdump LUKS support patches fix the UBSAN warning
as a by-product. And I'll resolve any conflict. Meanwhile, I'll need
more time to investigate Fuqiang's patch. For example, because of
commit 6dff31597264 ("crash_core: fix and simplify the logic of
crash_exclude_mem_range()"), cmem->ranges out of bounds doesn't seem to
happen any more because now -ENOMEM will be returned instead.

-- 
Best regards,
Coiby

Re: [PATCH v4] x86/kexec: fix potential cmem->ranges out of bounds

[Andrew Morton]

On Mon,  8 Jan 2024 21:06:47 +0800 fuqiang wang <fuqiang.wang@easystack.cn> wrote:

> In memmap_exclude_ranges(), elfheader will be excluded from crashk_res.
> In the current x86 architecture code, the elfheader is always allocated
> at crashk_res.start. It seems that there won't be a new split range.
> But it depends on the allocation position of elfheader in crashk_res. To
> avoid potential out of bounds in future, add a extra slot.
> 
> The similar issue also exists in fill_up_crash_elf_data(). The range to
> be excluded is [0, 1M], start (0) is special and will not appear in the
> middle of existing cmem->ranges[]. But in cast the low 1M could be
> changed in the future, add a extra slot too.
> 
> Previously discussed link:
> [1] https://lore.kernel.org/kexec/ZXk2oBf%2FT1Ul6o0c@MiWiFi-R3L-srv/
> [2] https://lore.kernel.org/kexec/273284e8-7680-4f5f-8065-c5d780987e59@easystack.cn/
> [3] https://lore.kernel.org/kexec/ZYQ6O%2F57sHAPxTHm@MiWiFi-R3L-srv/

So.

When I merge this ancient fix against mainline, it goes OK.

When I merge Coiby's "x86/crash: pass dm crypt keys to kdump kernel"
on top of this fix,  things do not go OK.

Here is what I did:

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;

	/*
	 * Using random kexec_buf for passing dm crypt keys may cause a range
	 * split. So use two slots here.
	 */
	nr_ranges = 2;

	/*
	 * In the current x86 architecture code, the elfheader is always
	 * allocated at crashk_res.start. But it depends on the allocation
	 * position of elfheader in crashk_res. To avoid potential out of
	 * bounds in future, add a extra slot.
	 */
	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
	if (!cmem)
		return -ENOMEM;

	cmem->max_nr_ranges = nr_ranges;
	cmem->nr_ranges = 0;

	memset(&cmd, 0, sizeof(struct crash_memmap_data));

Please triple check this, I changed a few things.

[PATCH v5] x86/kexec: fix potential cmem->ranges out of bounds

[Baoquan He]

In memmap_exclude_ranges(), elfheader will be excluded from crashk_res.
In the current x86 architecture code, the elfheader is always allocated
at crashk_res.start. It seems that there won't be a new split range.
But it depends on the allocation position of elfheader in crashk_res. To
avoid potential out of bounds in future, add a extra slot. And using
random kexec_buf for passing dm crypt keys may cause a range split too,
add another extra slot here.

The similar issue also exists in fill_up_crash_elf_data(). The range to
be excluded is [0, 1M], start (0) is special and will not appear in the
middle of existing cmem->ranges[]. But in cast the low 1M could be
changed in the future, add a extra slot too.

Previously discussed link:
[1] https://lore.kernel.org/kexec/ZXk2oBf%2FT1Ul6o0c@MiWiFi-R3L-srv/
[2] https://lore.kernel.org/kexec/273284e8-7680-4f5f-8065-c5d780987e59@easystack.cn/
[3] https://lore.kernel.org/kexec/ZYQ6O%2F57sHAPxTHm@MiWiFi-R3L-srv/

Signed-off-by: fuqiang wang <fuqiang.wang@easystack.cn>
Signed-off-by: Baoquan He <bhe@redhat.com>
---
v4->v5:
- This is on top of Coiby's LUKS patchset in branch mm-nonmm-unstable of
  akpm/mm.git. I did some adaption based on Coiby's patches.
- [PATCH v9 0/8] Support kdump with LUKS encryption by reusing LUKS volume keys

 arch/x86/kernel/crash.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index bcb534688dfe..749a60ce8b7f 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -165,8 +165,18 @@ static struct crash_mem *fill_up_crash_elf_data(void)
 	/*
 	 * Exclusion of crash region and/or crashk_low_res may cause
 	 * another range split. So add extra two slots here.
+	 *
+	 * Exclusion of low 1M may not cause another range split, because the
+	 * range of exclude is [0, 1M] and the condition for splitting a new
+	 * region is that the start, end parameters are both in a certain
+	 * existing region in cmem and cannot be equal to existing region's
+	 * start or end. Obviously, the start of [0, 1M] cannot meet this
+	 * condition.
+	 *
+	 * But in order to lest the low 1M could be changed in the future,
+	 * (e.g. [stare, 1M]), add a extra slot.
 	 */
-	nr_ranges += 2;
+	nr_ranges += 3;
 	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
 	if (!cmem)
 		return NULL;
@@ -313,10 +323,15 @@ int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
 	struct crash_mem *cmem;
 
 	/*
-	 * Using random kexec_buf for passing dm crypt keys may cause a range
-	 * split. So use two slots here.
+	 * In the current x86 architecture code, the elfheader is always
+	 * allocated at crashk_res.start. But it depends on the allocation
+	 * position of elfheader in crashk_res. To avoid potential out of
+	 * bounds in future, add an extra slot.
+	 *
+	 * And using random kexec_buf for passing dm crypt keys may cause a
+	 * range split too, add another extra slot here.
 	 */
-	nr_ranges = 2;
+	nr_ranges = 3;
 	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
 	if (!cmem)
 		return -ENOMEM;
-- 
2.41.0

Re: [PATCH v5] x86/kexec: fix potential cmem->ranges out of bounds

[Baoquan He]

On 05/08/25 at 03:38pm, Baoquan He wrote:
> In memmap_exclude_ranges(), elfheader will be excluded from crashk_res.
> In the current x86 architecture code, the elfheader is always allocated
> at crashk_res.start. It seems that there won't be a new split range.
> But it depends on the allocation position of elfheader in crashk_res. To
> avoid potential out of bounds in future, add a extra slot. And using
> random kexec_buf for passing dm crypt keys may cause a range split too,
> add another extra slot here.

Sorry, this should be from fuqiang wang, when I edited the patch to
reply to his patch, I forgot that. Please help makes change to set
Fuqiang as the patch author, I just adapted the content based on Coiby's
patches.

> 
> The similar issue also exists in fill_up_crash_elf_data(). The range to
> be excluded is [0, 1M], start (0) is special and will not appear in the
> middle of existing cmem->ranges[]. But in cast the low 1M could be
> changed in the future, add a extra slot too.
> 
> Previously discussed link:
> [1] https://lore.kernel.org/kexec/ZXk2oBf%2FT1Ul6o0c@MiWiFi-R3L-srv/
> [2] https://lore.kernel.org/kexec/273284e8-7680-4f5f-8065-c5d780987e59@easystack.cn/
> [3] https://lore.kernel.org/kexec/ZYQ6O%2F57sHAPxTHm@MiWiFi-R3L-srv/
> 
> Signed-off-by: fuqiang wang <fuqiang.wang@easystack.cn>
> Signed-off-by: Baoquan He <bhe@redhat.com>
> ---
> v4->v5:
> - This is on top of Coiby's LUKS patchset in branch mm-nonmm-unstable of
>   akpm/mm.git. I did some adaption based on Coiby's patches.
> - [PATCH v9 0/8] Support kdump with LUKS encryption by reusing LUKS volume keys
> 
>  arch/x86/kernel/crash.c | 23 +++++++++++++++++++----
>  1 file changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index bcb534688dfe..749a60ce8b7f 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -165,8 +165,18 @@ static struct crash_mem *fill_up_crash_elf_data(void)
>  	/*
>  	 * Exclusion of crash region and/or crashk_low_res may cause
>  	 * another range split. So add extra two slots here.
> +	 *
> +	 * Exclusion of low 1M may not cause another range split, because the
> +	 * range of exclude is [0, 1M] and the condition for splitting a new
> +	 * region is that the start, end parameters are both in a certain
> +	 * existing region in cmem and cannot be equal to existing region's
> +	 * start or end. Obviously, the start of [0, 1M] cannot meet this
> +	 * condition.
> +	 *
> +	 * But in order to lest the low 1M could be changed in the future,
> +	 * (e.g. [stare, 1M]), add a extra slot.
>  	 */
> -	nr_ranges += 2;
> +	nr_ranges += 3;
>  	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
>  	if (!cmem)
>  		return NULL;
> @@ -313,10 +323,15 @@ int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
>  	struct crash_mem *cmem;
>  
>  	/*
> -	 * Using random kexec_buf for passing dm crypt keys may cause a range
> -	 * split. So use two slots here.
> +	 * In the current x86 architecture code, the elfheader is always
> +	 * allocated at crashk_res.start. But it depends on the allocation
> +	 * position of elfheader in crashk_res. To avoid potential out of
> +	 * bounds in future, add an extra slot.
> +	 *
> +	 * And using random kexec_buf for passing dm crypt keys may cause a
> +	 * range split too, add another extra slot here.
>  	 */
> -	nr_ranges = 2;
> +	nr_ranges = 3;
>  	cmem = vzalloc(struct_size(cmem, ranges, nr_ranges));
>  	if (!cmem)
>  		return -ENOMEM;
> -- 
> 2.41.0
>