在 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 <[email protected]>
>> Cc: Hari Bathini <[email protected]>
>> Cc: Michael Ellerman <[email protected]>
>> Cc: [email protected]
>> Fixes: 180adfc532a8 ("powerpc/kexec_file: Add helper functions for
>> getting memory ranges")
>> Signed-off-by: Jinjie Ruan <[email protected]>
>> ---
>> 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];
>> }
>> }
>
