Re: [PATCH] kexec: fix stack buffer overflow in update_purgatory()

[Simon Horman <horms@kernel.org>]

On Tue, Nov 10, 2026 at 10:58:41AM +0000, Simon Horman wrote:
> On Fri, Oct 30, 2026 at 11:09:18AM +0800, zhangzikang1992@126.com wrote:
> > From: zhangzikang <zhangzikang@kylinos.cn>
> > 
> > Problem:
> > A stack buffer overflow exists in "update_purgatory()". The local array "sha256_region region[SHA256_REGIONS]" (size 16) is written during iteration over "info->nr_segments" without boundary checking for the write index "j".
> > 
> > "Add_degment_phys-virt()" only issues a warning when "nr_degments" exceeds "KEXEC_MAX_SGMENTS" (=16) and continues to append segments. Therefore, "update_purgation()" may be called, causing "nr_degments" to be much larger than "SHA256-REGIONS", resulting in "region [j]" being written outside of the 16 item stack array. This type of out of bounds write will damage the Canary stack and adjacent stack frames.
> > 
> > Impact:
> > Stack buffer overflow causing process crash (resulting in denial of service through Canary termination); The potential for controllable stack destruction depends on the content of the image.
> > 
> > Fix:
> > Before writing to "region[j]" in "update_purgatory()", add a bounds check on "j" to ensure that writing stops immediately when "j" reaches "SHA256_REGIONS", preventing out-of-bounds writes from corrupting adjacent global variables.
> > 
> > Signed-off-by: zhangzikang <zhangzikang@kylinos.cn>
> > ---
> >  kexec/kexec.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/kexec/kexec.c b/kexec/kexec.c
> > index c9e4bcb..732058e 100644
> > --- a/kexec/kexec.c
> > +++ b/kexec/kexec.c
> > @@ -728,6 +728,11 @@ static void update_purgatory(struct kexec_info *info)
> >  			sha256_update(&ctx, null_buf, bytes);
> >  			nullsz -= bytes;
> >  		}
> > +		if (j >= SHA256_REGIONS) {
> > +			fprintf(stderr, "Warning: SHA256 region limit reached. "
> > +				"Skipping remaining segments from checksum region list\n");
> > +			break;
> > +		}
> >  		region[j].start = (unsigned long) info->segment[i].mem;
> >  		region[j].len   = info->segment[i].memsz;
> >  		j++;
> 
> Hi Zikang,
> 
> Thanks for your patch. I agree this is a bug.
> And that your patch avoids kexec-tools crashing
> 
> However, I think by skipping regions as your patch proposes we move the
> burden of failing what is an invalid operation to the load call, which will
> fail because the Kernel enforces a 16 segment limit. And, moreover, it's
> probably not best to create a mismatch between the calculated digest and
> the populated regions, which appears to be possible with your patch.
> 
> What I would suggest is this:
> 
> 1. Add a segment limit check to add_segment_phys_virt,
>    logging an error and returning -1 if the check fails.
> 
> 2. As a failsafe mechanism, add a check to update_purgatory,
>    but place it as early in the loop as possible, and
>    call die() rather than just logging an error and continuing.

Hi again,

I tried to code-up my own suggestion and realised that point 1 was
slightly off the mark: add_segment_phys_virt doesn't have a return value,
and there is already a segment check present.

I think what would be best is to move the check, because it seems
to come too late to avoid a possible overrun within that function,
and die() if the check fails.

Something like this:

diff --git a/kexec/kexec.c b/kexec/kexec.c
index 99ec3ff3c373..b1d2b595682e 100644
--- a/kexec/kexec.c
+++ b/kexec/kexec.c
@@ -363,6 +363,10 @@ void add_segment_phys_virt(struct kexec_info *info,
 			(void *)base, (void *)last);
 	}
 
+	if (info->nr_segments >= KEXEC_MAX_SEGMENTS) {
+		die("Kernel segment limit reached.\n");
+	}
+
 	size = (info->nr_segments + 1) * sizeof(info->segment[0]);
 	info->segment = xrealloc(info->segment, size);
 	info->segment[info->nr_segments].buf   = buf;
@@ -370,10 +374,6 @@ void add_segment_phys_virt(struct kexec_info *info,
 	info->segment[info->nr_segments].mem   = (void *)base;
 	info->segment[info->nr_segments].memsz = memsz;
 	info->nr_segments++;
-	if (info->nr_segments > KEXEC_MAX_SEGMENTS) {
-		fprintf(stderr, "Warning: kernel segment limit reached. "
-			"This will likely fail\n");
-	}
 }
 
 unsigned long add_buffer_phys_virt(struct kexec_info *info,
@@ -718,6 +718,10 @@ static void update_purgatory(struct kexec_info *info)
 			continue;
 		}
 
+		if (j >= SHA256_REGIONS) {
+			die("update_purgatory: Exceeded maximum SHA256 regions\n");
+		}
+
 		sha256_update(&ctx, info->segment[i].buf,
 			      info->segment[i].bufsz);
 		nullsz = info->segment[i].memsz - info->segment[i].bufsz;