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

在 2026/7/29 14:44, Sourabh Jain 写道:

On 29/07/26 11:56, Jinjie Ruan wrote:

quoted

在 2026/7/29 12:43, Sourabh Jain 写道:

quoted

On 29/07/26 06:59, Jinjie Ruan wrote:

quoted

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 <redacted>
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 <redacted>
---
   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.

quoted

         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.

quoted

           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

quoted

Whether we remove it or keep it is fine.

quoted

- Sourabh Jain

quoted

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