[PATCH v2 0/3] powerpc/kexec_file: Fix some bugs

[PATCH v2 0/3] powerpc/kexec_file: Fix some bugs

[Jinjie Ruan]

Fix some powerpc bugs pointed out by Sashiko AI:
- Memory leak
- Null-ptr-def
- Overlapping memory range truncation .

Link: https://lore.kernel.org/all/20260601094805.2928614-1-ruanjinjie@huawei.com/

Changes in v2:
- Split Powerpc bugfix patch into this separate patch set.
- v1: https://lore.kernel.org/all/20260723131242.1537633-1-ruanjinjie@huawei.com/#t

Jinjie Ruan (3):
  powerpc/crash: Fix possible memory leak in update_crash_elfcorehdr()
  powerpc/kexec_file: Fix null-ptr-def in extra size calculation
  powerpc/kexec_file: Prevent kexec range truncation

 arch/powerpc/kexec/crash.c        |  2 +-
 arch/powerpc/kexec/file_load_64.c |  2 +-
 arch/powerpc/kexec/ranges.c       | 12 +++++-------
 3 files changed, 7 insertions(+), 9 deletions(-)

-- 
2.34.1

[PATCH v2 1/3] powerpc/crash: Fix possible memory leak in update_crash_elfcorehdr()

[Jinjie Ruan]

In get_crash_memory_ranges(), if crash_exclude_mem_range() failed
after realloc_mem_ranges() has successfully allocated the cmem
memory, it just returns an error but leaves cmem pointing to
the allocated memory, nor is it freed in the caller
update_crash_elfcorehdr(), which cause a memory leak, goto out
to free the cmem.

Cc: Sourabh Jain <sourabhjain@linux.ibm.com>
Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Fixes: 849599b702ef ("powerpc/crash: add crash memory hotplug support")
Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 arch/powerpc/kexec/crash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kexec/crash.c b/arch/powerpc/kexec/crash.c
index 60a917a6beaa..775895f31037 100644
--- a/arch/powerpc/kexec/crash.c
+++ b/arch/powerpc/kexec/crash.c
@@ -502,7 +502,7 @@ static void update_crash_elfcorehdr(struct kimage *image, struct memory_notify *
 	ret = get_crash_memory_ranges(&cmem);
 	if (ret) {
 		pr_err("Failed to get crash mem range\n");
-		return;
+		goto out;
 	}
 
 	/*
-- 
2.34.1

[PATCH v2 2/3] powerpc/kexec_file: Fix null-ptr-def in extra size calculation

[Jinjie Ruan]

A static Sashiko AI review identified a potential NULL pointer
dereference in kexec_extra_fdt_size_ppc64().

On platforms without any reserved memory regions,
get_reserved_memory_ranges() can return 0 while leaving 'rmem'
unallocated as NULL. Passing it directly leads to a kernel panic when
evaluating 'rmem->nr_ranges'.

Add a NULL check for 'rmem' to prevent this crash.

Cc: Sourabh Jain <sourabhjain@linux.ibm.com>
Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: stable@vger.kernel.org
Fixes: 0d3ff067331e ("powerpc/kexec_file: fix extra size calculation for kexec FDT")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 arch/powerpc/kexec/file_load_64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
index 8c72e12ea44e..6075b1c88511 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -664,7 +664,7 @@ unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image, struct crash_mem *
 		extra_size += (cpu_nodes - boot_cpu_node_count) * cpu_node_size();
 
 	/* Consider extra space for reserved memory ranges if any */
-	if (rmem->nr_ranges > 0)
+	if (rmem && rmem->nr_ranges > 0)
 		extra_size += sizeof(struct fdt_reserve_entry) * rmem->nr_ranges;
 
 	return extra_size + kdump_extra_fdt_size_ppc64(image, cpu_nodes);
-- 
2.34.1

Re: [PATCH v2 2/3] powerpc/kexec_file: Fix null-ptr-def in extra size calculation

[Sourabh Jain]

On 29/07/26 06:59, Jinjie Ruan wrote:
> A static Sashiko AI review identified a potential NULL pointer
> dereference in kexec_extra_fdt_size_ppc64().
>
> On platforms without any reserved memory regions,
> get_reserved_memory_ranges() can return 0 while leaving 'rmem'
> unallocated as NULL. Passing it directly leads to a kernel panic when
> evaluating 'rmem->nr_ranges'.

Yes that is possible, so it is worth adding NULL check for rmem.
Feel free to add:
Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>

>
> Add a NULL check for 'rmem' to prevent this crash.
>
> Cc: Sourabh Jain <sourabhjain@linux.ibm.com>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: stable@vger.kernel.org
> Fixes: 0d3ff067331e ("powerpc/kexec_file: fix extra size calculation for kexec FDT")
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
>   arch/powerpc/kexec/file_load_64.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kexec/file_load_64.c b/arch/powerpc/kexec/file_load_64.c
> index 8c72e12ea44e..6075b1c88511 100644
> --- a/arch/powerpc/kexec/file_load_64.c
> +++ b/arch/powerpc/kexec/file_load_64.c
> @@ -664,7 +664,7 @@ unsigned int kexec_extra_fdt_size_ppc64(struct kimage *image, struct crash_mem *
>   		extra_size += (cpu_nodes - boot_cpu_node_count) * cpu_node_size();
>   
>   	/* Consider extra space for reserved memory ranges if any */
> -	if (rmem->nr_ranges > 0)
> +	if (rmem && rmem->nr_ranges > 0)
>   		extra_size += sizeof(struct fdt_reserve_entry) * rmem->nr_ranges;
>   
>   	return extra_size + kdump_extra_fdt_size_ppc64(image, cpu_nodes);

[PATCH v2 3/3] powerpc/kexec_file: Prevent kexec range truncation

[Jinjie Ruan]

Sashiko AI review pointed out the following issue.

The __merge_memory_ranges() function incorrectly handles overlapping
memory ranges when merging them. Although sort_memory_ranges() sorts all
ranges by their start address in ascending order beforehand, the merge
logic remains defective in two ways:

1. It compares the current range's start against the previous element (i-1)
   instead of the running target index (idx)

2. It unconditionally overwrites 'ranges[idx].end' with 'ranges[i].end'.

This logic flaw leads to critical memory truncation when a larger memory
range completely subsumes subsequent smaller ranges.

For example, consider a sorted input array with three ranges:
  Range A (idx=0): [0x1000 - 0x9000]
  Range B (i=1):   [0x2000 - 0x5000] (completely inside Range A)
  Range C (i=2):   [0x6000 - 0x8000] (completely inside Range A)

1. When i=1 (Range B):
   ranges[1].start (0x2000) <= ranges[0].end + 1 (0x9001) is TRUE.
   The code executes: ranges[0].end = ranges[1].end, which erroneously
   shrinks Range A's end from 0x9000 down to 0x5000.

2. When i=2 (Range C):
   ranges[2].start (0x6000) <= ranges[1].end + 1 (0x5001) is FALSE.
   The code falls into the else block, creating a broken new range.

As a result, valid memory fragments [0x5001 - 0x5fff] and [0x8001 - 0x9000]
are completely lost from the kexec exclude lists, potentially allowing
the crash kernel to overwrite active memory, causing data corruption
or crashes.

Fix this by ensuring the start of the current range is compared against the
end of the active merged range (idx), and use max() to safely prevent the
outer boundary from being truncated.

Cc: Sourabh Jain <sourabhjain@linux.ibm.com>
Cc: Hari Bathini <hbathini@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: stable@vger.kernel.org
Fixes: 180adfc532a8 ("powerpc/kexec_file: Add helper functions for getting memory ranges")
Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
---
 arch/powerpc/kexec/ranges.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/kexec/ranges.c b/arch/powerpc/kexec/ranges.c
index e5fea23b191b..539061d14a77 100644
--- a/arch/powerpc/kexec/ranges.c
+++ b/arch/powerpc/kexec/ranges.c
@@ -21,6 +21,7 @@
 #include <linux/of.h>
 #include <linux/slab.h>
 #include <linux/memblock.h>
+#include <linux/minmax.h>
 #include <linux/crash_core.h>
 #include <asm/sections.h>
 #include <asm/kexec_ranges.h>
@@ -105,19 +106,16 @@ static void __merge_memory_ranges(struct crash_mem *mem_rngs)
 	struct range *ranges;
 	int i, idx;
 
-	if (!mem_rngs)
+	if (!mem_rngs || mem_rngs->nr_ranges <= 1)
 		return;
 
 	idx = 0;
-	ranges = &(mem_rngs->ranges[0]);
+	ranges = mem_rngs->ranges;
 	for (i = 1; i < mem_rngs->nr_ranges; i++) {
-		if (ranges[i].start <= (ranges[i-1].end + 1))
-			ranges[idx].end = ranges[i].end;
+		if (ranges[i].start <= (ranges[idx].end + 1))
+			ranges[idx].end = max(ranges[idx].end, ranges[i].end);
 		else {
 			idx++;
-			if (i == idx)
-				continue;
-
 			ranges[idx] = ranges[i];
 		}
 	}
-- 
2.34.1

Re: [PATCH v2 3/3] powerpc/kexec_file: Prevent kexec range truncation

[Sourabh Jain]

On 29/07/26 06:59, Jinjie Ruan wrote:
> Sashiko AI review pointed out the following issue.
>
> The __merge_memory_ranges() function incorrectly handles overlapping
> memory ranges when merging them. Although sort_memory_ranges() sorts all
> ranges by their start address in ascending order beforehand, the merge
> logic remains defective in two ways:
>
> 1. It compares the current range's start against the previous element (i-1)
>     instead of the running target index (idx)
>
> 2. It unconditionally overwrites 'ranges[idx].end' with 'ranges[i].end'.
>
> This logic flaw leads to critical memory truncation when a larger memory
> range completely subsumes subsequent smaller ranges.
>
> For example, consider a sorted input array with three ranges:
>    Range A (idx=0): [0x1000 - 0x9000]
>    Range B (i=1):   [0x2000 - 0x5000] (completely inside Range A)
>    Range C (i=2):   [0x6000 - 0x8000] (completely inside Range A)
>
> 1. When i=1 (Range B):
>     ranges[1].start (0x2000) <= ranges[0].end + 1 (0x9001) is TRUE.
>     The code executes: ranges[0].end = ranges[1].end, which erroneously
>     shrinks Range A's end from 0x9000 down to 0x5000.
>
> 2. When i=2 (Range C):
>     ranges[2].start (0x6000) <= ranges[1].end + 1 (0x5001) is FALSE.
>     The code falls into the else block, creating a broken new range.
>
> As a result, valid memory fragments [0x5001 - 0x5fff] and [0x8001 - 0x9000]
> are completely lost from the kexec exclude lists, potentially allowing
> the crash kernel to overwrite active memory, causing data corruption
> or crashes.
>
> Fix this by ensuring the start of the current range is compared against the
> end of the active merged range (idx), and use max() to safely prevent the
> outer boundary from being truncated.
>
> Cc: Sourabh Jain <sourabhjain@linux.ibm.com>
> Cc: Hari Bathini <hbathini@linux.ibm.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: stable@vger.kernel.org
> Fixes: 180adfc532a8 ("powerpc/kexec_file: Add helper functions for getting memory ranges")
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
>   arch/powerpc/kexec/ranges.c | 12 +++++-------
>   1 file changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/kexec/ranges.c b/arch/powerpc/kexec/ranges.c
> index e5fea23b191b..539061d14a77 100644
> --- a/arch/powerpc/kexec/ranges.c
> +++ b/arch/powerpc/kexec/ranges.c
> @@ -21,6 +21,7 @@
>   #include <linux/of.h>
>   #include <linux/slab.h>
>   #include <linux/memblock.h>
> +#include <linux/minmax.h>
>   #include <linux/crash_core.h>
>   #include <asm/sections.h>
>   #include <asm/kexec_ranges.h>
> @@ -105,19 +106,16 @@ static void __merge_memory_ranges(struct crash_mem *mem_rngs)
>   	struct range *ranges;
>   	int i, idx;
>   
> -	if (!mem_rngs)
> +	if (!mem_rngs || mem_rngs->nr_ranges <= 1)
>   		return;

Although the below loop handles this but it is good to return early when 
there is
only range.

>   
>   	idx = 0;
> -	ranges = &(mem_rngs->ranges[0]);
> +	ranges = mem_rngs->ranges;
>   	for (i = 1; i < mem_rngs->nr_ranges; i++) {
> -		if (ranges[i].start <= (ranges[i-1].end + 1))
> -			ranges[idx].end = ranges[i].end;
> +		if (ranges[i].start <= (ranges[idx].end + 1))
> +			ranges[idx].end = max(ranges[idx].end, ranges[i].end);

Yeah this changes is needed.

>   		else {
>   			idx++;
> -			if (i == idx)
> -				continue;

Do we really need to remove the above condition?

Isn't this condition is helpful till we find an overlap?

- Sourabh Jain

> -
>   			ranges[idx] = ranges[i];
>   		}
>   	}

Re: [PATCH v2 3/3] powerpc/kexec_file: Prevent kexec range truncation

[Jinjie Ruan]

在 2026/7/29 12:43, Sourabh Jain 写道:
> 
> 
> On 29/07/26 06:59, Jinjie Ruan wrote:
>> Sashiko AI review pointed out the following issue.
>>
>> The __merge_memory_ranges() function incorrectly handles overlapping
>> memory ranges when merging them. Although sort_memory_ranges() sorts all
>> ranges by their start address in ascending order beforehand, the merge
>> logic remains defective in two ways:
>>
>> 1. It compares the current range's start against the previous element
>> (i-1)
>>     instead of the running target index (idx)
>>
>> 2. It unconditionally overwrites 'ranges[idx].end' with 'ranges[i].end'.
>>
>> This logic flaw leads to critical memory truncation when a larger memory
>> range completely subsumes subsequent smaller ranges.
>>
>> For example, consider a sorted input array with three ranges:
>>    Range A (idx=0): [0x1000 - 0x9000]
>>    Range B (i=1):   [0x2000 - 0x5000] (completely inside Range A)
>>    Range C (i=2):   [0x6000 - 0x8000] (completely inside Range A)
>>
>> 1. When i=1 (Range B):
>>     ranges[1].start (0x2000) <= ranges[0].end + 1 (0x9001) is TRUE.
>>     The code executes: ranges[0].end = ranges[1].end, which erroneously
>>     shrinks Range A's end from 0x9000 down to 0x5000.
>>
>> 2. When i=2 (Range C):
>>     ranges[2].start (0x6000) <= ranges[1].end + 1 (0x5001) is FALSE.
>>     The code falls into the else block, creating a broken new range.
>>
>> As a result, valid memory fragments [0x5001 - 0x5fff] and [0x8001 -
>> 0x9000]
>> are completely lost from the kexec exclude lists, potentially allowing
>> the crash kernel to overwrite active memory, causing data corruption
>> or crashes.
>>
>> Fix this by ensuring the start of the current range is compared
>> against the
>> end of the active merged range (idx), and use max() to safely prevent the
>> outer boundary from being truncated.
>>
>> Cc: Sourabh Jain <sourabhjain@linux.ibm.com>
>> Cc: Hari Bathini <hbathini@linux.ibm.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: stable@vger.kernel.org
>> Fixes: 180adfc532a8 ("powerpc/kexec_file: Add helper functions for
>> getting memory ranges")
>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>> ---
>>   arch/powerpc/kexec/ranges.c | 12 +++++-------
>>   1 file changed, 5 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/powerpc/kexec/ranges.c b/arch/powerpc/kexec/ranges.c
>> index e5fea23b191b..539061d14a77 100644
>> --- a/arch/powerpc/kexec/ranges.c
>> +++ b/arch/powerpc/kexec/ranges.c
>> @@ -21,6 +21,7 @@
>>   #include <linux/of.h>
>>   #include <linux/slab.h>
>>   #include <linux/memblock.h>
>> +#include <linux/minmax.h>
>>   #include <linux/crash_core.h>
>>   #include <asm/sections.h>
>>   #include <asm/kexec_ranges.h>
>> @@ -105,19 +106,16 @@ static void __merge_memory_ranges(struct
>> crash_mem *mem_rngs)
>>       struct range *ranges;
>>       int i, idx;
>>   -    if (!mem_rngs)
>> +    if (!mem_rngs || mem_rngs->nr_ranges <= 1)
>>           return;
> 
> Although the below loop handles this but it is good to return early when
> there is
> only range.
> 
>>         idx = 0;
>> -    ranges = &(mem_rngs->ranges[0]);
>> +    ranges = mem_rngs->ranges;
>>       for (i = 1; i < mem_rngs->nr_ranges; i++) {
>> -        if (ranges[i].start <= (ranges[i-1].end + 1))
>> -            ranges[idx].end = ranges[i].end;
>> +        if (ranges[i].start <= (ranges[idx].end + 1))
>> +            ranges[idx].end = max(ranges[idx].end, ranges[i].end);
> 
> Yeah this changes is needed.
> 
>>           else {
>>               idx++;
>> -            if (i == idx)
>> -                continue;
> 
> Do we really need to remove the above condition?
> 
> Isn't this condition is helpful till we find an overlap?

Hi Sourabh,

I believe there is no functional change here. The else branch itself
already indicates that there will be no overlap this time, so we can
safely use ranges[i] as the next memory region.

Whether we remove it or keep it is fine.

> 
> - Sourabh Jain
> 
>> -
>>               ranges[idx] = ranges[i];
>>           }
>>       }
> 

Re: [PATCH v2 3/3] powerpc/kexec_file: Prevent kexec range truncation

[Sourabh Jain]

On 29/07/26 11:56, Jinjie Ruan wrote:
>
> 在 2026/7/29 12:43, Sourabh Jain 写道:
>>
>> On 29/07/26 06:59, Jinjie Ruan wrote:
>>> Sashiko AI review pointed out the following issue.
>>>
>>> The __merge_memory_ranges() function incorrectly handles overlapping
>>> memory ranges when merging them. Although sort_memory_ranges() sorts all
>>> ranges by their start address in ascending order beforehand, the merge
>>> logic remains defective in two ways:
>>>
>>> 1. It compares the current range's start against the previous element
>>> (i-1)
>>>      instead of the running target index (idx)
>>>
>>> 2. It unconditionally overwrites 'ranges[idx].end' with 'ranges[i].end'.
>>>
>>> This logic flaw leads to critical memory truncation when a larger memory
>>> range completely subsumes subsequent smaller ranges.
>>>
>>> For example, consider a sorted input array with three ranges:
>>>     Range A (idx=0): [0x1000 - 0x9000]
>>>     Range B (i=1):   [0x2000 - 0x5000] (completely inside Range A)
>>>     Range C (i=2):   [0x6000 - 0x8000] (completely inside Range A)
>>>
>>> 1. When i=1 (Range B):
>>>      ranges[1].start (0x2000) <= ranges[0].end + 1 (0x9001) is TRUE.
>>>      The code executes: ranges[0].end = ranges[1].end, which erroneously
>>>      shrinks Range A's end from 0x9000 down to 0x5000.
>>>
>>> 2. When i=2 (Range C):
>>>      ranges[2].start (0x6000) <= ranges[1].end + 1 (0x5001) is FALSE.
>>>      The code falls into the else block, creating a broken new range.
>>>
>>> As a result, valid memory fragments [0x5001 - 0x5fff] and [0x8001 -
>>> 0x9000]
>>> are completely lost from the kexec exclude lists, potentially allowing
>>> the crash kernel to overwrite active memory, causing data corruption
>>> or crashes.
>>>
>>> Fix this by ensuring the start of the current range is compared
>>> against the
>>> end of the active merged range (idx), and use max() to safely prevent the
>>> outer boundary from being truncated.
>>>
>>> Cc: Sourabh Jain <sourabhjain@linux.ibm.com>
>>> Cc: Hari Bathini <hbathini@linux.ibm.com>
>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>> Cc: stable@vger.kernel.org
>>> Fixes: 180adfc532a8 ("powerpc/kexec_file: Add helper functions for
>>> getting memory ranges")
>>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>>> ---
>>>    arch/powerpc/kexec/ranges.c | 12 +++++-------
>>>    1 file changed, 5 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kexec/ranges.c b/arch/powerpc/kexec/ranges.c
>>> index e5fea23b191b..539061d14a77 100644
>>> --- a/arch/powerpc/kexec/ranges.c
>>> +++ b/arch/powerpc/kexec/ranges.c
>>> @@ -21,6 +21,7 @@
>>>    #include <linux/of.h>
>>>    #include <linux/slab.h>
>>>    #include <linux/memblock.h>
>>> +#include <linux/minmax.h>
>>>    #include <linux/crash_core.h>
>>>    #include <asm/sections.h>
>>>    #include <asm/kexec_ranges.h>
>>> @@ -105,19 +106,16 @@ static void __merge_memory_ranges(struct
>>> crash_mem *mem_rngs)
>>>        struct range *ranges;
>>>        int i, idx;
>>>    -    if (!mem_rngs)
>>> +    if (!mem_rngs || mem_rngs->nr_ranges <= 1)
>>>            return;
>> Although the below loop handles this but it is good to return early when
>> there is
>> only range.
>>
>>>          idx = 0;
>>> -    ranges = &(mem_rngs->ranges[0]);
>>> +    ranges = mem_rngs->ranges;
>>>        for (i = 1; i < mem_rngs->nr_ranges; i++) {
>>> -        if (ranges[i].start <= (ranges[i-1].end + 1))
>>> -            ranges[idx].end = ranges[i].end;
>>> +        if (ranges[i].start <= (ranges[idx].end + 1))
>>> +            ranges[idx].end = max(ranges[idx].end, ranges[i].end);
>> Yeah this changes is needed.
>>
>>>            else {
>>>                idx++;
>>> -            if (i == idx)
>>> -                continue;
>> Do we really need to remove the above condition?
>>
>> Isn't this condition is helpful till we find an overlap?
> Hi Sourabh,
>
> I believe there is no functional change here. The else branch itself
> already indicates that there will be no overlap this time, so we can
> safely use ranges[i] as the next memory region.

I agree that keeping or removing it doesn't change the functionality.

My point is that it helps avoid copying a range onto itself. For example:

Range A (idx = 0): [0x1000 - 0x2000]
Range B (idx = 1): [0x3000 - 0x5000]
Range C (idx = 2): [0x6000 - 0x8000]

For all three ranges,  idx = i = x and kernel will do:
ranges[x] = ranges[x];

This self-assignment can be avoided if we keep the condition instead of
removing it. Isn't it?

- Sourabh Jain


> Whether we remove it or keep it is fine.
>
>> - Sourabh Jain
>>
>>> -
>>>                ranges[idx] = ranges[i];
>>>            }
>>>        }

Re: [PATCH v2 3/3] powerpc/kexec_file: Prevent kexec range truncation

[Jinjie Ruan]

在 2026/7/29 14:44, Sourabh Jain 写道:
> 
> 
> On 29/07/26 11:56, Jinjie Ruan wrote:
>>
>> 在 2026/7/29 12:43, Sourabh Jain 写道:
>>>
>>> On 29/07/26 06:59, Jinjie Ruan wrote:
>>>> Sashiko AI review pointed out the following issue.
>>>>
>>>> The __merge_memory_ranges() function incorrectly handles overlapping
>>>> memory ranges when merging them. Although sort_memory_ranges() sorts
>>>> all
>>>> ranges by their start address in ascending order beforehand, the merge
>>>> logic remains defective in two ways:
>>>>
>>>> 1. It compares the current range's start against the previous element
>>>> (i-1)
>>>>      instead of the running target index (idx)
>>>>
>>>> 2. It unconditionally overwrites 'ranges[idx].end' with
>>>> 'ranges[i].end'.
>>>>
>>>> This logic flaw leads to critical memory truncation when a larger
>>>> memory
>>>> range completely subsumes subsequent smaller ranges.
>>>>
>>>> For example, consider a sorted input array with three ranges:
>>>>     Range A (idx=0): [0x1000 - 0x9000]
>>>>     Range B (i=1):   [0x2000 - 0x5000] (completely inside Range A)
>>>>     Range C (i=2):   [0x6000 - 0x8000] (completely inside Range A)
>>>>
>>>> 1. When i=1 (Range B):
>>>>      ranges[1].start (0x2000) <= ranges[0].end + 1 (0x9001) is TRUE.
>>>>      The code executes: ranges[0].end = ranges[1].end, which
>>>> erroneously
>>>>      shrinks Range A's end from 0x9000 down to 0x5000.
>>>>
>>>> 2. When i=2 (Range C):
>>>>      ranges[2].start (0x6000) <= ranges[1].end + 1 (0x5001) is FALSE.
>>>>      The code falls into the else block, creating a broken new range.
>>>>
>>>> As a result, valid memory fragments [0x5001 - 0x5fff] and [0x8001 -
>>>> 0x9000]
>>>> are completely lost from the kexec exclude lists, potentially allowing
>>>> the crash kernel to overwrite active memory, causing data corruption
>>>> or crashes.
>>>>
>>>> Fix this by ensuring the start of the current range is compared
>>>> against the
>>>> end of the active merged range (idx), and use max() to safely
>>>> prevent the
>>>> outer boundary from being truncated.
>>>>
>>>> Cc: Sourabh Jain <sourabhjain@linux.ibm.com>
>>>> Cc: Hari Bathini <hbathini@linux.ibm.com>
>>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>>> Cc: stable@vger.kernel.org
>>>> Fixes: 180adfc532a8 ("powerpc/kexec_file: Add helper functions for
>>>> getting memory ranges")
>>>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>>>> ---
>>>>    arch/powerpc/kexec/ranges.c | 12 +++++-------
>>>>    1 file changed, 5 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/kexec/ranges.c b/arch/powerpc/kexec/ranges.c
>>>> index e5fea23b191b..539061d14a77 100644
>>>> --- a/arch/powerpc/kexec/ranges.c
>>>> +++ b/arch/powerpc/kexec/ranges.c
>>>> @@ -21,6 +21,7 @@
>>>>    #include <linux/of.h>
>>>>    #include <linux/slab.h>
>>>>    #include <linux/memblock.h>
>>>> +#include <linux/minmax.h>
>>>>    #include <linux/crash_core.h>
>>>>    #include <asm/sections.h>
>>>>    #include <asm/kexec_ranges.h>
>>>> @@ -105,19 +106,16 @@ static void __merge_memory_ranges(struct
>>>> crash_mem *mem_rngs)
>>>>        struct range *ranges;
>>>>        int i, idx;
>>>>    -    if (!mem_rngs)
>>>> +    if (!mem_rngs || mem_rngs->nr_ranges <= 1)
>>>>            return;
>>> Although the below loop handles this but it is good to return early when
>>> there is
>>> only range.
>>>
>>>>          idx = 0;
>>>> -    ranges = &(mem_rngs->ranges[0]);
>>>> +    ranges = mem_rngs->ranges;
>>>>        for (i = 1; i < mem_rngs->nr_ranges; i++) {
>>>> -        if (ranges[i].start <= (ranges[i-1].end + 1))
>>>> -            ranges[idx].end = ranges[i].end;
>>>> +        if (ranges[i].start <= (ranges[idx].end + 1))
>>>> +            ranges[idx].end = max(ranges[idx].end, ranges[i].end);
>>> Yeah this changes is needed.
>>>
>>>>            else {
>>>>                idx++;
>>>> -            if (i == idx)
>>>> -                continue;
>>> Do we really need to remove the above condition?
>>>
>>> Isn't this condition is helpful till we find an overlap?
>> Hi Sourabh,
>>
>> I believe there is no functional change here. The else branch itself
>> already indicates that there will be no overlap this time, so we can
>> safely use ranges[i] as the next memory region.
> 
> I agree that keeping or removing it doesn't change the functionality.
> 
> My point is that it helps avoid copying a range onto itself. For example:
> 
> Range A (idx = 0): [0x1000 - 0x2000]
> Range B (idx = 1): [0x3000 - 0x5000]
> Range C (idx = 2): [0x6000 - 0x8000]
> 
> For all three ranges,  idx = i = x and kernel will do:
> ranges[x] = ranges[x];
> 
> This self-assignment can be avoided if we keep the condition instead of
> removing it. Isn't it?

Yes, with the continue we can avoid the self-assignment.

If this review is fine, can it be updated and submitted directly, or
does it need to be updated in the new version?

> 
> - Sourabh Jain
> 
> 
>> Whether we remove it or keep it is fine.
>>
>>> - Sourabh Jain
>>>
>>>> -
>>>>                ranges[idx] = ranges[i];
>>>>            }
>>>>        }
> 

Re: [PATCH v2 3/3] powerpc/kexec_file: Prevent kexec range truncation

[Sourabh Jain]

On 29/07/26 12:36, Jinjie Ruan wrote:
>
> 在 2026/7/29 14:44, Sourabh Jain 写道:
>>
>> On 29/07/26 11:56, Jinjie Ruan wrote:
>>> 在 2026/7/29 12:43, Sourabh Jain 写道:
>>>> On 29/07/26 06:59, Jinjie Ruan wrote:
>>>>> Sashiko AI review pointed out the following issue.
>>>>>
>>>>> The __merge_memory_ranges() function incorrectly handles overlapping
>>>>> memory ranges when merging them. Although sort_memory_ranges() sorts
>>>>> all
>>>>> ranges by their start address in ascending order beforehand, the merge
>>>>> logic remains defective in two ways:
>>>>>
>>>>> 1. It compares the current range's start against the previous element
>>>>> (i-1)
>>>>>       instead of the running target index (idx)
>>>>>
>>>>> 2. It unconditionally overwrites 'ranges[idx].end' with
>>>>> 'ranges[i].end'.
>>>>>
>>>>> This logic flaw leads to critical memory truncation when a larger
>>>>> memory
>>>>> range completely subsumes subsequent smaller ranges.
>>>>>
>>>>> For example, consider a sorted input array with three ranges:
>>>>>      Range A (idx=0): [0x1000 - 0x9000]
>>>>>      Range B (i=1):   [0x2000 - 0x5000] (completely inside Range A)
>>>>>      Range C (i=2):   [0x6000 - 0x8000] (completely inside Range A)
>>>>>
>>>>> 1. When i=1 (Range B):
>>>>>       ranges[1].start (0x2000) <= ranges[0].end + 1 (0x9001) is TRUE.
>>>>>       The code executes: ranges[0].end = ranges[1].end, which
>>>>> erroneously
>>>>>       shrinks Range A's end from 0x9000 down to 0x5000.
>>>>>
>>>>> 2. When i=2 (Range C):
>>>>>       ranges[2].start (0x6000) <= ranges[1].end + 1 (0x5001) is FALSE.
>>>>>       The code falls into the else block, creating a broken new range.
>>>>>
>>>>> As a result, valid memory fragments [0x5001 - 0x5fff] and [0x8001 -
>>>>> 0x9000]
>>>>> are completely lost from the kexec exclude lists, potentially allowing
>>>>> the crash kernel to overwrite active memory, causing data corruption
>>>>> or crashes.
>>>>>
>>>>> Fix this by ensuring the start of the current range is compared
>>>>> against the
>>>>> end of the active merged range (idx), and use max() to safely
>>>>> prevent the
>>>>> outer boundary from being truncated.
>>>>>
>>>>> Cc: Sourabh Jain <sourabhjain@linux.ibm.com>
>>>>> Cc: Hari Bathini <hbathini@linux.ibm.com>
>>>>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>>>>> Cc: stable@vger.kernel.org
>>>>> Fixes: 180adfc532a8 ("powerpc/kexec_file: Add helper functions for
>>>>> getting memory ranges")
>>>>> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
>>>>> ---
>>>>>     arch/powerpc/kexec/ranges.c | 12 +++++-------
>>>>>     1 file changed, 5 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/arch/powerpc/kexec/ranges.c b/arch/powerpc/kexec/ranges.c
>>>>> index e5fea23b191b..539061d14a77 100644
>>>>> --- a/arch/powerpc/kexec/ranges.c
>>>>> +++ b/arch/powerpc/kexec/ranges.c
>>>>> @@ -21,6 +21,7 @@
>>>>>     #include <linux/of.h>
>>>>>     #include <linux/slab.h>
>>>>>     #include <linux/memblock.h>
>>>>> +#include <linux/minmax.h>
>>>>>     #include <linux/crash_core.h>
>>>>>     #include <asm/sections.h>
>>>>>     #include <asm/kexec_ranges.h>
>>>>> @@ -105,19 +106,16 @@ static void __merge_memory_ranges(struct
>>>>> crash_mem *mem_rngs)
>>>>>         struct range *ranges;
>>>>>         int i, idx;
>>>>>     -    if (!mem_rngs)
>>>>> +    if (!mem_rngs || mem_rngs->nr_ranges <= 1)
>>>>>             return;
>>>> Although the below loop handles this but it is good to return early when
>>>> there is
>>>> only range.
>>>>
>>>>>           idx = 0;
>>>>> -    ranges = &(mem_rngs->ranges[0]);
>>>>> +    ranges = mem_rngs->ranges;
>>>>>         for (i = 1; i < mem_rngs->nr_ranges; i++) {
>>>>> -        if (ranges[i].start <= (ranges[i-1].end + 1))
>>>>> -            ranges[idx].end = ranges[i].end;
>>>>> +        if (ranges[i].start <= (ranges[idx].end + 1))
>>>>> +            ranges[idx].end = max(ranges[idx].end, ranges[i].end);
>>>> Yeah this changes is needed.
>>>>
>>>>>             else {
>>>>>                 idx++;
>>>>> -            if (i == idx)
>>>>> -                continue;
>>>> Do we really need to remove the above condition?
>>>>
>>>> Isn't this condition is helpful till we find an overlap?
>>> Hi Sourabh,
>>>
>>> I believe there is no functional change here. The else branch itself
>>> already indicates that there will be no overlap this time, so we can
>>> safely use ranges[i] as the next memory region.
>> I agree that keeping or removing it doesn't change the functionality.
>>
>> My point is that it helps avoid copying a range onto itself. For example:
>>
>> Range A (idx = 0): [0x1000 - 0x2000]
>> Range B (idx = 1): [0x3000 - 0x5000]
>> Range C (idx = 2): [0x6000 - 0x8000]
>>
>> For all three ranges,  idx = i = x and kernel will do:
>> ranges[x] = ranges[x];
>>
>> This self-assignment can be avoided if we keep the condition instead of
>> removing it. Isn't it?
> Yes, with the continue we can avoid the self-assignment.
>
> If this review is fine, can it be updated and submitted directly, or
> does it need to be updated in the new version?

A new version will be helpful. And feel free to add:
Reviewed-by: Sourabh Jain <sourabhjain@linux.ibm.com>

>
>> - Sourabh Jain
>>
>>
>>> Whether we remove it or keep it is fine.
>>>
>>>> - Sourabh Jain
>>>>
>>>>> -
>>>>>                 ranges[idx] = ranges[i];
>>>>>             }
>>>>>         }